feat(material-experimental/mdc-tabs): add default fitInkBarToContent option #17556
Conversation
andrewseguin
added
the
P3
googlebot
added
the
cla: yes
| const tabElement = fixture.nativeElement.querySelector('.mdc-tab'); | ||
| const contentElement = tabElement.querySelector('.mdc-tab__content'); | ||
| const indicatorElement = tabElement.querySelector('.mdc-tab-indicator'); | ||
| expect(indicatorElement.parentElement).toBe(contentElement); |
There was a problem hiding this comment.
Might be a good idea to also add assertions that the values here are truthy.
There was a problem hiding this comment.
Good point, I added an expectation that the indicator's parent element is truthy
| @@ -355,6 +357,35 @@ describe('MatTabNavBar', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| describe('MatTabNavBar with a default config', () => { | |||
| let fixture: ComponentFixture<TabLinkWithTabIndexBinding>; | |||
There was a problem hiding this comment.
The indentation seems off here.
| super(elementRef, dir, ngZone, changeDetectorRef, viewportRuler, platform, animationMode); | ||
| this.disablePagination = defaultConfig && defaultConfig.disablePagination != null ? |
There was a problem hiding this comment.
Isn't this one handled by the base class already?
There was a problem hiding this comment.
Unfortunately not, it extends MatPaginatedTabHeader which doesn't handle the config. You may be thinking of the _MatTabGroupBase which does handle this
| @@ -81,8 +86,13 @@ export class MatTabNav extends _MatTabNavBase implements AfterContentInit { | |||
| * @deprecated @breaking-change 9.0.0 `platform` parameter to become required. | |||
| */ | |||
| @Optional() platform?: Platform, | |||
| @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string) { | |||
| @Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string, | |||
| @Optional() @Inject(MAT_TABS_CONFIG) defaultConfig?: MatTabsConfig) { | |||
| super(elementRef, dir, ngZone, changeDetectorRef, viewportRuler, platform, animationMode); | |||
There was a problem hiding this comment.
Should we pass the defaultConfig into the super class?
There was a problem hiding this comment.
This extends MatPaginatedTabHeader which doesn't take the config
| } | ||
|
|
||
| /** Injection token that can be used to provide the default options the tabs module. */ | ||
| export const MAT_TABS_CONFIG = new InjectionToken<MatTabsConfig>('MAT_TABS_CONFIG'); |
There was a problem hiding this comment.
I think this name is inconsistent with other components
There was a problem hiding this comment.
Based on in-person conversation, we'll file an issue for the inconsistent naming across components and address in the v10 range.
| } | ||
|
|
||
| /** Injection token that can be used to provide the default options the tabs module. */ | ||
| export const MAT_TABS_CONFIG = new InjectionToken<MatTabsConfig>('MAT_TABS_CONFIG'); |
There was a problem hiding this comment.
Based on in-person conversation, we'll file an issue for the inconsistent naming across components and address in the v10 range.
jelbourn
added
pr: lgtm
target: minor
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.