primer
Refactor Flash alert
What are you trying to accomplish?
This PR adds a new Flash component, meant to eventually replace our existing flash alert. This update aims to fix long standing issues found in it, mainly regarding responsive support.
Main updates:
Improved alignment and spacing
Responsive support
A lot of the spacing values used in this refactor are hardcoded, and will benefit from the Primer Primitives spacing update once that's available. (link only available for Hubbers).
What approach did you choose and why?
I've kept the current flash alert in place, and created a new flash-refactor.scss file, in a similar fashion to how @langermank has considered the refactor for the Button component (#1874).
After all instances of flash are replaced with this new component (specially after updating its ViewComponent, we can clean up these files.
Are additional changes needed?
As described in https://github.com/github/primer/issues/62 (internal to hubbers), some alerts may have multiple buttons. This refactor hasn't addressed for these use cases specifically.
- [ ] No, this PR should be ok to ship as is. 🚢
Closes https://github.com/github/primer/issues/154.
🦋 Changeset detected
Latest commit: 2b59674ffc25abdc082e031719cf015fe1ce0d88
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @primer/css | Minor |
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
![changeset-bot[bot] avatar](https://avatars.githubusercontent.com/in/28616?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'm moving this back to a draft to update the proposal with full support for accessibility and better naming. Thank you @alliethu and @ashygee 🎉 !
@vdepizzol after our conversation we had discussed that the full version of the Banner (Flash) component would be what we currently style as .flash-banner, see screenshot below. I did not see this reflected in Storybook as it still has the border at the top and bottom of the component. Are we still considering making this change?
@ashygee:
after our conversation we had discussed that the full version of the Banner (Flash) component would be what we currently style as
.flash-banner, see screenshot below. I did not see this reflected in Storybook as it still has the border at the top and bottom of the component. Are we still considering making this change?
So, I'm no longer including a -banner variant in the component since we'll no longer offer support for sticky banners. From the Primer CSS docs:
A flash message that is fixed positioned at the top of the page. Use for more global events where the content of the page is unknown.
Eventually a system tray should better support system banners. See https://github.com/primer/css/pull/1898.
In the meantime, a -full variant should still work inside bordered Box elements:
:v: :v: :v:
@langermank @simurai @ashygee opening this again for review!
I'm not addressing WAI-ARIA requirements at this time, I believe a full audit and decisions can happen once the PVC component is updated.
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
👋 @vdepizzol How do you feel about merging this component and start testing it out in production?
How do you feel about merging this component and start testing it out in production?
Is there already a need for this in production?
I think the main reason this hasn't shipped is that there is no plan for the PVC implementation yet. And we want to avoid using it with custom markup/HTML. So it would just be un-used CSS for now.
Is there already a need for this in production?
No rush, curious because some of the problems identified in this thread were reported again when I was first responder
Is there already a need for this in production?
No rush, curious because some of the problems identified in this thread were reported again when I was first responder
FWIW, it's been shipped in Figma.
Hey everyone! The PVC team has picked this up, see: https://github.com/primer/view_components/pull/1494
Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.