feat(badges): update to new SHINE visuals - part 2 - #2082
Conversation
🦋 Changeset detectedLatest commit: 400a890 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Structure looks great — we would be deprecating Icon and Number count sections so you can get rid of those. I'll add notes to the Mapping section in Figma for that. @giamir |
…e work on progress bars
giamir
changed the title
|
Code changes overall look good to me. Noticed a couple things though, let me know what you think:
|
Good catch I have updated that now.
This is an OS specific issue similar to what we had for the activity indicator component. I don't have that in macos so adding a negative margins will make it look odd in mac. I am honestly unsure how to mitigate that. Maybe for now we can merge it as is and we can have a follow up ticket to investigate further. We could probably play around with the line-height. I don't think using margins will cut it.
That's how the API currently works. |
mukunku
left a comment
mukunku
left a comment
There was a problem hiding this comment.
LGTM.
Regarding the negative margin for icons: since we have an upcoming change to adjust line heights probably best to leave it for later as you said 👍🏾
CGuindon
left a comment
CGuindon
left a comment
There was a problem hiding this comment.
- I think there are sizing tweaks missing for the
s-badge__squaredvariants forlgandsm. Looks like the square and icon don't change so the square is either too small or too big when the badge changes size.
- For the icon badges, we should have 4px padding between the icon and the text label and 6px on the right of the text (most icons don't go to the edge of the container so I reduced the space there to make it more visually balanced in most icon cases). Looks like it's 6px on both sides right now. (purple gutter and far right green padding side)
- For the Squared variant (not for
importantmodifyer, just regular), we should add an outline in HC mode around the square (the background colors because the same so we loose the box effect and they just look like non-squared badges).
- Feels weird that there's an empty option in the sizes (but maybe that's something we need to live with with storybook for the "default" size?
|
tagging @dancormier since Giarmir is out — found a few things above |
…xchange/Stacks into SPARK-113/badges-part-two
|
This is ready for re-review now 🙏🏾 |
There was a problem hiding this comment.
Changes look good, the only thing I'm finding struggling with is the order of the size examples for each type. I know our default size makes sense to put first but it looks really weird to have default, small then large in order and all the heights looks different. Can we order them to have small, default then large? Makes it easier to see the difference. (in Stacks Classic docs)
|
moot point about the sizes if they'll be removed from docs — but any areas where you keep all three, I would order them from smallest to largest size (not mixed) |
This reverts commit d3e3fc9.
5 participants
SPARK-113
This PR updates the remainder of the badges to the new SHINE visuals. It also clean up the related svelte component api.
Tip use the filter feature of GH to get a better diff experience
