Skip to content

fix(Banner): stack inline actions vertically on narrow viewports #7625

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
liuliu-dev merged 7 commits into from
Mar 11, 2026
+20 −0
Merged

fix(Banner): stack inline actions vertically on narrow viewports #7625

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
234434d
column-reverse on narrow
liuliu-dev Mar 4, 2026
047bfd6
chore: auto-fix lint and formatting issues
liuliu-dev Mar 4, 2026
cc204e5
column
liuliu-dev Mar 5, 2026
4754902
stack on narrow, add vrt
liuliu-dev Mar 10, 2026
4d61ae7
Merge branch 'main' into liuliu/fix-banner-stack-actions-on-narrow-vi…
liuliu-dev Mar 10, 2026
1605626
test(vrt): update snapshots
liuliu-dev Mar 10, 2026
c614cbb
revert unrelated ActionList snapshot change
liuliu-dev Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/gold-rooms-bake.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Banner: stack inline actions vertically on narrow viewports.
4 changes: 4 additions & 0 deletions packages/react/src/Banner/Banner.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@

& .BannerActions :where([data-primary-action='trailing']) {
display: flex;

@media screen and (--viewportRange-narrow) {
flex-direction: column-reverse;
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
flex-direction: column-reverse;
flex-direction: column;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@francinelucca francinelucca Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested and this seems to be correct 👀 let's address it

Copy link
Copy Markdown
Contributor Author

@liuliu-dev liuliu-dev Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch on keyboard focus @copilot!

I initially used column-reverse to put the primary button on top, this is how it looks after switched to column to keep visual order aligned with focus order:

Screenshot 2026-03-05 at 1 04 12 PM

Curious what you think about this @lukasoppermann 👀

Copy link
Copy Markdown
Contributor

@lukasoppermann lukasoppermann Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was actually that on small view ports the buttons are below:
CleanShot 2026-03-06 at 10 20 11@2x

Wonder why this is not working. Would it be possible to fix it like that? The text is very narrow if it is in two columns.

Copy link
Copy Markdown
Contributor Author

@liuliu-dev liuliu-dev Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue is specifically from the actionsLayout="inline" story, which overrides the default responsive behavior. From the docs:

Screenshot 2026-03-06 at 8 38 22 AM

When actionsLayout is inline, the container has nowrap which prevents wrapping at all viewport sizes. Should we allow inline to fall back to stacked at narrow viewports? or it should always stay inline?

Copy link
Copy Markdown
Contributor Author

@liuliu-dev liuliu-dev Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed with @lukasoppermann that we will stack on narrow view port:

inline is meant for the non-mobile view, where adding the x normally moves the buttons below, which can be bad if all you have is a very short title.

So for mobile, lets fall back to stacked.

Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
flex-direction: column-reverse;
flex-direction: column-reverse;
row-gap: var(--base-size-2);

Copilot uses AI. Check for mistakes.
}
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}

& .BannerActions :where([data-primary-action='leading']) {
Expand Down
Loading