From 9da5506b515c606ad4969319b6036ce880219333 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 16 Sep 2017 11:14:07 +0200 Subject: [PATCH] fix(stepper): switch to OnPush change detection * Adds OnPush change detection to all stepper-related components. * Enables the tslint rule that requires OnPush for all components. --- src/cdk/stepper/stepper.ts | 29 +++++++++++++++++++++++------ src/lib/dialog/dialog-container.ts | 4 ++++ src/lib/stepper/step-header.ts | 2 ++ src/lib/stepper/stepper.ts | 4 ++++ tslint.json | 3 ++- 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/cdk/stepper/stepper.ts b/src/cdk/stepper/stepper.ts index d4637008ec1b..499464e887e7 100644 --- a/src/cdk/stepper/stepper.ts +++ b/src/cdk/stepper/stepper.ts @@ -24,7 +24,10 @@ import { ViewEncapsulation, Optional, Inject, - forwardRef + forwardRef, + ChangeDetectionStrategy, + ChangeDetectorRef, + OnChanges, } from '@angular/core'; import {LEFT_ARROW, RIGHT_ARROW, ENTER, SPACE} from '@angular/cdk/keycodes'; import {CdkStepLabel} from './step-label'; @@ -62,8 +65,9 @@ export class StepperSelectionEvent { templateUrl: 'step.html', encapsulation: ViewEncapsulation.None, preserveWhitespaces: false, + changeDetection: ChangeDetectionStrategy.OnPush, }) -export class CdkStep { +export class CdkStep implements OnChanges { /** Template for step label if it exists. */ @ContentChild(CdkStepLabel) stepLabel: CdkStepLabel; @@ -73,12 +77,11 @@ export class CdkStep { /** The top level abstract control of the step. */ @Input() stepControl: AbstractControl; - /** Whether user has seen the expanded step content or not . */ + /** Whether user has seen the expanded step content or not. */ interacted = false; /** Label of the step. */ - @Input() - label: string; + @Input() label: string; @Input() get editable() { return this._editable; } @@ -115,6 +118,12 @@ export class CdkStep { select(): void { this._stepper.selected = this; } + + ngOnChanges() { + // Since basically all inputs of the MdStep get proxied through the view down to the + // underlying MdStepHeader, we have to make sure that change detection runs correctly. + this._stepper._stateChanged(); + } } @Directive({ @@ -164,7 +173,9 @@ export class CdkStepper { /** Used to track unique ID for each stepper component. */ _groupId: number; - constructor(@Optional() private _dir: Directionality) { + constructor( + @Optional() private _dir: Directionality, + private _changeDetectorRef: ChangeDetectorRef) { this._groupId = nextId++; } @@ -188,6 +199,11 @@ export class CdkStepper { return `mat-step-content-${this._groupId}-${i}`; } + /** Marks the component to be change detected. */ + _stateChanged() { + this._changeDetectorRef.markForCheck(); + } + /** Returns position state of the step with the given index. */ _getAnimationDirection(index: number): StepContentPositionState { const position = index - this._selectedIndex; @@ -218,6 +234,7 @@ export class CdkStepper { previouslySelectedStep: stepsArray[this._selectedIndex], }); this._selectedIndex = newIndex; + this._stateChanged(); } _onKeydown(event: KeyboardEvent) { diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts index 372e21ee072a..fdd382729175 100644 --- a/src/lib/dialog/dialog-container.ts +++ b/src/lib/dialog/dialog-container.ts @@ -17,6 +17,7 @@ import { ChangeDetectorRef, ViewChild, ViewEncapsulation, + ChangeDetectionStrategy, } from '@angular/core'; import {animate, AnimationEvent, state, style, transition, trigger} from '@angular/animations'; import {DOCUMENT} from '@angular/platform-browser'; @@ -51,6 +52,9 @@ export function throwMatDialogContentAlreadyAttachedError() { styleUrls: ['dialog.css'], encapsulation: ViewEncapsulation.None, preserveWhitespaces: false, + // Using OnPush for dialogs caused some G3 sync issues. Disabled until we can track them down. + // tslint:disable-next-line:validate-decorators + changeDetection: ChangeDetectionStrategy.Default, animations: [ trigger('slideDialog', [ // Note: The `enter` animation doesn't transition to something like `translate3d(0, 0, 0) diff --git a/src/lib/stepper/step-header.ts b/src/lib/stepper/step-header.ts index 42f11b755ee1..75aed4164b95 100644 --- a/src/lib/stepper/step-header.ts +++ b/src/lib/stepper/step-header.ts @@ -16,6 +16,7 @@ import { OnDestroy, ElementRef, Renderer2, + ChangeDetectionStrategy, } from '@angular/core'; import {MatStepLabel} from './step-label'; import {MatStepperIntl} from './stepper-intl'; @@ -33,6 +34,7 @@ import {Subscription} from 'rxjs/Subscription'; }, encapsulation: ViewEncapsulation.None, preserveWhitespaces: false, + changeDetection: ChangeDetectionStrategy.OnPush, }) export class MatStepHeader implements OnDestroy { private _intlSubscription: Subscription; diff --git a/src/lib/stepper/stepper.ts b/src/lib/stepper/stepper.ts index 2cf8a8feecd5..6fd0673bad2e 100644 --- a/src/lib/stepper/stepper.ts +++ b/src/lib/stepper/stepper.ts @@ -21,6 +21,7 @@ import { SkipSelf, ViewChildren, ViewEncapsulation, + ChangeDetectionStrategy, } from '@angular/core'; import {FormControl, FormGroupDirective, NgForm} from '@angular/forms'; import { @@ -43,6 +44,7 @@ export const _MatStepper = CdkStepper; providers: [{provide: MAT_ERROR_GLOBAL_OPTIONS, useExisting: MatStep}], encapsulation: ViewEncapsulation.None, preserveWhitespaces: false, + changeDetection: ChangeDetectionStrategy.OnPush, }) export class MatStep extends _MatStep implements ErrorOptions { /** Content for step label given by . */ @@ -107,6 +109,7 @@ export class MatStepper extends _MatStepper { providers: [{provide: MatStepper, useExisting: MatHorizontalStepper}], encapsulation: ViewEncapsulation.None, preserveWhitespaces: false, + changeDetection: ChangeDetectionStrategy.OnPush, }) export class MatHorizontalStepper extends MatStepper { } @@ -131,5 +134,6 @@ export class MatHorizontalStepper extends MatStepper { } providers: [{provide: MatStepper, useExisting: MatVerticalStepper}], encapsulation: ViewEncapsulation.None, preserveWhitespaces: false, + changeDetection: ChangeDetectionStrategy.OnPush, }) export class MatVerticalStepper extends MatStepper { } diff --git a/tslint.json b/tslint.json index 2e40ca7b22e9..31beb1d556e9 100644 --- a/tslint.json +++ b/tslint.json @@ -91,7 +91,8 @@ "Component": { "encapsulation": "\\.None$", "moduleId": "^module\\.id$", - "preserveWhitespaces": "false$" + "preserveWhitespaces": "false$", + "changeDetection": "\\.OnPush$" } }, "src/+(lib|cdk)/**/!(*.spec).ts"], "require-license-banner": [