Skip to content

Commit

Permalink
chore(tabs): switch to OnPush change detection (angular#5631)
Browse files Browse the repository at this point in the history
Switches all of the tab-related components to OnPush change detection and sorts out the issues that come from the changes.

Relates to angular#5035.
  • Loading branch information
crisbeto authored and jelbourn committed Jul 11, 2017
1 parent 70117ff commit ed2ece9
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 19 deletions.
2 changes: 2 additions & 0 deletions src/lib/tabs/tab-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
Optional,
AfterViewChecked,
ViewEncapsulation,
ChangeDetectionStrategy,
} from '@angular/core';
import {
trigger,
Expand Down Expand Up @@ -59,6 +60,7 @@ export type MdTabBodyOriginState = 'left' | 'right';
templateUrl: 'tab-body.html',
styleUrls: ['tab-body.css'],
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush,
host: {
'class': 'mat-tab-body',
},
Expand Down
70 changes: 62 additions & 8 deletions src/lib/tabs/tab-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,19 @@ import {
ContentChildren,
ElementRef,
Renderer2,
ChangeDetectionStrategy,
ChangeDetectorRef,
AfterViewChecked,
AfterContentInit,
AfterContentChecked,
OnDestroy,
} from '@angular/core';
import {coerceBooleanProperty} from '../core';
import {Observable} from 'rxjs/Observable';
import {Subscription} from 'rxjs/Subscription';
import {MdTab} from './tab';
import {map} from '../core/rxjs/index';
import {merge} from 'rxjs/observable/merge';


/** Used to generate unique ID's for each tab component */
Expand All @@ -45,13 +53,16 @@ export type MdTabHeaderPosition = 'above' | 'below';
selector: 'md-tab-group, mat-tab-group',
templateUrl: 'tab-group.html',
styleUrls: ['tab-group.css'],
changeDetection: ChangeDetectionStrategy.OnPush,
host: {
'class': 'mat-tab-group',
'[class.mat-tab-group-dynamic-height]': 'dynamicHeight',
'[class.mat-tab-group-inverted-header]': 'headerPosition === "below"',
}
})
export class MdTabGroup {
export class MdTabGroup implements AfterContentInit, AfterContentChecked,
AfterViewChecked, OnDestroy {

@ContentChildren(MdTab) _tabs: QueryList<MdTab>;

@ViewChild('tabBodyWrapper') _tabBodyWrapper: ElementRef;
Expand All @@ -65,6 +76,12 @@ export class MdTabGroup {
/** Snapshot of the height of the tab body wrapper before another tab is activated. */
private _tabBodyWrapperHeight: number = 0;

/** Subscription to tabs being added/removed. */
private _tabsSubscription: Subscription;

/** Subscription to changes in the tab labels. */
private _tabLabelSubscription: Subscription;

/** Whether the tab group should grow to the size of the active tab. */
@Input()
get dynamicHeight(): boolean { return this._dynamicHeight; }
Expand All @@ -82,17 +99,14 @@ export class MdTabGroup {
set disableRipple(value) { this._disableRipple = coerceBooleanProperty(value); }
private _disableRipple: boolean = false;


private _selectedIndex: number | null = null;

/** The index of the active tab. */
@Input()
set selectedIndex(value: number | null) { this._indexToSelect = value; }
get selectedIndex(): number | null { return this._selectedIndex; }
private _selectedIndex: number | null = null;

/** Position of the tab header. */
@Input()
headerPosition: MdTabHeaderPosition = 'above';
@Input() headerPosition: MdTabHeaderPosition = 'above';

/** Output to enable support for two-way binding on `[(selectedIndex)]` */
@Output() get selectedIndexChange(): Observable<number> {
Expand All @@ -107,7 +121,7 @@ export class MdTabGroup {

private _groupId: number;

constructor(private _renderer: Renderer2) {
constructor(private _renderer: Renderer2, private _changeDetectorRef: ChangeDetectorRef) {
this._groupId = nextId++;
}

Expand Down Expand Up @@ -141,7 +155,31 @@ export class MdTabGroup {
}
});

this._selectedIndex = indexToSelect;
if (this._selectedIndex !== indexToSelect) {
this._selectedIndex = indexToSelect;
this._changeDetectorRef.markForCheck();
}
}

ngAfterContentInit() {
this._subscribeToTabLabels();

// Subscribe to changes in the amount of tabs, in order to be
// able to re-render the content as new tabs are added or removed.
this._tabsSubscription = this._tabs.changes.subscribe(() => {
this._subscribeToTabLabels();
this._changeDetectorRef.markForCheck();
});
}

ngOnDestroy() {
if (this._tabsSubscription) {
this._tabsSubscription.unsubscribe();
}

if (this._tabLabelSubscription) {
this._tabLabelSubscription.unsubscribe();
}
}

/**
Expand All @@ -165,6 +203,22 @@ export class MdTabGroup {
return event;
}

/**
* Subscribes to changes in the tab labels. This is needed, because the @Input for the label is
* on the MdTab component, whereas the data binding is inside the MdTabGroup. In order for the
* binding to be updated, we need to subscribe to changes in it and trigger change detection
* manually.
*/
private _subscribeToTabLabels() {
if (this._tabLabelSubscription) {
this._tabLabelSubscription.unsubscribe();
}

this._tabLabelSubscription = merge(...this._tabs.map(tab => tab._labelChange)).subscribe(() => {
this._changeDetectorRef.markForCheck();
});
}

/** Returns a unique id for each tab label element */
_getTabLabelId(i: number): string {
return `md-tab-label-${this._groupId}-${i}`;
Expand Down
20 changes: 14 additions & 6 deletions src/lib/tabs/tab-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
OnDestroy,
NgZone,
Renderer2,
ChangeDetectionStrategy,
ChangeDetectorRef,
} from '@angular/core';
import {
RIGHT_ARROW,
Expand Down Expand Up @@ -66,6 +68,7 @@ const EXAGGERATED_OVERSCROLL = 60;
templateUrl: 'tab-header.html',
styleUrls: ['tab-header.css'],
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush,
host: {
'class': 'mat-tab-header',
'[class.mat-tab-header-pagination-controls-enabled]': '_showPaginationControls',
Expand All @@ -74,7 +77,6 @@ const EXAGGERATED_OVERSCROLL = 60;
})
export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDestroy {
@ContentChildren(MdTabLabelWrapper) _labelWrappers: QueryList<MdTabLabelWrapper>;

@ViewChild(MdInkBar) _inkBar: MdInkBar;
@ViewChild('tabListContainer') _tabListContainer: ElementRef;
@ViewChild('tabList') _tabList: ElementRef;
Expand Down Expand Up @@ -137,13 +139,15 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
private _elementRef: ElementRef,
private _ngZone: NgZone,
private _renderer: Renderer2,
private _changeDetectorRef: ChangeDetectorRef,
@Optional() private _dir: Directionality) { }

ngAfterContentChecked(): void {
// If the number of tab labels have changed, check if scrolling should be enabled
if (this._tabLabelCount != this._labelWrappers.length) {
this._updatePagination();
this._tabLabelCount = this._labelWrappers.length;
this._changeDetectorRef.markForCheck();
}

// If the selected index has changed, scroll to the label and check if the scrolling controls
Expand All @@ -153,13 +157,15 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
this._checkScrollingControls();
this._alignInkBarToSelectedTab();
this._selectedIndexChanged = false;
this._changeDetectorRef.markForCheck();
}

// If the scroll distance has been changed (tab selected, focused, scroll controls activated),
// then translate the header to reflect this.
if (this._scrollDistanceChanged) {
this._updateTabScrollPosition();
this._scrollDistanceChanged = false;
this._changeDetectorRef.markForCheck();
}
}

Expand Down Expand Up @@ -207,6 +213,7 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
_onContentChanges() {
this._updatePagination();
this._alignInkBarToSelectedTab();
this._changeDetectorRef.markForCheck();
}

/**
Expand All @@ -224,7 +231,6 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes

this._focusIndex = value;
this.indexFocused.emit(value);

this._setTabFocus(value);
}

Expand Down Expand Up @@ -259,6 +265,7 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
// should be the full width minus the offset width.
const containerEl = this._tabListContainer.nativeElement;
const dir = this._getLayoutDirection();

if (dir == 'ltr') {
containerEl.scrollLeft = 0;
} else {
Expand All @@ -274,6 +281,7 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
_moveFocus(offset: number) {
if (this._labelWrappers) {
const tabs: MdTabLabelWrapper[] = this._labelWrappers.toArray();

for (let i = this.focusIndex + offset; i < tabs.length && i >= 0; i += offset) {
if (this._isValidIndex(i)) {
this.focusIndex = i;
Expand Down Expand Up @@ -314,7 +322,6 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
// Mark that the scroll distance has changed so that after the view is checked, the CSS
// transformation can move the header.
this._scrollDistanceChanged = true;

this._checkScrollingControls();
}
get scrollDistance(): number { return this._scrollDistance; }
Expand All @@ -341,9 +348,7 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
* should be called sparingly.
*/
_scrollToLabel(labelIndex: number) {
const selectedLabel = this._labelWrappers
? this._labelWrappers.toArray()[labelIndex]
: null;
const selectedLabel = this._labelWrappers ? this._labelWrappers.toArray()[labelIndex] : null;

if (!selectedLabel) { return; }

Expand Down Expand Up @@ -386,6 +391,8 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
if (!this._showPaginationControls) {
this.scrollDistance = 0;
}

this._changeDetectorRef.markForCheck();
}

/**
Expand All @@ -401,6 +408,7 @@ export class MdTabHeader implements AfterContentChecked, AfterContentInit, OnDes
// Check if the pagination arrows should be activated.
this._disableScrollBefore = this.scrollDistance == 0;
this._disableScrollAfter = this.scrollDistance == this._getMaxScrollDistance();
this._changeDetectorRef.markForCheck();
}

/**
Expand Down
14 changes: 12 additions & 2 deletions src/lib/tabs/tab-nav-bar/tab-nav-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import {
OnDestroy,
Optional,
ViewChild,
ViewEncapsulation
ViewEncapsulation,
ChangeDetectionStrategy,
ChangeDetectorRef,
} from '@angular/core';
import {MdInkBar} from '../ink-bar';
import {CanDisable, mixinDisabled} from '../../core/common-behaviors/disabled';
Expand All @@ -43,6 +45,7 @@ import {fromEvent} from 'rxjs/observable/fromEvent';
styleUrls: ['tab-nav-bar.css'],
host: {'class': 'mat-tab-nav-bar'},
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class MdTabNav implements AfterContentInit, OnDestroy {
/** Subject that emits when the component has been destroyed. */
Expand All @@ -56,12 +59,19 @@ export class MdTabNav implements AfterContentInit, OnDestroy {
/** Subscription for window.resize event **/
private _resizeSubscription: Subscription;

constructor(@Optional() private _dir: Directionality, private _ngZone: NgZone) { }
constructor(
@Optional() private _dir: Directionality,
private _ngZone: NgZone,
private _changeDetectorRef: ChangeDetectorRef) { }

/** Notifies the component that the active link has been changed. */
updateActiveLink(element: ElementRef) {
this._activeLinkChanged = this._activeLinkElement != element;
this._activeLinkElement = element;

if (this._activeLinkChanged) {
this._changeDetectorRef.markForCheck();
}
}

ngAfterContentInit(): void {
Expand Down
21 changes: 18 additions & 3 deletions src/lib/tabs/tab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
import {TemplatePortal} from '../core/portal/portal';
import {
ViewContainerRef, Input, TemplateRef, ViewChild, OnInit, ContentChild,
Component
Component, ChangeDetectionStrategy, OnDestroy, OnChanges, SimpleChanges,
} from '@angular/core';
import {CanDisable, mixinDisabled} from '../core/common-behaviors/disabled';
import {MdTabLabel} from './tab-label';
import {Subject} from 'rxjs/Subject';

// Boilerplate for applying mixins to MdTab.
/** @docs-private */
Expand All @@ -23,9 +24,10 @@ export const _MdTabMixinBase = mixinDisabled(MdTabBase);
moduleId: module.id,
selector: 'md-tab, mat-tab',
templateUrl: 'tab.html',
inputs: ['disabled']
inputs: ['disabled'],
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class MdTab extends _MdTabMixinBase implements OnInit, CanDisable {
export class MdTab extends _MdTabMixinBase implements OnInit, CanDisable, OnChanges, OnDestroy {
/** Content for the tab label given by <ng-template md-tab-label>. */
@ContentChild(MdTabLabel) templateLabel: MdTabLabel;

Expand All @@ -39,6 +41,9 @@ export class MdTab extends _MdTabMixinBase implements OnInit, CanDisable {
private _contentPortal: TemplatePortal | null = null;
get content(): TemplatePortal | null { return this._contentPortal; }

/** Emits whenever the label changes. */
_labelChange = new Subject<void>();

/**
* The relatively indexed position where 0 represents the center, negative is left, and positive
* represents the right.
Expand All @@ -55,6 +60,16 @@ export class MdTab extends _MdTabMixinBase implements OnInit, CanDisable {
super();
}

ngOnChanges(changes: SimpleChanges) {
if (changes.hasOwnProperty('textLabel')) {
this._labelChange.next();
}
}

ngOnDestroy() {
this._labelChange.complete();
}

ngOnInit() {
this._contentPortal = new TemplatePortal(this._content, this._viewContainerRef);
}
Expand Down

0 comments on commit ed2ece9

Please sign in to comment.