feat(banner): update styles for SHINE #2118
Conversation
🦋 Changeset detectedLatest commit: 6c8d4ad 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. |
mukunku
added
the
do-not-merge
|
This should be ready for review @CGuindon ! Let me know what you think 🙏🏾 I also saw your comment about maybe removing the pinned option. Didn't know if we should do that so I kept it for now. |
CGuindon
left a comment
CGuindon
left a comment
There was a problem hiding this comment.
When I fixed the ellipsis icon to be the Cross, I noticed it had also reverted to it's default #000 everywhere. I fixed this morning as well in the Figma, for the Important variants, the Cross/X icon should be white like the text (to pass a11y) — except for yellow/warning.
I think that was an oversight by me. It should be fixed now @CGuindon 👍🏾 |
| } | ||
|
|
||
| :has(>button&--dismiss) { | ||
| &--actions { |
There was a problem hiding this comment.
I created s-notice--actions and s-banner--actions to define this container. It was getting a bit unruly with the selector we had before.
dancormier
left a comment
dancormier
left a comment
There was a problem hiding this comment.
Thanks for these changes @mukunku. The styling seems good and I only found one very minor issue with the docs. I have a couple of other notes.
A few suggested changes
- I'd recommend adding the new variants to the visual/a11y test files
- I think it mainly be updating the array of
variantsin those test files - We should probably add them for Notice/Toast as well
- I think it mainly be updating the array of
- I think this would be a good time to add a
BannerSvelte component- I would expect this to be pretty trivial since it's really similar to
Notice - I wonder if it would be easiest to make a private component that accepts a class name that we could just import into
Banner.svelte,Notice.svelte, and maybeToast.svelte
- I would expect this to be pretty trivial since it's really similar to
Let me know if you'd like any help or clarification. Thanks again 🫶
| {% highlight html %} | ||
| <div class="s-banner s-banner__featured" role="alert" aria-hidden="false">…</div> | ||
| <div class="s-banner s-banner__featured s-banner__important" role="alert" aria-hidden="false">…</div> | ||
| {% endhighlight %} |
There was a problem hiding this comment.
Very minor issue: There's a few of these highlight sections that are indented but shouldn't which results in weirdly offset code.
Screenshot
Summary
This PR Updates the Banner component styles to match the latest SHINE designs: https://www.figma.com/design/do4Ug0Yws8xCfRjHe9cJfZ/Project-SHINE---Product-UI?node-id=5982-7648&m=dev
This PR should be merged after #2109
How to Test
Check out the Banner component: https://deploy-preview-2118--stacks.netlify.app/product/components/banners/
There is no Svelte component for banners.