Skip to content

feat(stepper): Add support for linear stepper #6116

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 7 commits into from
Aug 8, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
10 changes: 8 additions & 2 deletions src/cdk/stepper/stepper-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import {CdkStepper} from './stepper';
/** Button that moves to the next step in a stepper workflow. */
@Directive({
selector: 'button[cdkStepperNext]',
host: {'(click)': '_stepper.next()'}
host: {
'(click)': '_stepper.next()',
'type': 'button'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this, depending on how they've set up their forms they might want this button to trigger submission

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could add guidance to the docs instead?

Copy link
Author

Choose a reason for hiding this comment

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

I removed 'type': 'button'. I'll add it to the docs instead as @kara suggested.

}
})
export class CdkStepperNext {
constructor(public _stepper: CdkStepper) { }
Expand All @@ -21,7 +24,10 @@ export class CdkStepperNext {
/** Button that moves to the previous step in a stepper workflow. */
@Directive({
selector: 'button[cdkStepperPrevious]',
host: {'(click)': '_stepper.previous()'}
host: {
'(click)': '_stepper.previous()',
'type': 'button'
}
})
export class CdkStepperPrevious {
constructor(public _stepper: CdkStepper) { }
Expand Down
23 changes: 20 additions & 3 deletions src/cdk/stepper/stepper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
} from '@angular/core';
import {LEFT_ARROW, RIGHT_ARROW, ENTER, SPACE} from '@angular/cdk/keyboard';
import {CdkStepLabel} from './step-label';
import {coerceBooleanProperty} from '@angular/cdk/coercion';

/** Used to generate unique ID for each stepper component. */
let nextId = 0;
Expand All @@ -45,7 +46,7 @@ export class CdkStepperSelectionEvent {

@Component({
selector: 'cdk-step',
templateUrl: 'step.html',
templateUrl: 'step.html'
})
export class CdkStep {
/** Template for step label if it exists. */
Expand All @@ -54,6 +55,21 @@ export class CdkStep {
/** Template for step content. */
@ViewChild(TemplateRef) content: TemplateRef<any>;

/** Whether step is disabled or not. */
@Input()
get disabled() { return this._disabled; }
set disabled(value: any) {
this._disabled = coerceBooleanProperty(value);
}
private _disabled = false;
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a TODO to use the disabled mixin when it's moved to the cdk?


/** Whether the user has interacted with step or not. */
Copy link
Member

Choose a reason for hiding this comment

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

Can you expand this comment to describe what "interacted with" means?

get interacted() { return this._interacted; }
set interacted(value: any) {
this._interacted = coerceBooleanProperty(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

since this isn't an @Input() you don't have to worry about the coerceBooleanProperty stuff

}
private _interacted = false;

/** Label of the step. */
@Input()
label: string;
Expand Down Expand Up @@ -84,7 +100,8 @@ export class CdkStepper {
@Input()
get selectedIndex() { return this._selectedIndex; }
set selectedIndex(index: number) {
if (this._selectedIndex != index) {
this._steps.toArray()[this._selectedIndex].interacted = true;
if (this._selectedIndex != index && !this._steps.toArray()[index].disabled) {
this._emitStepperSelectionEvent(index);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we only want to emit a change event if the selected index changes due to user interaction, therefore this should be moved to one of the user event handlers

Copy link
Author

Choose a reason for hiding this comment

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

I thought this is commonly called by all user interactions and that's why we decided to delegate emitting a change event here. Is this not what you meant?

this._focusStep(this._selectedIndex);
}
Expand Down Expand Up @@ -153,7 +170,7 @@ export class CdkStepper {
break;
case SPACE:
case ENTER:
this._emitStepperSelectionEvent(this._focusIndex);
this.selectedIndex = this._focusIndex;
break;
default:
// Return to avoid calling preventDefault on keys that are not explicitly handled.
Expand Down
3 changes: 2 additions & 1 deletion src/cdk/stepper/tsconfig-build.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"outDir": "../../../dist/packages/cdk",
"baseUrl": ".",
"paths": {
"@angular/cdk/keyboard": ["../../../dist/packages/cdk/keyboard/public_api"]
"@angular/cdk/keyboard": ["../../../dist/packages/cdk/keyboard/public_api"],
"@angular/cdk/coercion": ["../../../dist/packages/cdk/coercion/public_api"]
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to delete the entire paths property in this tsconfig now

Copy link
Contributor

Choose a reason for hiding this comment

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

possibly... depends where her stepper branch is synced to

Copy link
Author

Choose a reason for hiding this comment

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

@jelbourn @mmalerba The stepper branch is not synced up-to-date with the master branch yet. I was going to finish merging the two form control PRs that are already out, then do the syncing all together.

}
},
"files": [
Expand Down
49 changes: 49 additions & 0 deletions src/demo-app/stepper/stepper-demo.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,52 @@
<h2>Linear Vertical Stepper Demo</h2>
<form [formGroup]="formGroup" novalidate>
Copy link
Contributor

Choose a reason for hiding this comment

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

why novalidate?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, not necessary

<div formArrayName="formArray">
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the div? why not just put formArrayName on the md-vertical-stepper

<md-vertical-stepper>
<md-step>
<div [formGroupName]="0">
Copy link
Contributor

Choose a reason for hiding this comment

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

formGroupName on md-step and get rid of extra div

<ng-template mdStepLabel>Fill out your name</ng-template>
<md-input-container>
<input mdInput placeholder="First Name" formControlName="firstNameFormCtrl" required>
<md-error>This field is required</md-error>
</md-input-container>

<md-input-container>
<input mdInput placeholder="Last Name" formControlName="lastNameFormCtrl" required>
<md-error>This field is required</md-error>
</md-input-container>
<div>
<button md-button mdStepperNext>Next</button>
</div>
</div>
</md-step>

<md-step [disabled]="!formGroup.controls.formArray.controls[0].valid">
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally speaking, using the get() method is preferred over the controls property. I'd also suggest creating a getter for the array so it's less painful:

<md-step [disabled]="formArray.get([0]).valid">
get formArray() { return this.formGroup.get('formArray'); }

But see comment below about strategy

<div [formGroupName]="1">
<ng-template mdStepLabel>
<div>Fill out your phone number</div>
</ng-template>
<md-input-container>
<input mdInput placeholder="Phone number" formControlName="phoneFormCtrl" required>
<md-error>This field is required</md-error>
</md-input-container>
<div>
<button md-button mdStepperPrevious>Back</button>
<button md-button mdStepperNext>Next</button>
</div>
</div>
</md-step>

<md-step [disabled]="!formGroup.controls.formArray.controls[1].valid">
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is quite right, you also need to make sure the previous step is enabled, otherwise you might end up in a situation like this:

  • the 1st step is invalid
  • the 2nd step is disabled, but has no controls with validation so it's considered valid
  • the 3rd step would be set to enabled since it's enabledness is based solely off the previous step's validity

(It would be nice if there was some way we could make this easier for users. I don't have any ideas off the top of my head though...)

Copy link
Contributor

Choose a reason for hiding this comment

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

What about creating a custom validator function for the group or array? It seems like you need to consider more than one form control here, which is the classic situation where that is needed. A higher-level validator would allow you to surface more information in a custom error message, like all step validity or the latest step where all steps before it are valid.

<ng-template mdStepLabel>Confirm your information</ng-template>
Everything seems correct.
<div>
<button md-button>Done</button>
</div>
</md-step>
</md-vertical-stepper>
</div>
</form>

<h2>Horizontal Stepper Demo</h2>
<md-horizontal-stepper>
<md-step *ngFor="let step of steps" [label]="step.label">
Expand Down
19 changes: 19 additions & 0 deletions src/demo-app/stepper/stepper-demo.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Component} from '@angular/core';
import {Validators, FormBuilder, FormGroup} from '@angular/forms';

@Component({
moduleId: module.id,
Expand All @@ -7,10 +8,28 @@ import {Component} from '@angular/core';
styleUrls: ['stepper-demo.scss'],
})
export class StepperDemo {
formGroup: FormGroup;

steps = [
{label: 'Confirm your name', content: 'Last name, First name.'},
{label: 'Confirm your contact information', content: '123-456-7890'},
{label: 'Confirm your address', content: '1600 Amphitheater Pkwy MTV'},
{label: 'You are now done', content: 'Finished!'}
];

constructor(private _fb: FormBuilder) { }
Copy link
Member

Choose a reason for hiding this comment

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

_fb -> _formBuilder


ngOnInit() {
this.formGroup = this._fb.group({
formArray: this._fb.array([
this._fb.group({
firstNameFormCtrl: ['', Validators.required],
lastNameFormCtrl: ['', Validators.required],
}),
this._fb.group({
phoneFormCtrl: ['', Validators.required],
})
])
});
}
}
10 changes: 8 additions & 2 deletions src/lib/stepper/stepper-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,21 @@ import {MdStepper} from './stepper';
/** Button that moves to the next step in a stepper workflow. */
@Directive({
selector: 'button[mdStepperNext], button[matStepperNext]',
host: {'(click)': '_stepper.next()'},
host: {
'(click)': '_stepper.next()',
'type': 'button'
},
providers: [{provide: CdkStepper, useExisting: MdStepper}]
})
export class MdStepperNext extends CdkStepperNext { }

/** Button that moves to the previous step in a stepper workflow. */
@Directive({
selector: 'button[mdStepperPrevious], button[matStepperPrevious]',
host: {'(click)': '_stepper.previous()'},
host: {
'(click)': '_stepper.previous()',
'type': 'button'
},
providers: [{provide: CdkStepper, useExisting: MdStepper}]
})
export class MdStepperPrevious extends CdkStepperPrevious { }
30 changes: 28 additions & 2 deletions src/lib/stepper/stepper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,52 @@ import {
// considers such imports as unused (https://github.com/Microsoft/TypeScript/issues/14953)
// tslint:disable-next-line:no-unused-variable
ElementRef,
Inject,
Optional,
QueryList,
SkipSelf,
ViewChildren
}from '@angular/core';
import {MdStepLabel} from './step-label';
import {
defaultErrorStateMatcher,
ErrorOptions,
MD_ERROR_GLOBAL_OPTIONS,
ErrorStateMatcher
} from '../core/error/error-options';
import {FormControl, FormGroupDirective, NgForm} from '@angular/forms';

@Component({
moduleId: module.id,
selector: 'md-step, mat-step',
templateUrl: 'step.html',
providers: [{provide: MD_ERROR_GLOBAL_OPTIONS, useExisting: MdStep}]
})
export class MdStep extends CdkStep {
/** Content for step label given by <ng-template matStepLabel> or <ng-template mdStepLabel>. */
@ContentChild(MdStepLabel) stepLabel: MdStepLabel;

constructor(mdStepper: MdStepper) {
/** Original ErrorStateMatcher that checks the validity of form control. */
private _originalErrorStateMatcher: ErrorStateMatcher;

constructor(mdStepper: MdStepper,
@Optional() @SkipSelf() @Inject(MD_ERROR_GLOBAL_OPTIONS) errorOptions: ErrorOptions) {
super(mdStepper);
this._originalErrorStateMatcher =
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, #6147 should make this easier

Copy link
Author

Choose a reason for hiding this comment

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

I saw the PR, but should this change be made after that PR is merged into master and 'stepper' branch has synced to upstream master?

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, just an fyi

errorOptions ? errorOptions.errorStateMatcher || defaultErrorStateMatcher
: defaultErrorStateMatcher;
}

/** Custom error state matcher that additionally checks for validity of interacted form. */
errorStateMatcher = (control: FormControl, form: FormGroupDirective | NgForm) => {
Copy link
Member

Choose a reason for hiding this comment

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

The class should implement ErrorOptions

Copy link
Member

Choose a reason for hiding this comment

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

Does it work if this is a prototype method rather than a property?

Copy link
Contributor

Choose a reason for hiding this comment

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

No because the ErrorOptions stuff is implemented kind of strangely and the context would be lost if we did it as a prototype method. I would like to refactor it, probably not as part of this PR though

let originalErrorState = this._originalErrorStateMatcher(control, form);
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment that explains the background for why we are doing this custom error matcher stuff? (i.e., everything we talked about in the meeting)

let customErrorState = control.invalid && this.interacted;

return originalErrorState || customErrorState;
}
}

export class MdStepper extends CdkStepper {
export class MdStepper extends CdkStepper implements ErrorOptions {
/** The list of step headers of the steps in the stepper. */
@ViewChildren('stepHeader') _stepHeader: QueryList<ElementRef>;

Expand Down
8 changes: 7 additions & 1 deletion src/lib/table/cell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
*/

import {Directive, ElementRef, Input, Renderer2} from '@angular/core';
import {CdkCell, CdkCellDef, CdkColumnDef, CdkHeaderCell, CdkHeaderCellDef} from '@angular/cdk/table';
import {
CdkCell,
CdkCellDef,
CdkColumnDef,
CdkHeaderCell,
CdkHeaderCellDef
} from '@angular/cdk/table';

/** Workaround for https://github.com/angular/angular/issues/17849 */
export const _MdCellDef = CdkCellDef;
Expand Down
8 changes: 7 additions & 1 deletion src/lib/table/row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@
*/

import {ChangeDetectionStrategy, Component, Directive} from '@angular/core';
import {CdkHeaderRow, CdkRow, CDK_ROW_TEMPLATE, CdkRowDef, CdkHeaderRowDef} from '@angular/cdk/table';
import {
CdkHeaderRow,
CdkRow,
CDK_ROW_TEMPLATE,
CdkRowDef,
CdkHeaderRowDef
} from '@angular/cdk/table';

/** Workaround for https://github.com/angular/angular/issues/17849 */
export const _MdHeaderRowDef = CdkHeaderRowDef;
Expand Down