fix(Banner): stack inline actions vertically on narrow viewports#7625
fix(Banner): stack inline actions vertically on narrow viewports#7625liuliu-dev wants to merge 2 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 047bfd6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Uh oh! @liuliu-dev, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:
Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.
|
github-actions
bot
added
the
integration-tests: recommended
|
|
🤖 Lint issues have been automatically fixed and committed to this PR. |
|
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/15162 |
There was a problem hiding this comment.
Pull request overview
Adjusts @primer/react’s Banner styling to prevent inline action buttons from overlapping at narrow viewport widths, addressing an accessibility-audits issue.
Changes:
- Add a narrow-viewport media rule for inline (
actionsLayout="inline") trailing actions to stack vertically. - Add a patch changeset entry for the Banner fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/react/src/Banner/Banner.module.css | Changes inline trailing actions layout at narrow viewports to avoid button overlap. |
| .changeset/gold-rooms-bake.md | Declares a patch release for the Banner layout adjustment. |
| display: flex; | ||
|
|
||
| @media screen and (--viewportRange-narrow) { | ||
| flex-direction: column-reverse; |
There was a problem hiding this comment.
Using flex-direction: column-reverse will make the visual order of the actions differ from DOM/focus order (Banner renders secondaryAction before primaryAction in the trailing container). On narrow viewports this can cause keyboard focus to move bottom→top, which is an accessibility issue. Prefer flex-direction: column (to keep visual order aligned with focus order), or otherwise change the rendered DOM order for this breakpoint so visual and focus order match.
| flex-direction: column-reverse; | |
| flex-direction: column; |
There was a problem hiding this comment.
tested and this seems to be correct 👀 let's address it
| display: flex; | ||
|
|
||
| @media screen and (--viewportRange-narrow) { | ||
| flex-direction: column-reverse; |
There was a problem hiding this comment.
When switching this container to a column layout, the existing spacing defined on .BannerActionsContainer via column-gap won’t add vertical spacing between stacked buttons. Consider adding a row-gap/gap override for the narrow breakpoint (or switching the base container to gap) so the vertically stacked actions don’t touch.
| flex-direction: column-reverse; | |
| flex-direction: column-reverse; | |
| row-gap: var(--base-size-2); |
| @media screen and (--viewportRange-narrow) { | ||
| flex-direction: column-reverse; | ||
| } |
There was a problem hiding this comment.
This change fixes a narrow-viewport layout issue, but the existing Banner stories don’t currently cover actionsLayout="inline" at ~320px with both primary and secondary actions (the mobile example only shows a primary action). Adding/updating a Storybook case (and corresponding VRT/AVT coverage if applicable) for inline+two-actions at narrow widths would help prevent regressions.
francinelucca
left a comment
francinelucca
left a comment
There was a problem hiding this comment.
Lets address https://github.com/primer/react/pull/7625/changes#r2891377700
Would also be nice to add VRT on narrow for this if possible 👀
Aaaand can we get design to sign off on the new narrow visuals? 🙏🏽
Addresses issue #14971
Changelog
Fixes an overlap issue in
BannerwhenactionsLayout="inline"is used with both a primary and secondary action at narrow viewport (320×256px).Solution: At narrow viewports (
--viewportRange-narrow), the trailing actions container switches toflex-direction: column-reverse, stacking the primary and secondary action buttons vertically.Before:
After:
Rollout strategy
Testing & Reviewing
Merge checklist