Skip to content
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(material-experimental/mdc-tabs): add default fitInkBarToContent option #17556

Merged
merged 4 commits into from
Nov 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 34 additions & 1 deletion src/material-experimental/mdc-tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {By} from '@angular/platform-browser';
import {BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations';
import {CommonModule} from '@angular/common';
import {Observable} from 'rxjs';
import {MatTab, MatTabGroup, MatTabHeaderPosition, MatTabsModule} from './index';
import {MAT_TABS_CONFIG, MatTab, MatTabGroup, MatTabHeaderPosition, MatTabsModule} from './index';


describe('MatTabGroup', () => {
Expand Down Expand Up @@ -698,6 +698,7 @@ describe('MatTabGroup with ink bar fit to content', () => {
const tabElement = fixture.nativeElement.querySelector('.mdc-tab');
const contentElement = tabElement.querySelector('.mdc-tab__content');
const indicatorElement = tabElement.querySelector('.mdc-tab-indicator');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(contentElement);
});

Expand All @@ -707,12 +708,44 @@ describe('MatTabGroup with ink bar fit to content', () => {

const tabElement = fixture.nativeElement.querySelector('.mdc-tab');
const indicatorElement = tabElement.querySelector('.mdc-tab-indicator');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(tabElement);

fixture.componentInstance.fitInkBarToContent = true;
fixture.detectChanges();

const contentElement = tabElement.querySelector('.mdc-tab__content');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(contentElement);
});
});


describe('MatTabNavBar with a default config', () => {
let fixture: ComponentFixture<SimpleTabsTestApp>;

beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({
imports: [MatTabsModule, BrowserAnimationsModule],
declarations: [SimpleTabsTestApp],
providers: [
{provide: MAT_TABS_CONFIG, useValue: {fitInkBarToContent: true}}
]
});

TestBed.compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(SimpleTabsTestApp);
fixture.detectChanges();
});

it('should set whether the ink bar fits to content', () => {
const tabElement = fixture.nativeElement.querySelector('.mdc-tab');
const contentElement = tabElement.querySelector('.mdc-tab__content');
const indicatorElement = tabElement.querySelector('.mdc-tab-indicator');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(contentElement);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be a good idea to also add assertions that the values here are truthy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I added an expectation that the indicator's parent element is truthy

});
});
Expand Down
2 changes: 2 additions & 0 deletions src/material-experimental/mdc-tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,7 @@ export class MatTabGroup extends _MatTabGroupBase {
@Inject(MAT_TABS_CONFIG) @Optional() defaultConfig?: MatTabsConfig,
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string) {
super(elementRef, changeDetectorRef, defaultConfig, animationMode);
this.fitInkBarToContent = defaultConfig && defaultConfig.fitInkBarToContent != null ?
defaultConfig.fitInkBarToContent : false;
}
}
35 changes: 35 additions & 0 deletions src/material-experimental/mdc-tabs/tab-nav-bar/tab-nav-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {Direction, Directionality} from '@angular/cdk/bidi';
import {Subject} from 'rxjs';
import {MatTabsModule} from '../module';
import {MatTabLink, MatTabNav} from './tab-nav-bar';
import {BrowserAnimationsModule} from '@angular/platform-browser/animations';
import {MAT_TABS_CONFIG} from '../index';


describe('MatTabNavBar', () => {
Expand Down Expand Up @@ -335,6 +337,7 @@ describe('MatTabNavBar', () => {
const tabElement = fixture.nativeElement.querySelector('.mdc-tab');
const contentElement = tabElement.querySelector('.mdc-tab__content');
const indicatorElement = tabElement.querySelector('.mdc-tab-indicator');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(contentElement);
});

Expand All @@ -344,17 +347,49 @@ describe('MatTabNavBar', () => {

const tabElement = fixture.nativeElement.querySelector('.mdc-tab');
const indicatorElement = tabElement.querySelector('.mdc-tab-indicator');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(tabElement);

fixture.componentInstance.fitInkBarToContent = true;
fixture.detectChanges();

const contentElement = tabElement.querySelector('.mdc-tab__content');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(contentElement);
});
});
});

describe('MatTabNavBar with a default config', () => {
let fixture: ComponentFixture<TabLinkWithTabIndexBinding>;

beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({
imports: [MatTabsModule, BrowserAnimationsModule],
declarations: [TabLinkWithTabIndexBinding],
providers: [
{provide: MAT_TABS_CONFIG, useValue: {fitInkBarToContent: true}}
]
});

TestBed.compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(TabLinkWithTabIndexBinding);
fixture.detectChanges();
});

