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 option to fit ink bar to content #17507

Merged
merged 3 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
17 changes: 17 additions & 0 deletions src/dev-app/mdc-tabs/mdc-tabs-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,23 @@ <h2>Template labels</h2>
</mat-tab>
</mat-tab-group>

<h2>Ink bar fit to content</h2>
<button (click)="fitInkBarToContent = !fitInkBarToContent"> Toggle Fit To Content </button>
<mat-tab-group [fitInkBarToContent]="fitInkBarToContent">
<mat-tab label="First">Content 1</mat-tab>
<mat-tab label="Second">Content 2</mat-tab>
<mat-tab label="Third">Content 3</mat-tab>
<mat-tab label="Fourth" disabled>Content 4</mat-tab>
</mat-tab-group>

<h2>Ink bar fit to content</h2>
<nav mat-tab-nav-bar [fitInkBarToContent]="fitInkBarToContent">
<a mat-tab-link *ngFor="let link of links"
(click)="activeLink = link"
[active]="activeLink == link">{{link}}</a>
<a mat-tab-link disabled>Disabled Link</a>
</nav>

<h2>Lazy tabs</h2>
<mat-tab-group>
<mat-tab label="One">
Expand Down
1 change: 1 addition & 0 deletions src/dev-app/mdc-tabs/mdc-tabs-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {Component} from '@angular/core';
styleUrls: ['mdc-tabs-demo.css'],
})
export class MdcTabsDemo {
fitInkBarToContent = true;
links = ['First', 'Second', 'Third'];
lotsOfTabs = new Array(30).fill(0).map((_, index) => `Tab ${index}`);
activeLink = this.links[0];
Expand Down
91 changes: 61 additions & 30 deletions src/material-experimental/mdc-tabs/ink-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

import {ElementRef, QueryList} from '@angular/core';
import {
MDCTabIndicatorFoundation,
MDCSlidingTabIndicatorFoundation,
MDCTabIndicatorAdapter
MDCTabIndicatorAdapter,
MDCTabIndicatorFoundation
} from '@material/tab-indicator';

/**
Expand All @@ -23,7 +23,7 @@ export interface MatInkBarItem {
}

/**
* Abstraction around the MDC tab indicator that manages the ink bar of a tab header.
* Abstraction around the MDC tab indicator that acts as the tab header's ink bar.
* @docs-private
*/
export class MatInkBar {
Expand All @@ -50,49 +50,61 @@ export class MatInkBar {
const clientRect = currentItem ?
currentItem._foundation.computeContentClientRect() : undefined;

// The MDC indicator won't animate unless we give it the `ClientRect` of the previous item.
// The ink bar won't animate unless we give it the `ClientRect` of the previous item.
correspondingItem._foundation.activate(clientRect);
this._currentItem = correspondingItem;
}
}
}

