Skip to content

fix(stepper): switch to OnPush change detection #7119

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

Merged
merged 1 commit into from
Oct 4, 2017
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
29 changes: 23 additions & 6 deletions src/cdk/stepper/stepper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;

Expand All @@ -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; }
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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++;
}

Expand All @@ -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;
Expand Down Expand Up @@ -218,6 +234,7 @@ export class CdkStepper {
previouslySelectedStep: stepsArray[this._selectedIndex],
});
this._selectedIndex = newIndex;
this._stateChanged();
}

_onKeydown(event: KeyboardEvent) {
Expand Down
4 changes: 4 additions & 0 deletions src/lib/dialog/dialog-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions src/lib/stepper/step-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
OnDestroy,
ElementRef,
Renderer2,
ChangeDetectionStrategy,
} from '@angular/core';
import {MatStepLabel} from './step-label';
import {MatStepperIntl} from './stepper-intl';
Expand All @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions src/lib/stepper/stepper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
SkipSelf,
ViewChildren,
ViewEncapsulation,
ChangeDetectionStrategy,
} from '@angular/core';
import {FormControl, FormGroupDirective, NgForm} from '@angular/forms';
import {
Expand All @@ -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 <ng-template matStepLabel>. */
Expand Down Expand Up @@ -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 { }

Expand All @@ -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 { }
3 changes: 2 additions & 1 deletion tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@
"Component": {
"encapsulation": "\\.None$",
"moduleId": "^module\\.id$",
"preserveWhitespaces": "false$"
"preserveWhitespaces": "false$",
"changeDetection": "\\.OnPush$"
}
}, "src/+(lib|cdk)/**/!(*.spec).ts"],
"require-license-banner": [
Expand Down