it('should set whether the ink bar fits to content', () => {
const tabElement = fixture.nativeElement.querySelector('.mdc-tab');
const contentElement = tabElement.querySelector('.mdc-tab__content');
const indicatorElement = tabElement.querySelector('.mdc-tab-indicator');
expect(indicatorElement.parentElement).toBeTruthy();
expect(indicatorElement.parentElement).toBe(contentElement);
});
});


@Component({
selector: 'test-app',
template: `
Expand Down
14 changes: 12 additions & 2 deletions src/material-experimental/mdc-tabs/tab-nav-bar/tab-nav-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ import {
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {MAT_RIPPLE_GLOBAL_OPTIONS, RippleGlobalOptions} from '@angular/material/core';
import {FocusMonitor} from '@angular/cdk/a11y';
import {_MatTabNavBase, _MatTabLinkBase} from '@angular/material/tabs';
import {
_MatTabNavBase,
_MatTabLinkBase,
MAT_TABS_CONFIG,
MatTabsConfig
} from '@angular/material/tabs';
import {DOCUMENT} from '@angular/common';
import {Directionality} from '@angular/cdk/bidi';
import {ViewportRuler} from '@angular/cdk/scrolling';
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we pass the defaultConfig into the super class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extends MatPaginatedTabHeader which doesn't take the config

this.disablePagination = defaultConfig && defaultConfig.disablePagination != null ?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this one handled by the base class already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not, it extends MatPaginatedTabHeader which doesn't handle the config. You may be thinking of the _MatTabGroupBase which does handle this

defaultConfig.disablePagination : false;
this.fitInkBarToContent = defaultConfig && defaultConfig.fitInkBarToContent != null ?
defaultConfig.fitInkBarToContent : false;
}

ngAfterContentInit() {
Expand Down
1 change: 1 addition & 0 deletions src/material/tabs/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ export {MatTabNav, MatTabLink, _MatTabNavBase, _MatTabLinkBase} from './tab-nav-
export {MatTabContent} from './tab-content';
export {ScrollDirection} from './paginated-tab-header';
export * from './tabs-animations';
export {MAT_TABS_CONFIG, MatTabsConfig} from './tab-config';
29 changes: 29 additions & 0 deletions src/material/tabs/tab-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {InjectionToken} from '@angular/core';

/** Object that can be used to configure the default options for the tabs module. */
export interface MatTabsConfig {
/** Duration for the tab animation. Must be a valid CSS value (e.g. 600ms). */
animationDuration?: string;

/**
* Whether pagination should be disabled. This can be used to avoid unnecessary
* layout recalculations if it's known that pagination won't be required.
*/
disablePagination?: boolean;

/**
* Whether the ink bar should fit its width to the size of the tab label content.
* This only applies to the MDC-based tabs.
*/
fitInkBarToContent?: boolean;
}

/** 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');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this name is inconsistent with other components

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on in-person conversation, we'll file an issue for the inconsistent naming across components and address in the v10 range.

25 changes: 5 additions & 20 deletions src/material/tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,17 @@ import {
ChangeDetectorRef,
Component,
ContentChildren,
Directive,
ElementRef,
EventEmitter,
Inject,
Input,
OnDestroy,
Optional,
Output,
QueryList,
ViewChild,
ViewEncapsulation,
Optional,
Inject,
InjectionToken,
Directive,
} from '@angular/core';
import {
CanColor,
Expand All @@ -39,7 +38,8 @@ import {
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {merge, Subscription} from 'rxjs';
import {startWith} from 'rxjs/operators';
import {MatTab, MAT_TAB_GROUP} from './tab';
import {MAT_TAB_GROUP, MatTab} from './tab';
import {MAT_TABS_CONFIG, MatTabsConfig} from './tab-config';


/** Used to generate unique ID's for each tab component */
Expand All @@ -56,21 +56,6 @@ export class MatTabChangeEvent {
/** Possible positions for the tab header. */
export type MatTabHeaderPosition = 'above' | 'below';

/** Object that can be used to configure the default options for the tabs module. */
export interface MatTabsConfig {
/** Duration for the tab animation. Must be a valid CSS value (e.g. 600ms). */
animationDuration?: string;

/**
* Whether pagination should be disabled. This can be used to avoid unnecessary
* layout recalculations if it's known that pagination won't be required.
*/
disablePagination?: boolean;
}

/** 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');

// Boilerplate for applying mixins to MatTabGroup.
/** @docs-private */
class MatTabGroupMixinBase {
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/material/tabs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export declare const matTabsAnimations: {
export interface MatTabsConfig {
animationDuration?: string;
disablePagination?: boolean;
fitInkBarToContent?: boolean;
}

export declare class MatTabsModule {
Expand Down