feat(stepper): Support additional properties for step #6509

g1shin · 2017-08-16T21:22:37Z

Add support for editable, optional, and completed properties of each step.

g1shin · 2017-08-16T21:23:42Z

I had to close and reopen the PR for additional properties of step b/c of rebase issues 😢
Previous comments addressed; ready for review.

mmalerba · 2017-08-17T15:50:03Z

src/demo-app/stepper/stepper-demo.html

@@ -19,12 +19,12 @@
      </div>
    </md-step>

-    <md-step formGroupName="1" [stepControl]="formArray.get([1])">
+    <md-step formGroupName="1" [stepControl]="formArray.get([1])" [optional]="true">


nit: can just do optional="true"

also, can you add a optional horizontal step

Or just optional with no expression

mmalerba · 2017-08-17T16:07:46Z

src/cdk/stepper/stepper.ts

+  private _optional = false;
+
+  /** Return whether step is completed or not. */
+  get completed() {


Hmm I wonder if this is something we want to give the user control of by making it an @Input()?

@jelbourn WDYT of something like this:

@Input() get completed() { return this._customCompleted == null ? this._defaultCompleted : this._customCompleted; } set completed(value) { this._customCompleted = value == null ? null : coerceBooleanProperty(value); } private _customCompleted: boolean | null = null; private get _defaultCompleted() { return this._stepControl ? this._stepControl.valid && this.interacted : this.interacted; }

This way most users could use the default completeness behavior, but they could still override if they want

Seems reasonable, though _defaultCompleted would be a function

mmalerba · 2017-08-17T16:21:59Z

src/cdk/stepper/stepper.ts

@@ -211,7 +232,7 @@ export class CdkStepper {
    const stepsArray = this._steps.toArray();
    stepsArray[this._selectedIndex].interacted = true;
    if (this._linear) {
-      return stepsArray.slice(0, index).some(step => step.stepControl.invalid);
+      return stepsArray.slice(0, index).some(step => step.stepControl.invalid && !step.optional);


This makes sense if you're thinking about the step being invalid due to a required field not being filled out, but it makes less sense if you consider invalid for other reasons (e.g. entering "1234" as your email).

Need to think more about what the right behavior for optional is...

@mmalerba Do you mean that if the step is optional but the field is invalid due to some other reason, the user shouldn't be able to move on? If it is optional, shouldn't the user be able to move on even if the input is not valid? The field will still fire an error, so the user will still be aware that the input is invalid as they move on.

I think if its optional they should be able to skip it, not enter invalid input

mmalerba · 2017-08-17T16:36:23Z

src/lib/stepper/stepper-horizontal.html

        {{i + 1}}
      </div>
+      <div class="mat-stepper-index-interacted" *ngIf="step.completed && selectedIndex != i">


I think we can simplify this a bit:

get _indicatorType(): 'number' | 'check' | 'edit' {...}

<div class="mat-step-indicator" [ngSwitch]="_indicatorType"> <span *ngSwitchCase="number">{{i + 1}}</span> <md-icon *ngSwitchCase="check">done</md-icon> <md-icon *ngSwitchCase="edit">create</md-icon> </div>

mmalerba · 2017-08-17T16:36:54Z

src/lib/stepper/stepper-horizontal.html


-      <div class="mat-stepper-label">
+      <div [ngClass]="{


do these as separate [class.xxx] bindings

jelbourn · 2017-08-17T16:51:36Z

src/cdk/stepper/stepper.ts

+  private _optional = false;
+
+  /** Return whether step is completed or not. */
+  get completed() {


Seems reasonable, though _defaultCompleted would be a function

jelbourn · 2017-08-17T16:52:36Z

src/demo-app/stepper/stepper-demo.html

@@ -19,12 +19,12 @@
      </div>
    </md-step>

-    <md-step formGroupName="1" [stepControl]="formArray.get([1])">
+    <md-step formGroupName="1" [stepControl]="formArray.get([1])" [optional]="true">


Or just optional with no expression

jelbourn · 2017-08-17T16:55:15Z

src/lib/stepper/stepper.spec.ts

@@ -457,6 +485,57 @@ function checkStepHeaderBlur(stepHeaderEl: HTMLElement, fixture: ComponentFixtur
  expect(stepHeaderEl.blur).toHaveBeenCalled();
 }

+function checkEditableStep(stepperComponent: MdStepper,


Need function descriptions. I'd also prefer to have the functions names start with assert instead of check, and potentially making the function names more specific about what they do

kara · 2017-08-17T17:13:41Z

src/lib/stepper/stepper-vertical.html

      {{i + 1}}
    </div>
+    <div class="mat-stepper-index-interacted" *ngIf="step.completed && selectedIndex != i">


Could we share this icon group between stepper vertical and stepper horizontal?

@kara Sorry, what do you mean by sharing the icon group? Do you mean like creating a separate icon group component with the template?

Yeah, I meant an internal directive for the shared functionality. If they have a lot in common, would be easier to update together.

g1shin · 2017-08-17T18:58:08Z

Ready for review again. I also left some follow-up comments 👍

g1shin · 2017-08-18T01:22:15Z

Addressed comment regarding shared code between horizontal and vertical stepper. Ready for review 😃

…l stepper

mmalerba · 2017-08-18T18:35:07Z

src/cdk/stepper/step-icon.ts

+@Directive({
+  selector: 'cdkStepIcon'
+})
+export class CdkStepIcon {


I don't think this needs a cdk version, the icon is a material design specific piece of UI

mmalerba · 2017-08-18T18:37:15Z

src/cdk/stepper/step-label-container.ts

+@Directive({
+  selector: 'cdkStepLabelContainer'
+})
+export class CdkStepLabelContainer {


also don't see a need for a cdk version of this

mmalerba · 2017-08-18T18:38:57Z

src/cdk/stepper/stepper.ts

@@ -211,7 +232,7 @@ export class CdkStepper {
    const stepsArray = this._steps.toArray();
    stepsArray[this._selectedIndex].interacted = true;
    if (this._linear) {
-      return stepsArray.slice(0, index).some(step => step.stepControl.invalid);
+      return stepsArray.slice(0, index).some(step => step.stepControl.invalid && !step.optional);


I think if its optional they should be able to skip it, not enter invalid input

mmalerba · 2017-08-18T18:41:51Z

src/lib/stepper/stepper-horizontal.html

-        <!-- It there is no label template, fall back to the text label. -->
-        <div *ngIf="!step.stepLabel">{{step.label}}</div>
-      </div>
+      <md-step-icon [step]="step" [selected]="selectedIndex == i" [index]="i"></md-step-icon>


could we not just combine these 2 internal components (and their parent div) into a single md-step-header component?

g1shin · 2017-08-21T22:04:05Z

Refactoring and optional step logic change done. Ready for review again 👍

g1shin · 2017-08-22T01:24:59Z

Changes made based on discussion. Ready for another review

mmalerba · 2017-08-22T15:34:13Z

src/lib/stepper/step-header.ts

+
+  /** Text label of the given step. */
+  @Input()
+  get label() { return this._label; }


How about just one @Input() with type MdStepLabel | string, internally you can then have getters to use in your template:

get _stringLabel(): string | null { return this.label instanceof MdStepLabel ? null : this.label; } get _templateLabel(): MdStepLabel | null { return this.label instanceof MdStepLabel ? this.label : null; }

mmalerba · 2017-08-22T15:39:34Z

src/lib/stepper/step-header.ts

+export class MdStepHeader {
+  /** Unique label ID of step header. */
+  @Input()
+  labelId: string;


rather than having these as inputs you can just do it in the parent template:

<mat-step-header [id]="_getLabelId(i)" [attr.aria-controls]="_getContentId(i)" ...></mat-step-header>

mmalerba · 2017-08-22T16:05:00Z

src/lib/stepper/stepper-vertical.html

-      <div *ngIf="!step.stepLabel">{{step.label}}</div>
-    </div>
-
+  <div class="mat-stepper-header" #stepperHeader (click)="step.select()"


you can remove this div and get access to the md-step-header element like this:

@ViewChildren(MdStepHeader, {read: ElementRef}) _stepHeaders;

(and same for horizontal)

mmalerba · 2017-08-22T16:11:01Z

src/lib/stepper/step-header.scss

+.mat-step-text-label {
+  text-overflow: ellipsis;
+  overflow: hidden;
+}


$mat-step-header-icon-size: 16px !default; .mat-step-icon { display: flex; align-items: center; justify-content: center; } .mat-step-icon .mat-icon { font-size: $mat-step-header-icon-size; height: $mat-step-header-icon-size; width: $mat-step-header-icon-size; }

g1shin · 2017-08-22T17:16:46Z

Ready for review again 😃

mmalerba

LGTM with a few nits

mmalerba · 2017-08-22T17:19:01Z

src/lib/stepper/step-header.ts

+
+  /** Label of the given step. */
+  @Input()
+  get label() { return this._label; }


nit: don't need getter/setter for this now

mmalerba · 2017-08-22T17:20:23Z

src/lib/stepper/stepper-horizontal.html

+                     [attr.aria-selected]="selectedIndex == i"
+                     [index]="i"
+                     [icon]="_getIndicatorType(i)"
+                     [label]="step.stepLabel ? step.stepLabel : step.label"


nit: can shorten to step.stepLabel || step.label

mmalerba · 2017-08-22T17:21:05Z

src/lib/stepper/stepper-vertical.html

+                   [attr.aria-selected]="selectedIndex == i"
+                   [index]="i"
+                   [icon]="_getIndicatorType(i)"
+                   [label]="step.stepLabel ? step.stepLabel : step.label"


nit: step.stepLabel || step.label

* 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

@input

…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

@input

…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

@input

…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

angular-automatic-lock-bot · 2019-09-06T22:02:52Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

g1shin assigned jelbourn, kara, g1shin and mmalerba Aug 16, 2017

g1shin requested review from jelbourn, kara and mmalerba August 16, 2017 21:22

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 16, 2017

jwshinjwshin added 3 commits August 16, 2017 16:02

Additional properties for step

af7d725

Unit tests

1bd2486

Merge branch 'stepper' of github.com:angular/material2 into steps1

81eee36

g1shin force-pushed the steps1 branch from f8c6bb0 to 81eee36 Compare August 16, 2017 23:34

mmalerba reviewed Aug 17, 2017

View reviewed changes

jelbourn reviewed Aug 17, 2017

View reviewed changes

kara reviewed Aug 17, 2017

View reviewed changes

kara unassigned jelbourn, kara, g1shin and mmalerba Aug 17, 2017

Code changes based on review + test name changes

edd0a03

g1shin assigned g1shin, kara and mmalerba Aug 17, 2017

kara removed their assignment Aug 17, 2017

g1shin force-pushed the steps1 branch from 8ddc6db to 94cbd4c Compare August 18, 2017 01:20

Refactor code for shared functionality between vertical and horizonta…

b214f70

…l stepper

g1shin force-pushed the steps1 branch from 94cbd4c to b214f70 Compare August 18, 2017 18:17

mmalerba reviewed Aug 18, 2017

View reviewed changes

g1shin force-pushed the steps1 branch from 6a0653e to add81b0 Compare August 21, 2017 22:02

g1shin force-pushed the steps1 branch from add81b0 to aa2dc53 Compare August 21, 2017 22:59

g1shin assigned kara Aug 21, 2017

Refactor md-step-header and md-step-content + optional step change

a1f1977

g1shin force-pushed the steps1 branch 2 times, most recently from ecad044 to b8b5815 Compare August 22, 2017 01:19

Simplify code based on review

ec3993a

g1shin force-pushed the steps1 branch from b8b5815 to ec3993a Compare August 22, 2017 01:20

mmalerba reviewed Aug 22, 2017

View reviewed changes

Changes to step-header based on review

c8a71f4

g1shin force-pushed the steps1 branch from 57c0751 to c8a71f4 Compare August 22, 2017 17:15

mmalerba approved these changes Aug 22, 2017

View reviewed changes

Minor changes

1a5af09

g1shin merged commit 6aa0e09 into angular:stepper Aug 22, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(stepper): Support additional properties for step #6509

feat(stepper): Support additional properties for step #6509

g1shin commented Aug 16, 2017 •

edited

g1shin commented Aug 16, 2017

mmalerba Aug 17, 2017

jelbourn Aug 17, 2017

mmalerba Aug 17, 2017

jelbourn Aug 17, 2017

mmalerba Aug 17, 2017

g1shin Aug 17, 2017 •

edited

mmalerba Aug 18, 2017

mmalerba Aug 17, 2017

mmalerba Aug 17, 2017

jelbourn Aug 17, 2017

jelbourn Aug 17, 2017

jelbourn Aug 17, 2017

kara Aug 17, 2017

g1shin Aug 17, 2017

kara Aug 17, 2017

g1shin commented Aug 17, 2017

g1shin commented Aug 18, 2017

mmalerba Aug 18, 2017

mmalerba Aug 18, 2017

mmalerba Aug 18, 2017

mmalerba Aug 18, 2017

g1shin commented Aug 21, 2017

g1shin commented Aug 22, 2017

mmalerba Aug 22, 2017

mmalerba Aug 22, 2017

mmalerba Aug 22, 2017

mmalerba Aug 22, 2017

g1shin commented Aug 22, 2017

mmalerba left a comment

mmalerba Aug 22, 2017

mmalerba Aug 22, 2017

mmalerba Aug 22, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

feat(stepper): Support additional properties for step #6509

feat(stepper): Support additional properties for step #6509

Conversation

g1shin commented Aug 16, 2017 • edited

g1shin commented Aug 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g1shin Aug 17, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g1shin commented Aug 17, 2017

g1shin commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g1shin commented Aug 21, 2017

g1shin commented Aug 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g1shin commented Aug 22, 2017

mmalerba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 6, 2019

g1shin commented Aug 16, 2017 •

edited

g1shin Aug 17, 2017 •

edited