feat(design-system): add Banner, and render WarningMessage with it - #8452
Draft
talissoncosta wants to merge 3 commits into
Draft
feat(design-system): add Banner, and render WarningMessage with it#8452talissoncosta wants to merge 3 commits into
talissoncosta wants to merge 3 commits into
Conversation
Five components render the same icon-plus-message shell by hand, each assembling Bootstrap alert classes and most reaching for inline styles. Banner is that shell: a tone, its icon, and children, coloured from surface/border/icon tokens rather than .alert-*, so it themes without the hand-written .dark overrides in _alert.scss. Only danger carries role='alert'. The rest are read in place, so announcing them would talk over whatever the user was doing. WarningMessage moves onto it first, being the simplest of the five, and loses the class-string juggling. OrganisationLimit stops passing enabledButton, which WarningMessage never read. ErrorMessage is left alone here: it is already being fixed on fix/error-message-announce, and should compose over Banner after that lands. InfoMessage and SuccessMessage need title, action and dismiss slots first, since they carry collapse state and close buttons. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
front-end
…ass props Both components now compose Banner instead of assembling alert classes by hand, which removes the inline display hack and the string juggling. errorMessageClass and warningMessageClass are gone. Neither was a class name: passing one replaced the component's default layout and flipped it to display:initial, which is why one caller had to pass flex-1 back and another passed a class that no stylesheet defines. Of the five call sites, four were setting an external margin, which now sits on the parent, and the fifth was buying the display side effect alone. errorStyles and enabledButton go too. Both had one caller each, and neither was doing anything a class could not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
feature
github-actions
Bot
added
feature
… ErrorMessage messages/ErrorMessage was a near-copy of the one beside it, differing only in import style, interface vs type, ?? vs ||, and a displayName. That drift is how enabledButton ended up alive in one copy and dead in the other. The file stays so existing imports keep working, but it now re-exports the real one rather than carrying a second implementation. SuccessMessage renders a Banner and keeps its bold title. Its url, buttonText, isClosable, close and infoMessageClass props had no callers at all, and its one successStyles caller was setting a margin the Banner does not have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
talissoncosta
force-pushed
the
feat/banner-component
branch
from
September 3, 2026 13:51
e1d7795 to
cd84a43
Compare
github-actions
Bot
added
feature
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Five components render the same icon-plus-message shell by hand:
ErrorMessage(twice, there are two copies),WarningMessage,InfoMessageandSuccessMessage. Each assembles Bootstrapalert alert-*classes and most reach for inline styles. A dozen more places write those classes directly.Banneris that shell: a tone, its icon, and children. It colours from--color-surface-*,--color-border-*and--color-icon-*rather than.alert-*, so it themes from tokens instead of the hand-written.darkblocks in_alert.scss.Only
dangergetsrole='alert'. The rest are read in place, so announcing them would talk over whatever the user is doing.WarningMessagemoves onto it first, being the simplest of the five.OrganisationLimitstops passingenabledButton, whichWarningMessagehas never read.Deliberately not here.
ErrorMessageis already being fixed onfix/error-message-announce(it addsrole='alert'and repairs seven call sites that pass the error as children to a component that only reads a prop, so the error renders as nothing). It should compose overBanneronce that lands.InfoMessageandSuccessMessageneedBannerto grow title, action and dismiss slots first, since they carry collapse state in localStorage and close buttons.How did you test this code?
Storybook,
Components/Banner: the four tones, one with an action, one with a wrapping body. Chromatic snapshots are enabled for this story.WarningMessage's callers are worth a look in the visual diff, since.alert-warningand.banner--warningdiffer slightly in radius and padding.