feat(tabs): Convert JS to TypeScript #4412
Conversation
|
All 624 screenshot tests passed for commit 338a9cd vs. |
|
All 624 screenshot tests passed for commit 12ddf9d vs. |
|
All 624 screenshot tests passed for commit 6cf5ef1 vs. |
Codecov Report
@@ Coverage Diff @@
## feat/typescript #4412 +/- ##
===================================================
+ Coverage 99.03% 99.05% +0.02%
===================================================
Files 95 98 +3
Lines 6110 6166 +56
Branches 804 808 +4
===================================================
+ Hits 6051 6108 +57
+ Misses 58 57 -1
Partials 1 1
Continue to review full report at Codecov.
|
|
All 624 screenshot tests passed for commit 85d518e vs. |
|
All 624 screenshot tests passed for commit 69d249f vs. |
| this.tabBar_ = tabBarFactory(this.tabBarEl_); | ||
| } | ||
|
|
||
| getDefaultFoundation() { | ||
| return new MDCTabBarScrollerFoundation({ | ||
| // tslint:disable:object-literal-sort-keys | ||
| const adapter: MDCTabBarScrollerAdapter = { |
There was a problem hiding this comment.
Curious why you have separate const adapter here whereas in others we've just written the adapter implementation inline? Did this need to be explicitly typed for some reason?
Same for TabBar
There was a problem hiding this comment.
The foundation argument is a Partial<MDCFooAdapter>, so tsc won't complain if we omit a method; it'll just merge it with the default implementation, which does nothing. This ensures that we'll get a compiler error if we forget a field.
| registerCapturedInteractionHandler: (evt, handler) => | ||
| this.root_.addEventListener(evt, handler, true), | ||
| this.root_.addEventListener(evt, handler as EventListener, true), |
There was a problem hiding this comment.
Are these consistent with how we've done them in other components? E.g. in checkbox and form field it looks like we specified types for the parameters... in floating label it looks like we didn't have to. So I'm a little confused... maybe this is something to revisit separately across packages anyway...
There was a problem hiding this comment.
Can you clarify? Which parameter types are you referring to in checkbox and form-field?
There was a problem hiding this comment.
I don't think you need cast this or the deregister method
There was a problem hiding this comment.
Hm. I may have been looking in different contexts for checkbox and form field. Form field and floating label don't appear to use casts at all.
material-components-web/packages/mdc-form-field/index.ts
Lines 60 to 68 in 36ce7b9
material-components-web/packages/mdc-floating-label/index.ts
Lines 58 to 59 in 36ce7b9
Meanwhile checkbox does this in its overrides for ripple's adapter:
Although ripple itself doesn't seem to involve any casts or generics either:
material-components-web/packages/mdc-ripple/index.ts
Lines 58 to 59 in 36ce7b9
So I'm probably looking at all of these out of context but I'm wondering why they seem inconsistent.
| registerCapturedInteractionHandler: (evt, handler) => | ||
| this.root_.addEventListener(evt, handler, true), | ||
| this.root_.addEventListener(evt, handler as EventListener, true), |
There was a problem hiding this comment.
I don't think you need cast this or the deregister method
…ventListener` cast; remove inferred param type
Refs #4225