-
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): allow for header icons to be customized #7482
feat(stepper): allow for header icons to be customized #7482
Conversation
c19ecbd
to
585023a
Compare
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.
This is a breaking API change that wasn't on the radar for RC. I'm not sure we are able to swing it, but can do a presubmit to see
src/lib/stepper/step-header.ts
Outdated
/** Icon for the given step. */ | ||
@Input() icon: string; | ||
/** State of the given step. */ | ||
@Input() state: string; |
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.
From the name I'm not sure that state
is meant to be
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.
This component is internal anyway, but it's supposed to mean the state that the header is in (e.g. done, current).
|
||
/** Label of the given step. */ | ||
@Input() label: MatStepLabel | string; | ||
|
||
/** Overrides for the header icons, passed in via the stepper. */ | ||
@Input() iconOverrides: {[key: string]: TemplateRef<any>}; |
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 a map?
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.
It doesn't need to be. It's just a string->TemplateRef
map.
* Template to be used to override the icons inside the step header. | ||
*/ | ||
@Directive({ | ||
selector: 'ng-template[matStepperIcon]', |
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.
Does this work with
<mat-icon *matStepperIcon>
?
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.
Hmm, I haven't tried it but I suppose.
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.
@jelbourn It's not a breaking API change. It's true that I renamed an input, however it's on the mat-step-header
which is only used internally (see https://github.com/angular/material2/blob/master/src/lib/stepper/stepper-horizontal.html).
|
||
/** Label of the given step. */ | ||
@Input() label: MatStepLabel | string; | ||
|
||
/** Overrides for the header icons, passed in via the stepper. */ | ||
@Input() iconOverrides: {[key: string]: TemplateRef<any>}; |
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.
It doesn't need to be. It's just a string->TemplateRef
map.
src/lib/stepper/step-header.ts
Outdated
/** Icon for the given step. */ | ||
@Input() icon: string; | ||
/** State of the given step. */ | ||
@Input() state: string; |
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.
This component is internal anyway, but it's supposed to mean the state that the header is in (e.g. done, current).
* Template to be used to override the icons inside the step header. | ||
*/ | ||
@Directive({ | ||
selector: 'ng-template[matStepperIcon]', |
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.
Hmm, I haven't tried it but I suppose.
Ah, I didn't know it was an internal component. I'll leave it to @mmalerba to review, then |
Additionally, currently the steppers icons are rendered using the web-font variant. SVG icons (using the |
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.
Looks good, needs rebase though
aa115f8
to
f120014
Compare
8fd1dd0
to
8cc1aff
Compare
Any update on this PR? |
so, guys, what are you waiting for? Merge that thing! |
Currently users are locked into using the Material `create` and `done` icon for the step headers. These changes add the ability to customize the icons by providing an `ng-template` with an override. Fixes angular#7384.
8cc1aff
to
0515a0b
Compare
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. |
Currently users are locked into using the Material
create
anddone
icon for the step headers. These changes add the ability to customize the icons by providing anng-template
with an override.Fixes #7384.
Note: I'm not too sure about the naming (
matStepperIcon
vsmatStepIcon
). I went withmatStepperIcon
, because it applies to the entire stepper, rather than an individual step.