Allowing more status options for EuiSteps
#1088
Conversation
| } | ||
|
|
||
| &--complete { | ||
| .euiStepNumber__icon path { |
There was a problem hiding this comment.
Might wanna comment this one to explain why you're targeting the innards of the svg.
There was a problem hiding this comment.
@snide So that's how I had to do it because the icon itself uses the <defs> and <use> that really aren't necessary. Are you ok with me modifying the svg itself?
There was a problem hiding this comment.
Oh, yeah. I know why you're doing it and its fine to do it this way. Just think it's a little off pattern so you should probably comment the markup to explain why you're doing it.
There was a problem hiding this comment.
Yeah but I'm thinking it's bad to be doing it this way and the svg itself should be written correctly versus hacking it here.
src-docs/src/views/steps/status.js
Outdated
| }); | ||
| } | ||
|
|
||
| handleUncomplete() { |
There was a problem hiding this comment.
handleIncomplete? I guess it's "un" completing it. Words are hard.
| EuiIcon, | ||
| } from '../icon'; | ||
|
|
||
| const statusToClassNameMap = { |
There was a problem hiding this comment.
Know a lot of this is baked into not wanting this to be a breaking change (meaning I should have named complete as success...etc), but i wish this map read a little more as a set. Tempted to say warning / danger here should instead be "alert" and "error" so they all read the same and it doesn't feel tied to our coloring system (since this forces the actual icon usage as well).
I don't think it matters much, but just something I noticed when looking at the code.
In either case, can you update the doc description to make it more readible what it accepts. Can do that either by including the new component in the prop docs and referencing it, or just writing them in in the description.
There was a problem hiding this comment.
Referencing these docs if that wasn't clear.
There was a problem hiding this comment.
Yeah I wish the auto-doc props list would be able to list imported constants here.
Fixes #994
A while back we added the option for a "completion" step, but it had only one option which was "complete" with a checkmark. I've noticed in some implementations of
EuiStepsthe last step can have an error or warning like so:I separated the actual step number render from both
EuiStepandEuiStepHorizontalintoEuiStepNumberwhich allows for these status options: 'incomplete', 'complete', 'disabled', 'danger', 'warning'.I purposefully did not add this new
EuiStepNumbercomponent to the index.js manifest as it's just a dependency ofEuiStepsandEuiStepsHorizontaland should not be used on it's own.Also, there is some odd-ish logic happening in
EuiStepHorizontalbecause of the way the component was already handling it's own status and I didn't want to cause any breaking changes.