Skip to content

fix(stepper): unable to skip step if completed value is overwritten #15403

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 2, 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
103 changes: 53 additions & 50 deletions src/cdk/stepper/stepper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
import {FocusableOption, FocusKeyManager} from '@angular/cdk/a11y';
import {Direction, Directionality} from '@angular/cdk/bidi';
import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion';
import {END, ENTER, HOME, SPACE, hasModifierKey} from '@angular/cdk/keycodes';
import {END, ENTER, hasModifierKey, HOME, SPACE} from '@angular/cdk/keycodes';
import {DOCUMENT} from '@angular/common';
import {
AfterViewInit,
ChangeDetectionStrategy,
Expand All @@ -18,10 +19,11 @@ import {
ContentChild,
ContentChildren,
Directive,
EventEmitter,
ElementRef,
EventEmitter,
forwardRef,
Inject,
InjectionToken,
Input,
OnChanges,
OnDestroy,
Expand All @@ -31,13 +33,12 @@ import {
TemplateRef,
ViewChild,
ViewEncapsulation,
InjectionToken,
} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {CdkStepLabel} from './step-label';
import {Observable, Subject, of as obaservableOf} from 'rxjs';
import {Observable, of as obaservableOf, Subject} from 'rxjs';
import {startWith, takeUntil} from 'rxjs/operators';

import {CdkStepHeader} from './step-header';
import {CdkStepLabel} from './step-label';

/** Used to generate unique ID for each stepper component. */
let nextId = 0;
Expand All @@ -46,10 +47,10 @@ let nextId = 0;
* Position state of the content of each step in stepper that is used for transitioning
* the content into correct position upon step selection change.
*/
export type StepContentPositionState = 'previous' | 'current' | 'next';
export type StepContentPositionState = 'previous'|'current'|'next';

/** Possible orientation of a stepper. */
export type StepperOrientation = 'horizontal' | 'vertical';
export type StepperOrientation = 'horizontal'|'vertical';

/** Change event emitted on selection changes. */
export class StepperSelectionEvent {
Expand All @@ -67,7 +68,7 @@ export class StepperSelectionEvent {
}

/** The state of each step. */
export type StepState = 'number' | 'edit' | 'done' | 'error' | string;
export type StepState = 'number'|'edit'|'done'|'error'|string;

/** Enum to represent the different states of the steps. */
export const STEP_STATE = {
Expand All @@ -78,8 +79,7 @@ export const STEP_STATE = {
};

/** InjectionToken that can be used to specify the global stepper options. */
export const STEPPER_GLOBAL_OPTIONS =
new InjectionToken<StepperOptions>('STEPPER_GLOBAL_OPTIONS');
export const STEPPER_GLOBAL_OPTIONS = new InjectionToken<StepperOptions>('STEPPER_GLOBAL_OPTIONS');

/**
* InjectionToken that can be used to specify the global stepper options.
Expand Down Expand Up @@ -149,15 +149,19 @@ export class CdkStep implements OnChanges {

/** Whether the user can return to this step once it has been marked as completed. */
@Input()
get editable(): boolean { return this._editable; }
get editable(): boolean {
return this._editable;
}
set editable(value: boolean) {
this._editable = coerceBooleanProperty(value);
}
private _editable = true;

/** Whether the completion of step is optional. */
@Input()
get optional(): boolean { return this._optional; }
get optional(): boolean {
return this._optional;
}
set optional(value: boolean) {
this._optional = coerceBooleanProperty(value);
}
Expand All @@ -166,12 +170,12 @@ export class CdkStep implements OnChanges {
/** Whether step is marked as completed. */
@Input()
get completed(): boolean {
return this._customCompleted == null ? this._getDefaultCompleted() : this._customCompleted;
return this._completedOverride == null ? this._getDefaultCompleted() : this._completedOverride;
}
set completed(value: boolean) {
this._customCompleted = coerceBooleanProperty(value);
this._completedOverride = coerceBooleanProperty(value);
}
private _customCompleted: boolean | null = null;
_completedOverride: boolean|null = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason this was made non-private?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's been a long time since I opened this PR, but from what I can tell, it's because CdkStepper also checks it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, yeah sorry was going through my old assigned PRs and found this that I never responded to


private _getDefaultCompleted() {
return this.stepControl ? this.stepControl.valid && this.interacted : this.interacted;
Expand All @@ -185,16 +189,16 @@ export class CdkStep implements OnChanges {
set hasError(value: boolean) {
this._customError = coerceBooleanProperty(value);
}
private _customError: boolean | null = null;
private _customError: boolean|null = null;

private _getDefaultError() {
return this.stepControl && this.stepControl.invalid && this.interacted;
}

/** @breaking-change 8.0.0 remove the `?` after `stepperOptions` */
constructor(
@Inject(forwardRef(() => CdkStepper)) private _stepper: CdkStepper,
@Optional() @Inject(STEPPER_GLOBAL_OPTIONS) stepperOptions?: StepperOptions) {
@Inject(forwardRef(() => CdkStepper)) private _stepper: CdkStepper,
@Optional() @Inject(STEPPER_GLOBAL_OPTIONS) stepperOptions?: StepperOptions) {
this._stepperOptions = stepperOptions ? stepperOptions : {};
this._displayDefaultIndicatorType = this._stepperOptions.displayDefaultIndicatorType !== false;
this._showError = !!this._stepperOptions.showError;
Expand All @@ -209,8 +213,8 @@ export class CdkStep implements OnChanges {
reset(): void {
this.interacted = false;

if (this._customCompleted != null) {
this._customCompleted = false;
if (this._completedOverride != null) {
this._completedOverride = false;
}

if (this._customError != null) {
Expand Down Expand Up @@ -244,7 +248,7 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
* @breaking-change 8.0.0 Remove `| undefined` once the `_document`
* constructor param is required.
*/
private _document: Document | undefined;
private _document: Document|undefined;

/**
* The list of step components that the stepper is holding.
Expand All @@ -254,7 +258,7 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
@ContentChildren(CdkStep) _steps: QueryList<CdkStep>;

/** The list of step components that the stepper is holding. */
get steps(): QueryList<CdkStep> {
get steps(): QueryList<CdkStep> {
return this._steps;
}

Expand All @@ -267,13 +271,19 @@ export class CdkStepper implements AfterViewInit, OnDestroy {

/** Whether the validity of previous steps should be checked or not. */
@Input()
get linear(): boolean { return this._linear; }
set linear(value: boolean) { this._linear = coerceBooleanProperty(value); }
get linear(): boolean {
return this._linear;
}
set linear(value: boolean) {
this._linear = coerceBooleanProperty(value);
}
private _linear = false;

/** The index of the selected step. */
@Input()
get selectedIndex() { return this._selectedIndex; }
get selectedIndex() {
return this._selectedIndex;
}
set selectedIndex(index: number) {
const newIndex = coerceNumberProperty(index);

Expand All @@ -283,8 +293,7 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
throw Error('cdkStepper: Cannot assign out-of-bounds value to `selectedIndex`.');
}

if (this._selectedIndex != newIndex &&
!this._anyControlsInvalidOrPending(newIndex) &&
if (this._selectedIndex != newIndex && !this._anyControlsInvalidOrPending(newIndex) &&
(newIndex >= this._selectedIndex || this.steps.toArray()[newIndex].editable)) {
this._updateSelectedItemIndex(index);
}
Expand All @@ -305,20 +314,18 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
}

/** Event emitted when the selected step has changed. */
@Output() selectionChange: EventEmitter<StepperSelectionEvent>
= new EventEmitter<StepperSelectionEvent>();
@Output()
selectionChange: EventEmitter<StepperSelectionEvent> = new EventEmitter<StepperSelectionEvent>();

/** Used to track unique ID for each stepper component. */
_groupId: number;

protected _orientation: StepperOrientation = 'horizontal';

constructor(
@Optional() private _dir: Directionality,
private _changeDetectorRef: ChangeDetectorRef,
// @breaking-change 8.0.0 `_elementRef` and `_document` parameters to become required.
private _elementRef?: ElementRef<HTMLElement>,
@Inject(DOCUMENT) _document?: any) {
@Optional() private _dir: Directionality, private _changeDetectorRef: ChangeDetectorRef,
// @breaking-change 8.0.0 `_elementRef` and `_document` parameters to become required.
private _elementRef?: ElementRef<HTMLElement>, @Inject(DOCUMENT) _document?: any) {
this._groupId = nextId++;
this._document = _document;
}
Expand All @@ -328,12 +335,12 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
// extend this one might have them as view chidren. We initialize the keyboard handling in
// AfterViewInit so we're guaranteed for both view and content children to be defined.
this._keyManager = new FocusKeyManager<FocusableOption>(this._stepHeader)
.withWrap()
.withVerticalOrientation(this._orientation === 'vertical');
.withWrap()
.withVerticalOrientation(this._orientation === 'vertical');

(this._dir ? this._dir.change as Observable<Direction> : obaservableOf<Direction>())
.pipe(startWith(this._layoutDirection()), takeUntil(this._destroyed))
.subscribe(direction => this._keyManager.withHorizontalOrientation(direction));
(this._dir ? (this._dir.change as Observable<Direction>) : obaservableOf<Direction>())
.pipe(startWith(this._layoutDirection()), takeUntil(this._destroyed))
.subscribe(direction => this._keyManager.withHorizontalOrientation(direction));

this._keyManager.updateActiveItemIndex(this._selectedIndex);

Expand Down Expand Up @@ -397,9 +404,8 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
const step = this.steps.toArray()[index];
const isCurrentStep = this._isCurrentStep(index);

return step._displayDefaultIndicatorType
? this._getDefaultIndicatorLogic(step, isCurrentStep)
: this._getGuidelineLogic(step, isCurrentStep, state);
return step._displayDefaultIndicatorType ? this._getDefaultIndicatorLogic(step, isCurrentStep) :
this._getGuidelineLogic(step, isCurrentStep, state);
}

private _getDefaultIndicatorLogic(step: CdkStep, isCurrentStep: boolean): StepState {
Expand All @@ -413,9 +419,7 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
}

private _getGuidelineLogic(
step: CdkStep,
isCurrentStep: boolean,
state: StepState = STEP_STATE.NUMBER): StepState {
step: CdkStep, isCurrentStep: boolean, state: StepState = STEP_STATE.NUMBER): StepState {
if (step._showError && step.hasError && !isCurrentStep) {
return STEP_STATE.ERROR;
} else if (step.completed && !isCurrentStep) {
Expand Down Expand Up @@ -486,10 +490,9 @@ export class CdkStepper implements AfterViewInit, OnDestroy {
if (this._linear && index >= 0) {
return steps.slice(0, index).some(step => {
const control = step.stepControl;
const isIncomplete = control ?
(control.invalid || control.pending || !step.interacted) :
!step.completed;
return isIncomplete && !step.optional;
const isIncomplete =
control ? (control.invalid || control.pending || !step.interacted) : !step.completed;
return isIncomplete && !step.optional && !step._completedOverride;
});
}

Expand Down
21 changes: 21 additions & 0 deletions src/material/stepper/stepper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,27 @@ describe('MatStepper', () => {
expect(steps[2].completed).toBe(true,
'Expected third step to be considered complete when doing a run after a reset.');
});

it('should be able to skip past the current step if a custom `completed` value is set', () => {
expect(testComponent.oneGroup.get('oneCtrl')!.value).toBe('');
expect(testComponent.oneGroup.get('oneCtrl')!.valid).toBe(false);
expect(testComponent.oneGroup.valid).toBe(false);
expect(stepperComponent.selectedIndex).toBe(0);

const nextButtonNativeEl = fixture.debugElement
.queryAll(By.directive(MatStepperNext))[0].nativeElement;
nextButtonNativeEl.click();
fixture.detectChanges();

expect(stepperComponent.selectedIndex).toBe(0);

stepperComponent.steps.first.completed = true;
nextButtonNativeEl.click();
fixture.detectChanges();

expect(testComponent.oneGroup.valid).toBe(false);
expect(stepperComponent.selectedIndex).toBe(1);
});
});

describe('linear stepper with a pre-defined selectedIndex', () => {
Expand Down
1 change: 1 addition & 0 deletions tools/public_api_guard/cdk/stepper.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export declare class CdkStep implements OnChanges {
_completedOverride: boolean | null;
_displayDefaultIndicatorType: boolean;
_showError: boolean;
ariaLabel: string;
Expand Down