feat(progress-indicator): update to use carbon icons #5122
Conversation
ghost
requested review from
|
Deploy preview for the-carbon-components ready! Built with commit 0907c5a https://deploy-preview-5122--the-carbon-components.netlify.com |
|
Deploy preview for carbon-elements ready! Built with commit 0907c5a |
|
Deploy preview for carbon-components-react failed. Built with commit 0907c5a https://app.netlify.com/sites/carbon-components-react/deploys/5e304132797c22000ae909d4 |
| fill: $interactive-01; | ||
| margin-top: rem(9.5px); | ||
|
|
||
| path:last-of-type { |
There was a problem hiding this comment.
Just curious, what is this selector being used for? 👀
There was a problem hiding this comment.
Looks like it is being used to fill in the circle by increasing the width of the checked circle
There was a problem hiding this comment.
yeah...I couldn't find a Filled Circle Icon in the carbon/icon library...so I'm improvised 😅
should I try to get a filled circle icon in the icon package instead?
There was a problem hiding this comment.
should I try to get a filled circle icon in the icon package instead?
👍
There was a problem hiding this comment.
cc @laurenmrice do you know of an icon we could use or will this work as is? 👀
There was a problem hiding this comment.
Yeah we do not have a filled circle icon. I think we can do this for now and I can talk with conrad about possibly adding one to our library for the future. @joshblack
|
@annawen1 thank you for doing this! I might be wrong but it seems like the filled blue circe looks a tiny bit bigger than the one we have right now |
|
@abbeyhrt I think that's because the current one is an incorrect |
|
Wait a minute we do have a |
|
@tw15egan makes sense! |
@laurenmrice the dotmark icon doesn't take up the entire svg, there's some white space around it. Is there anyway to change this? |
|
@laurenmrice @annawen1 I also tried using that icon when I tested this PR, but yeah it is sized smaller than the other 16px icons (makes sense, since it is a dot mark). We would need a full 16px circle |
|
Ahh okay I see. Alright I will still talk to Conrad about this to see the reason behind the dot mark size and if he can adjust that one to match the other circular icons, or if we need to make a separate icon. |
|
Update on filled icon for current state: Conrad is going to make a |
|
@laurenmrice awesome! |
|
@laurenmrice Does this mean this PR itself is good to go...? Thanks! |
|
@laurenmrice is this good to merge? |
…wen1/carbon into feat/progressive-icon-props
|
I removed the margin-top that was being set on the completed icon, looks closer in size now |
tw15egan
added
status: ready to merge 🎉
and removed
status: visual review 🔍
status: waiting for reviewer's response 💬
labels
Closes #5086
Updates the
<ProgressIndicator />to pull icons from the carbon icons package instead of hardcoding the svg paths. Used the<RadioButton16 />and<RadioButtonChecked16 />icons - I didn't notice any filled circle icons so I had to style the<RadioButtonChecked16 />stroke.Changelog
New
<RadioButtonChecked16 />for current progress step icon and<RadioButton16 />for complete and disabled progress step iconsChanged
<RadioButtonChecked16 />to produce the filled circle we need for complete-progress-step iconTesting / Reviewing
Testing should ensure
<ProgressIndicator />is consistent with designs