/**
* Implementation of MDC's sliding tab indicator foundation.
* Implementation of MDC's sliding tab indicator (ink bar) foundation.
* @docs-private
*/
export class MatInkBarFoundation {
private _destroyed: boolean;
private _foundation: MDCTabIndicatorFoundation;
private _element: HTMLElement;
private _indicator: HTMLElement;
private _indicatorContent: HTMLElement;
private _inkBarElement: HTMLElement;
private _inkBarContentElement: HTMLElement;
private _adapter: MDCTabIndicatorAdapter = {
addClass: className => {
if (!this._destroyed) {
this._element.classList.add(className);
this._hostElement.classList.add(className);
}
},
removeClass: className => {
if (!this._destroyed) {
this._element.classList.remove(className);
this._hostElement.classList.remove(className);
}
},
setContentStyleProperty: (propName, value) => {
this._indicatorContent.style.setProperty(propName, value);
this._inkBarContentElement.style.setProperty(propName, value);
},
computeContentClientRect: () => {
// `getBoundingClientRect` isn't available on the server.
return this._destroyed || !this._indicatorContent.getBoundingClientRect ? {
return this._destroyed || !this._inkBarContentElement.getBoundingClientRect ? {
width: 0, height: 0, top: 0, left: 0, right: 0, bottom: 0
} : this._indicatorContent.getBoundingClientRect();
} : this._inkBarContentElement.getBoundingClientRect();
}
};

constructor(elementRef: ElementRef<HTMLElement>, document: Document) {
this._element = elementRef.nativeElement;
/**
* Whether the ink bar should be appended to the content, which will cause the ink bar
* to match the width of the content rather than the tab host element.
*/
get fitToContent(): boolean { return this._fitToContent; }
set fitToContent(fitToContent: boolean) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need the getter here and the setter could be turned into a function. Since this is a private class we don't need to deal with getters and setters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just tried that approach but I'm not sure I feel as comfortable with it. There are two conditions I need to meet:

  • Let clients see what the current value is
  • Let clients set a new value, and the foundation reacts to it

To satisfy the first condition, there needs to be a public value fitToContent. However, it would not be clear to clients that in order to change this value, they should not write directly to it, but instead use a setFitToContent function.

If clients didn't need to read the value, I'd agree that a setter would work, but in this case I think the getter/setter pattern is ideal

Copy link
Member

Choose a reason for hiding this comment

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

Kristiyan is saying that this component (ink bar) is not part of the public API, and as such doesn't need a getter/setter (which we only need for Angular inputs) compared to using functions

Copy link
Contributor Author

@andrewseguin andrewseguin Oct 28, 2019

Choose a reason for hiding this comment

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

EDIT: Talked with Jeremy, I did miss that y'all are saying to do "getFitToContent" and "setFitToContent" rather than the ES2015 getter/setter. I'll make the change

OLD REPLY:

Sorry, I see that my explanation may be vague regarding this.

When fitInkBarToContent is provided as an input to the MatTabGroup and MatTabNav, they store the value into the MatInkBarFoundation. This will be the source of truth for it's value (not cached in the tab group or nav). When the value is set, the foundation needs to render again, hence the need for a setter (this can be either a native setter or setFitToContent).

When our users want to access the property fitInkBarToContent, we give them the value stored in MatInkBarFoundation. There are two ways we can do this, expose the property fitToContent, or provide a getFitToContent function. If we expose the property, it's not clear from the MatTabGroup/MatTabNav that it should only use that property for getting, not for setting. In the future, we may make a change and not realize the setter must be used, not the property.

If this doesn't make sense, I may be missing what you are proposing. Let's chat in the next team meeting

Copy link
Member

Choose a reason for hiding this comment

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

Andrew and I chatted, he just didn't follow that we meant do e.g. getFitToContent and setFitToContent instead

if (this._fitToContent !== fitToContent) {
this._fitToContent = fitToContent;
if (this._inkBarElement) {
this._appendInkBarElement();
}
}
}
private _fitToContent = false;

constructor(private _hostElement: HTMLElement, private _document: Document) {
this._foundation = new MDCSlidingTabIndicatorFoundation(this._adapter);
this._createIndicator(document);
}

/** Aligns the ink bar to the current item. */
Expand All @@ -105,39 +117,58 @@ export class MatInkBarFoundation {
this._foundation.deactivate();
}

/** Gets the ClientRect of the indicator. */
/** Gets the ClientRect of the ink bar. */
computeContentClientRect() {
return this._foundation.computeContentClientRect();
}

/** Initializes the foundation. */
init() {
this._createInkBarElement(this._document);
this._foundation.init();
}

/** Destroys the foundation. */
destroy() {
const indicator = this._indicator;

if (indicator.parentNode) {
indicator.parentNode.removeChild(indicator);
if (this._inkBarElement.parentNode) {
this._inkBarElement.parentNode.removeChild(this._inkBarElement);
}

this._element = this._indicator = this._indicatorContent = null!;
this._hostElement = this._inkBarElement = this._inkBarContentElement = null!;
this._foundation.destroy();
this._destroyed = true;
}

private _createIndicator(document: Document) {
if (!this._indicator) {
const indicator = this._indicator = document.createElement('span');
const content = this._indicatorContent = document.createElement('span');
/** Creates and appends the ink bar element. */
private _createInkBarElement(document: Document) {
this._inkBarElement = document.createElement('span');
this._inkBarContentElement = document.createElement('span');

indicator.className = 'mdc-tab-indicator';
content.className = 'mdc-tab-indicator__content mdc-tab-indicator__content--underline';
this._inkBarElement.className = 'mdc-tab-indicator';
this._inkBarContentElement.className = 'mdc-tab-indicator__content' +
' mdc-tab-indicator__content--underline';

indicator.appendChild(content);
this._element.appendChild(indicator);
this._inkBarElement.appendChild(this._inkBarContentElement);
this._appendInkBarElement();
}

/**
* Appends the ink bar to the tab host element or content, depending on whether
* the ink bar should fit to content.
*/
private _appendInkBarElement() {
if (!this._inkBarElement) {
throw Error('Ink bar element has not been created and cannot be appended');
}

const parentElement = this._fitToContent ?
this._hostElement.querySelector('.mdc-tab__content') :
this._hostElement;

if (!parentElement) {
throw Error('Missing element to host the ink bar');
}

parentElement.appendChild(this._inkBarElement);
}
}
1 change: 1 addition & 0 deletions src/material-experimental/mdc-tabs/tab-group.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
[attr.aria-labelledby]="(!tab.ariaLabel && tab.ariaLabelledby) ? tab.ariaLabelledby : null"
[class.mdc-tab--active]="selectedIndex == i"
[disabled]="tab.disabled"
[fitInkBarToContent]="fitInkBarToContent"
(click)="_handleClick(tab, tabHeader, i)">
<span class="mdc-tab__ripple"></span>

Expand Down
54 changes: 54 additions & 0 deletions src/material-experimental/mdc-tabs/tab-group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,47 @@ describe('nested MatTabGroup with enabled animations', () => {
});


describe('MatTabGroup with ink bar fit to content', () => {
let fixture: ComponentFixture<TabGroupWithInkBarFitToContent>;

beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({
imports: [MatTabsModule, BrowserAnimationsModule],
declarations: [TabGroupWithInkBarFitToContent]
});

TestBed.compileComponents();
}));

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

it('should properly nest the ink bar when 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).toBe(contentElement);
});

it('should be able to move the ink bar between content and full', () => {
fixture.componentInstance.fitInkBarToContent = false;
fixture.detectChanges();

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

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

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


@Component({
template: `
<mat-tab-group class="tab-group"
Expand Down Expand Up @@ -928,3 +969,16 @@ class TabsWithCustomAnimationDuration {}
class TabGroupWithIndirectDescendantTabs {
@ViewChild(MatTabGroup) tabGroup: MatTabGroup;
}


@Component({
template: `
<mat-tab-group [fitInkBarToContent]="fitInkBarToContent">
<mat-tab label="One">Tab one content</mat-tab>
<mat-tab label="Two">Tab two content</mat-tab>
</mat-tab-group>
`,
})
class TabGroupWithInkBarFitToContent {
fitInkBarToContent = true;
}
16 changes: 12 additions & 4 deletions src/material-experimental/mdc-tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,27 @@

import {
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
ContentChildren,
ElementRef,
Inject,
Input,
Optional,
QueryList,
ViewChild,
ViewEncapsulation,
ChangeDetectorRef,
Inject,
Optional,
} from '@angular/core';
import {
_MatTabGroupBase,
MAT_TAB_GROUP,
MAT_TABS_CONFIG,
MatTabsConfig,
MAT_TAB_GROUP,
} from '@angular/material/tabs';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {MatTab} from './tab';
import {MatTabHeader} from './tab-header';
import {coerceBooleanProperty} from '@angular/cdk/coercion';

/**
* Material design tab-group component. Supports basic tab pairs (label + content) and includes
Expand Down Expand Up @@ -57,6 +59,12 @@ export class MatTabGroup extends _MatTabGroupBase {
@ViewChild('tabBodyWrapper') _tabBodyWrapper: ElementRef;
@ViewChild('tabHeader') _tabHeader: MatTabHeader;

/** Whether the ink bar should fit its width to the size of the tab label content. */
@Input()
get fitInkBarToContent(): boolean { return this._fitInkBarToContent; }
set fitInkBarToContent(v: boolean) { this._fitInkBarToContent = coerceBooleanProperty(v); }
private _fitInkBarToContent = false;

constructor(elementRef: ElementRef,
changeDetectorRef: ChangeDetectorRef,
@Inject(MAT_TABS_CONFIG) @Optional() defaultConfig?: MatTabsConfig,
Expand Down
19 changes: 15 additions & 4 deletions src/material-experimental/mdc-tabs/tab-label-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, ElementRef, Inject, OnDestroy} from '@angular/core';
import {Directive, ElementRef, Inject, Input, OnDestroy, OnInit} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {MatTabLabelWrapper as BaseMatTabLabelWrapper} from '@angular/material/tabs';
import {MatInkBarFoundation, MatInkBarItem} from './ink-bar';
import {coerceBooleanProperty} from '@angular/cdk/coercion';

/**
* Used in the `mat-tab-group` view to display tab labels.
Expand All @@ -23,12 +24,22 @@ import {MatInkBarFoundation, MatInkBarItem} from './ink-bar';
'[attr.aria-disabled]': '!!disabled',
}
})
export class MatTabLabelWrapper extends BaseMatTabLabelWrapper implements MatInkBarItem, OnDestroy {
_foundation: MatInkBarFoundation;
export class MatTabLabelWrapper extends BaseMatTabLabelWrapper
implements MatInkBarItem, OnInit, OnDestroy {
private _document: Document;

_foundation = new MatInkBarFoundation(this.elementRef.nativeElement, this._document);

/** Whether the ink bar should fit its width to the size of the tab label content. */
@Input()
get fitInkBarToContent(): boolean { return this._foundation.fitToContent; }
set fitInkBarToContent(v: boolean) { this._foundation.fitToContent = coerceBooleanProperty(v); }

constructor(public elementRef: ElementRef, @Inject(DOCUMENT) _document: any) {
super(elementRef);
this._foundation = new MatInkBarFoundation(elementRef, _document);
}

ngOnInit() {
this._foundation.init();
}

Expand Down
Loading