-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(menu): Convert JS to TypeScript #4342
feat(menu): Convert JS to TypeScript #4342
Conversation
…` from properties
…to `constants.ts`
…omponents/material-components-web into feat/typescript--menu-surface
…pescript--menu-surface
90855bb
to
af4ea8f
Compare
…cript--menu-surface--menu
…cript--menu-surface--menu
Codecov Report
@@ Coverage Diff @@
## feat/typescript #4342 +/- ##
===================================================
+ Coverage 98.4% 98.52% +0.12%
===================================================
Files 93 93
Lines 5819 5832 +13
Branches 783 789 +6
===================================================
+ Hits 5726 5746 +20
+ Misses 92 85 -7
Partials 1 1
Continue to review full report at Codecov.
|
All 621 screenshot tests passed for commit 2cfb7de vs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good - just some minor changes
packages/mdc-menu/README.md
Outdated
`getParentElement(element: HTMLElement) => ?HTMLElement` | Returns the `.parentElement` element of the `element` provided. | ||
`getSelectedElementIndex(element: HTMLElement) => number` | Returns the `index` value of the element within the selection group provided, `element` that contains the `mdc-menu-item--selected` class. | ||
`getElementIndex(element: Element) => number` | Returns the `index` value of the `element`. | ||
`getParentElement(element: Element) => ?Element` | Returns the `.parentElement` element of the `element` provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also TS lib.dom.d.ts
states it is an HTMLElement
/**
* Returns the parent element.
*/
readonly parentElement: HTMLElement | null;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*/ | ||
getParentElement(element) {} | ||
getParentElement(element: Element): Element | null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to TS Node
class it is an HTMLElement that is returned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ha - ya that makes sense. I would think they want it to be Element. Ok let's leave it for now.
All 621 screenshot tests passed for commit e53bd13 vs. |
All 621 screenshot tests passed for commit 06892e7 vs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one comment about the year. looks good!
All 621 screenshot tests passed for commit ac33ef4 vs. |
All 621 screenshot tests passed for commit 4b99848 vs. |
Refs #4225