feat(Banner): theme target for the description

[freddymeta]

Follows #5417 — same problem, one more component.

What is unreachable today

Banner exposes three theme targets — astryx-banner (the tinted header), astryx-banner-icon, and astryx-banner-content. The header's supporting line renders with bare stylex.props, so a theme that wants to restyle it has to reach in with a structural selector:

.astryx-banner > div:nth-child(2) > div:nth-child(2) { … }

That says "the second div inside the second div", which is a guess about internals — it keeps parsing and silently stops matching the day the DOM changes.

The change

One target: astryx-banner-description.

It names a painted element with a demonstrated use. Astryx paints the description --color-text-secondary; a downstream theme of ours wants it primary, and today the only way there is re-scoping --color-text-secondary across the whole header, which repaints everything else in there that reads the token. It also owns the 2px gap under the title — the header's text column is gap: 0, and banner's rowGap is the wrap gap, not this one.

Purely additive: no existing class, data attribute or style changes, and the render is byte-identical.

What came out, and why

Two things from review, both correct:

banner-actions is gone. It named the end area, which is a layout row — flex, wrap, margin-inline-start: auto, and the negative block margin that lets its buttons overhang the header padding. Not a painted surface, so it fails the theme-target rule the same way a display: contents wrapper does. The use it was meant to serve — a header that grows around its buttons rather than letting them overhang — is padding-block on the existing banner target, which reaches the same 44→52px without exposing a private margin as contract.

The centring commit is reverted, and @cixzhang was right that it changed nothing. I checked before deleting it. Measured in Chromium, forcing both alignments on the same DOM:

                    flex-start (main)   center (my commit)
header height       44                  44
title column mid    38                  38
toggle mid          38                  38
DELTA               0                   0

The arithmetic: the title's line box is 14px × 1.4286 = 20px, and the toggle is a 28px control carrying marginBlock: -4px — so its outer height is 28 − 8 = 20px too. Both items are 20px, the row is 20px, and align-items has nothing to resolve. That negative margin exists precisely to cancel the 4px I claimed was a bug: I reasoned from the button's 28px and missed the margin two lines above it in the same stylesheet.

It would only ever have fired once a theme overrode that margin — which is to say, only through banner-actions. Removing one removed the other's reason to exist.

Verified the alternative too: padding-block 12px → 16px on banner takes a title-only header 44 → 52px with the title and control centres still equal at 0px offset.

Nothing else in the header becomes a target

The title, the two controls and the text column are deliberately left alone: the column paints nothing (display: flex; flex-direction: column; gap: 0) and the space it owns is expressible on banner-description, while the title and the controls already render the way the consuming theme wants them. The changeset says so, so the next person does not re-propose the row.

Verification

• pnpm exec vitest run packages/core/src/Banner — 52 pass.
• tsc --noEmit clean, lint:strict 0 errors in the touched files.
• Chromium measurements above, against a real Storybook build.

[vercel]

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

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

[github-actions]

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	427	-
Complexity	N/A	Very High (44)	-

Bundle Size Summary

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

Accessibility Audit

Status: No accessibility violations detected.

Visual Regression

268 added · 0 removed. View the report

To accept these exact frames: /accept-visual 33371804626/1 <reason>

Added — After

Added — After

Added — After

---

_{Generated by PR Enrichment workflow | Storybook | Sandbox | View full report}

[cixzhang]

Thanks for narrowing this. banner-description now names a painted element with a demonstrated use, but banner-actions still exposes the flex/wrap/margin/edge-compensation row rather than a visual surface.

In Chromium, the centering commit changes zero pixels under shipped styles in light, dark, RTL, and forced colors; it only changes pixels after that private row margin is overridden. Setting padding-block on the existing banner target gets the same 44→52px result and keeps the title/control centers equal without exposing the row.

Could we keep banner-description and drop banner-actions plus the centering commit?

[Reviewed by Robohands]

[freddymeta]

Title and body refreshed — thanks for catching that; both still described the two-target, two-commit version that no longer exists.

Title is now feat(Banner): theme target for the description (singular), and the body covers what actually ships: one target, why banner-actions came out, and the Chromium measurement showing the centring commit was inert — including the arithmetic I got wrong the first time (the toggle's marginBlock: -4px already cancels the 4px I claimed to be fixing).

[freddymeta]

Merged latest main (17 commits behind) — which also regenerates the visual evidence, since the report from 27 Aug had expired to a 404 and visual-acceptance was the only thing left blocking this.

Component suite re-run green on the merge. No source change in this push.

[freddymeta]

/accept-visual

[github-actions]

Visual acceptance refused: copy the exact /accept-visual <run>/<attempt> <reason> command from the report.

[cixzhang]

The target itself is well placed on documented, painted anatomy, and unthemed pixels remain unchanged. One mechanical requirement is missing: visual:probe-theme:check fails because generated probe coverage does not include banner-description.

Please regenerate and commit the required probe output so the visual gate exercises the new public target.

[Reviewed by Robohands]

[freddymeta]

Regenerated and pushed in 2777396cb (on a merge of current main). pnpm visual:probe-theme:check is green: 272 targets, 881 selectors, and banner-description is now covered.

The generated file carries more than my target, and I want to be explicit about which is which. The generator emits probeTheme.ts wholesale, so regenerating also picks up coverage main is already missing:

Added entry	Owner
banner-description	this PR
carousel-scroller + presentation:popover / presentation:bottom-sheet / presentation:adaptive	#5601
dropdown-menu-divider → size:xl	main

I could not commit only my own entry — the file is @generated and the check regenerates it whole — so those five ride along. Clean origin/main fails this same check today, at fbfe729d1:

$ git worktree add /tmp/mainprobe origin/main && cd /tmp/mainprobe && pnpm visual:probe-theme:check
::error::The probe theme is out of date — a theming target changed without regenerating it.

The reason it drifted is that no workflow runs this guard. visual:probe-theme:check appears only in package.json; ci.yml never invokes it, and check:repo in lint.yml does not include it. So the check is only ever run by hand — as you did on this PR — and a target added without regenerating stays uncovered until someone else's PR happens to regenerate. Happy to wire it into check:repo as a separate one-line PR if you want it enforced; I have left it out of this one.

Verification

• pnpm visual:probe-theme:check — ✅ 272 targets, 881 selectors
• Banner suite 56/56; packages/core/src/theme/ + .github/scripts/visual-gate/ 36 files / 1077 tests, all pass
• Negative control: deleting the astryx-banner-description line from Banner.doc.mjs makes the check fail again, so the coverage is genuinely tied to the doc entry rather than passing by construction

[cixzhang]

Thanks, this addresses my previous review: probe coverage now includes banner-description. Please add a direct Banner test pinning that target before merge; no further review needed.

[Reviewed by Robohands]