Skip to content

fix(Banner): center a title-only banner whose only control is the collapse toggle - #5485

Closed
freddymeta wants to merge 1 commit into
mainfrom
fix/banner-single-line-toggle
Closed

fix(Banner): center a title-only banner whose only control is the collapse toggle#5485
freddymeta wants to merge 1 commit into
mainfrom
fix/banner-single-line-toggle

Conversation

@freddymeta

Copy link
Copy Markdown
Contributor

The bug

Banner centers its header vertically when the banner is one line of text with controls beside it — otherwise a 28px ghost button next to a 20px title reads as hanging. That is what headerCentered is for, and its comment says so: "When there's only a title (no description) and actions, center everything vertically."

But the test that drives it counts only two of the three things that can appear in the actions row:

const hasActions = isRenderable(endContent) || isDismissable;
const isSingleLine = !isRenderable(description) && hasActions;

The collapse toggle is missing. So two banners that render the same shape — a title, and one 28px icon-only ghost Button in the end area — align differently:

<Banner status="info" title="Deploy finished" isDismissable />   // centered
<Banner status="info" title="Deploy finished"><p>Details</p></Banner>  // top-aligned

In the second, the icon and title sit 4px above the toggle they are level with. showEndArea on the line above already counts hasToggle; only hasActions does not.

The fix

const hasActions = isRenderable(endContent) || isDismissable || hasToggle;

One term. hasToggle is already computed two lines up, and is itself isCollapsible && hasChildren, so a collapsible={false} banner and a banner with no children are untouched.

Verification

pnpm exec vitest run packages/core/src/Banner — 56 passed (52 existing + 4 added under a new single-line centering describe, covering all four combinations: dismiss-only, toggle-only, described-with-toggle, and no controls).

Negative control: reverting the one-term change and re-running fails exactly one test — "centers a title-only banner whose only control is the collapse toggle" — and the other 55 stay green. So the new test really is watching this behaviour, and the change moves nothing else.

Found while bringing an internal EPS theme onto the real Banner (context in #5417 / #5483): the theme had to reproduce this centering with its own CSS rule to match, which is how the asymmetry surfaced.

…lapse toggle

`isSingleLine` drives the header's vertical centering and counted `endContent`
and the dismiss button, but not the collapse toggle. A collapsible title-only
banner therefore kept `align-items: flex-start` while the same banner with a
dismiss button centered — its icon and title sat 4px above the 28px toggle
beside them. The toggle is the same control in the same row, so it counts.
@vercel

vercel Bot commented Aug 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview Aug 25, 2026 8:33am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 25, 2026
@github-actions github-actions Bot added community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge labels Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

Modified Components

Banner (@astryxdesign/core) · View in Storybook
Metric Before After Delta
Bundle Size (ESM) N/A N/A N/A
Lines of Code N/A 412 -
Complexity N/A Very High (43) -

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.8KB 1.2KB

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

github-actions Bot added a commit that referenced this pull request Aug 25, 2026
@freddymeta

Copy link
Copy Markdown
Contributor Author

Folding this into #5483 — it is the same file and the same read, and two PRs on one component in one afternoon is more review surface than the change deserves. The commit is unchanged there (fix(Banner): center a title-only banner whose only control is the collapse toggle), tests and negative control included, as the second of two commits. Happy to split it back out if you would rather review the fix on its own.

@freddymeta freddymeta closed this Aug 25, 2026
@github-actions
github-actions Bot deleted the fix/banner-single-line-toggle branch August 26, 2026 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. community Authored by a community contributor (not on the eng/design team) needs:code-review High-risk change (new package/component/API) — needs human code review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant