-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(drawer): Convert JS to TypeScript #4390
Changes from 14 commits
ed69731
c8e9df4
e7719ba
cf36082
d53d1bc
01d0276
53a46ee
d6d7132
b6f32cd
d3240f8
b152591
24f442c
b34993e
b5a06e5
1cd4d00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,7 +70,7 @@ class MDCDialog extends MDCComponent<MDCDialogFoundation> { | |
private container_!: HTMLElement; // assigned in initialize() | ||
private content_!: HTMLElement | null; // assigned in initialize() | ||
private defaultButton_!: HTMLElement | null; // assigned in initialize() | ||
private initialFocusEl_!: HTMLElement | null; // assigned in initialize() | ||
private initialFocusEl_!: HTMLElement | undefined; // assigned in initialize() | ||
|
||
private focusTrap_!: createFocusTrap.FocusTrap; // assigned in initialSyncWithDOM() | ||
private focusTrapFactory_!: FocusTrapFactory; // assigned in initialize() | ||
|
@@ -83,7 +83,7 @@ class MDCDialog extends MDCComponent<MDCDialogFoundation> { | |
|
||
initialize( | ||
focusTrapFactory: FocusTrapFactory = createFocusTrap as unknown as FocusTrapFactory, | ||
initialFocusEl: Element | null = null) { | ||
initialFocusEl?: Element) { | ||
const container = this.root_.querySelector<HTMLElement>(strings.CONTAINER_SELECTOR); | ||
if (!container) { | ||
throw new Error(`Dialog component requires a ${strings.CONTAINER_SELECTOR} container element`); | ||
|
@@ -93,7 +93,7 @@ class MDCDialog extends MDCComponent<MDCDialogFoundation> { | |
this.buttons_ = [].slice.call(this.root_.querySelectorAll<HTMLElement>(strings.BUTTON_SELECTOR)); | ||
this.defaultButton_ = this.root_.querySelector<HTMLElement>(strings.DEFAULT_BUTTON_SELECTOR); | ||
this.focusTrapFactory_ = focusTrapFactory; | ||
this.initialFocusEl_ = initialFocusEl as HTMLElement; | ||
this.initialFocusEl_ = initialFocusEl as HTMLElement | undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we cast to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was trying to make things simpler for Closure users (who typically pass an However, this method is called indirectly via the I'll change the type of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
this.buttonRipples_ = []; | ||
|
||
for (const buttonEl of this.buttons_) { | ||
|
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.
This seems like a contradiction in terms. Isn't this effectively
private initialFocusEl_?: HTMLElement
?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.
Confusingly, the
!
here has a slightly different meaning than when it's used after an expression.When declaring member properties,
!
is a "definite assignment assertion", meaning "I pinky swear that some value is assigned to this property, but it happens somewhere other than the constructor.In an expression (e.g.,
querySelector('.foo')!
), the bang means "this value will never be null or undefined."You make a good point though; this can be shortened to
private initialFocusEl_?: HTMLElement
.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.
Done