-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
feat(stepper): Add support for linear stepper #2 - each step as its own form. #6117
Conversation
src/cdk/stepper/stepper.ts
Outdated
} | ||
private _disabled = false; | ||
|
||
/** Whether the user has interacted with step or not. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious what interacted means specifically. Clicked on the step ? Add more to the comment?
<h2>Linear Vertical Stepper Demo</h2> | ||
<md-vertical-stepper> | ||
<md-step> | ||
<form [formGroup]="nameFormGroup" novalidate> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove novalidate
? It's added automatically by the forms module. Same in forms below.
</form> | ||
</md-step> | ||
|
||
<md-step [disabled]="!nameFormGroup.valid"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably more idiomatic to use nameFormGroup.invalid
here.
Changes made; ready for review. |
src/cdk/stepper/stepper.ts
Outdated
@@ -84,6 +96,10 @@ export class CdkStepper { | |||
@Input() | |||
get selectedIndex() { return this._selectedIndex; } | |||
set selectedIndex(index: number) { | |||
this._steps.toArray()[this._selectedIndex].interacted = true; | |||
for (let i = 0; i < index; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment here explaining why you return early if a preceding step is invalid
src/cdk/stepper/tsconfig-build.json
Outdated
@@ -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"] |
There was a problem hiding this comment.
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 remove the paths
from this config completely now (the tooling will just work for anything now)
</form> | ||
</md-step> | ||
|
||
<md-step [valid]="phoneFormGroup.valid"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have one step use the label
property instead of mdStepLabel
?
<md-step [valid]="phoneFormGroup.valid"> | ||
<form [formGroup]="phoneFormGroup"> | ||
<ng-template mdStepLabel> | ||
<div>Fill out your phone number</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need the div in here?
This PR is now ready for review 👍 |
src/demo-app/stepper/stepper-demo.ts
Outdated
|
||
this.phoneFormGroup = new FormGroup({ | ||
phoneCtrl: new FormControl('') | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not simplify this? Something like:
ngOnInit() {
this.nameFormGroup = new FormGroup({
firstNameCtrl: new FormControl('', Validators.required),
lastNameCtrl: new FormControl('', Validators.required)
});
this.phoneFormGroup = new FormGroup({
phoneCtrl: new FormControl('')
});
// Also, maybe use FormBuilder (this._formBuilder.group and this._formBuilder.control) above?
this.formGroup = this._formBuilder.group({
formArray: this._formBuilder.array([
this.nameFormGroup,
this.phoneFormGroup
])
});
}
src/cdk/stepper/stepper.ts
Outdated
@@ -191,7 +191,7 @@ export class CdkStepper { | |||
stepsArray[this._selectedIndex].interacted = true; | |||
if (this._linear) { | |||
for (let i = 0; i < index; i++) { | |||
if (!stepsArray[i].stepControl.valid) { | |||
if (stepsArray[i].stepControl.invalid) { | |||
return true; | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be replaced by
if (this._linear) {
return stepsArray.some(step => step.stepControl.invalid);
}
e9a9b4b
to
3f05ce2
Compare
<md-checkbox [(ngModel)]="isNonLinear">Disable linear mode</md-checkbox> | ||
|
||
<h3>Linear Vertical Stepper Demo using FormArray</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... using a single form
@@ -42,7 +43,51 @@ | |||
</md-vertical-stepper> | |||
</form> | |||
|
|||
<h2>Vertical Stepper Demo</h2> | |||
<h3>Linear Horizontal Stepper Demo using multiple FormGroups</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... using a different form for each step
<md-step [stepControl]="nameFormGroup"> | ||
<form [formGroup]="nameFormGroup"> | ||
<ng-template mdStepLabel>Fill out your name</ng-template> | ||
<md-input-container> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change all md-input-container
to md-form-field
src/demo-app/stepper/stepper-demo.ts
Outdated
@@ -35,5 +38,14 @@ export class StepperDemo { | |||
}) | |||
]) | |||
}); | |||
|
|||
this.nameFormGroup = new FormGroup({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use form builder like above
Ready for review again 😃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one nit.
<form [formGroup]="phoneFormGroup"> | ||
<ng-template mdStepLabel>Fill out your phone number</ng-template> | ||
<md-form-field> | ||
<input mdInput placeholder="Phone number" formControlName="phoneCtrl"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This is the only required field without the required
attribute.
LGTM, feel free to merge after addressing kara's nit |
…wn form. (#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review
…s its own form. (angular#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review
…s its own form. (angular#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review
…s its own form. (angular#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review
…wn form. (#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review
…tepper branch. (#5742) * Prototyping * Further work * Further prototyping * Further prototyping * Further work * Adding event emitters * Adding "selectedIndex" attribute to stepper and working on TemplateOulet. * Prototyping * Further work * Further prototyping * Further prototyping * Further work * Adding event emitters * Template rendering and selectIndex control done. * Work in progress for accessibility * Added functionalities based on the tentative API doc. * Refactor code for cdk-stepper and cdk-step * Add support for templated label * Added support for keyboard events and focus changes for accessibility. * Updated vertical stepper + added comments * Fix package-lock.json * Fix indention * Changes made based on the review * Changes based on review - event properties, selectors, SPACE support, etc. + demo * Add select() for step component + refactor to avoid circular dependency + support cycling using arrow keys * API change based on review * Minor code clean up based on review. * Several name changes, etc based on review * Add to compatibility mode list and refactor to avoid circular dependency feat(stepper): Create stepper button directives to enable adding buttons to stepper (#5951) * Create stepper button directives to enable adding buttons to stepper * Changes made based on review * Minor changes with click handlers Build changes feat(stepper): Add initial styles to stepper based on Material guidelines (#6242) * Add initial styles to stepper based on Material guidelines * Fix flex-shrink and min-width * Changes made based on review * Fix alignment * Margin modifications feat(stepper): Add support for linear stepper (#6116) * Add form controls and custom error state matcher * Modify form controls for stepper-demo and add custom validator * Move custom step validation function so that users can simply import and use * Implement @input() stepControl for each step * Add linear attribute to stepper * Add enabling/disabling linear state of demo feat(stepper): Add animation to stepper (#6361) * Add animation * Implement Angular animation * Clean up unnecessary code * Generalize animation so that vertical and horizontal steppers can use the same function Rebase onto upstream/master feat(stepper): Add unit tests for stepper (#6428) * Add unit tests for stepper * Changes made based on review * More changes based on review feat(stepper): Add support for linear stepper #2 - each step as its own form. (#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review Fix focus logic and CSS changes (#6507) feat(stepper): Add documentation for stepper (#6533) * Documentation for stepper * Revision based on review + add accessibility section feat(stepper): Support additional properties for step (#6509) * Additional properties for step * Unit tests * Code changes based on review + test name changes * Refactor code for shared functionality between vertical and horizontal stepper * Refactor md-step-header and md-step-content + optional step change * Simplify code based on review * Changes to step-header based on review * Minor changes Fix host style and demo page (#6592) Revert package.json and package-lock.json Changes made along with BUILD changes in google3 Add typography mixin Changes to address aot compiler failures fix rtl bugs
…tepper branch. (angular#5742) * Prototyping * Further work * Further prototyping * Further prototyping * Further work * Adding event emitters * Adding "selectedIndex" attribute to stepper and working on TemplateOulet. * Prototyping * Further work * Further prototyping * Further prototyping * Further work * Adding event emitters * Template rendering and selectIndex control done. * Work in progress for accessibility * Added functionalities based on the tentative API doc. * Refactor code for cdk-stepper and cdk-step * Add support for templated label * Added support for keyboard events and focus changes for accessibility. * Updated vertical stepper + added comments * Fix package-lock.json * Fix indention * Changes made based on the review * Changes based on review - event properties, selectors, SPACE support, etc. + demo * Add select() for step component + refactor to avoid circular dependency + support cycling using arrow keys * API change based on review * Minor code clean up based on review. * Several name changes, etc based on review * Add to compatibility mode list and refactor to avoid circular dependency feat(stepper): Create stepper button directives to enable adding buttons to stepper (angular#5951) * Create stepper button directives to enable adding buttons to stepper * Changes made based on review * Minor changes with click handlers Build changes feat(stepper): Add initial styles to stepper based on Material guidelines (angular#6242) * Add initial styles to stepper based on Material guidelines * Fix flex-shrink and min-width * Changes made based on review * Fix alignment * Margin modifications feat(stepper): Add support for linear stepper (angular#6116) * Add form controls and custom error state matcher * Modify form controls for stepper-demo and add custom validator * Move custom step validation function so that users can simply import and use * Implement @input() stepControl for each step * Add linear attribute to stepper * Add enabling/disabling linear state of demo feat(stepper): Add animation to stepper (angular#6361) * Add animation * Implement Angular animation * Clean up unnecessary code * Generalize animation so that vertical and horizontal steppers can use the same function Rebase onto upstream/master feat(stepper): Add unit tests for stepper (angular#6428) * Add unit tests for stepper * Changes made based on review * More changes based on review feat(stepper): Add support for linear stepper angular#2 - each step as its own form. (angular#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review Fix focus logic and CSS changes (angular#6507) feat(stepper): Add documentation for stepper (angular#6533) * Documentation for stepper * Revision based on review + add accessibility section feat(stepper): Support additional properties for step (angular#6509) * Additional properties for step * Unit tests * Code changes based on review + test name changes * Refactor code for shared functionality between vertical and horizontal stepper * Refactor md-step-header and md-step-content + optional step change * Simplify code based on review * Changes to step-header based on review * Minor changes Fix host style and demo page (angular#6592) Revert package.json and package-lock.json Changes made along with BUILD changes in google3 Add typography mixin Changes to address aot compiler failures fix rtl bugs
…tepper branch. (angular#5742) * Prototyping * Further work * Further prototyping * Further prototyping * Further work * Adding event emitters * Adding "selectedIndex" attribute to stepper and working on TemplateOulet. * Prototyping * Further work * Further prototyping * Further prototyping * Further work * Adding event emitters * Template rendering and selectIndex control done. * Work in progress for accessibility * Added functionalities based on the tentative API doc. * Refactor code for cdk-stepper and cdk-step * Add support for templated label * Added support for keyboard events and focus changes for accessibility. * Updated vertical stepper + added comments * Fix package-lock.json * Fix indention * Changes made based on the review * Changes based on review - event properties, selectors, SPACE support, etc. + demo * Add select() for step component + refactor to avoid circular dependency + support cycling using arrow keys * API change based on review * Minor code clean up based on review. * Several name changes, etc based on review * Add to compatibility mode list and refactor to avoid circular dependency feat(stepper): Create stepper button directives to enable adding buttons to stepper (angular#5951) * Create stepper button directives to enable adding buttons to stepper * Changes made based on review * Minor changes with click handlers Build changes feat(stepper): Add initial styles to stepper based on Material guidelines (angular#6242) * Add initial styles to stepper based on Material guidelines * Fix flex-shrink and min-width * Changes made based on review * Fix alignment * Margin modifications feat(stepper): Add support for linear stepper (angular#6116) * Add form controls and custom error state matcher * Modify form controls for stepper-demo and add custom validator * Move custom step validation function so that users can simply import and use * Implement @input() stepControl for each step * Add linear attribute to stepper * Add enabling/disabling linear state of demo feat(stepper): Add animation to stepper (angular#6361) * Add animation * Implement Angular animation * Clean up unnecessary code * Generalize animation so that vertical and horizontal steppers can use the same function Rebase onto upstream/master feat(stepper): Add unit tests for stepper (angular#6428) * Add unit tests for stepper * Changes made based on review * More changes based on review feat(stepper): Add support for linear stepper angular#2 - each step as its own form. (angular#6117) * Add form control - consider each step as its own form group * Comment edits * Add 'valid' to MdStep for form validation * Add [stepControl] to each step based on merging * Changes based on review Fix focus logic and CSS changes (angular#6507) feat(stepper): Add documentation for stepper (angular#6533) * Documentation for stepper * Revision based on review + add accessibility section feat(stepper): Support additional properties for step (angular#6509) * Additional properties for step * Unit tests * Code changes based on review + test name changes * Refactor code for shared functionality between vertical and horizontal stepper * Refactor md-step-header and md-step-content + optional step change * Simplify code based on review * Changes to step-header based on review * Minor changes Fix host style and demo page (angular#6592) Revert package.json and package-lock.json Changes made along with BUILD changes in google3 Add typography mixin Changes to address aot compiler failures fix rtl bugs
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This is the PR shows the approach of considering each step as its own
form
to add support for linear stepper.Alternative: #6116
(cell.ts and row.ts files were changed because there were lint errors regarding long lines.)