Repository navigation
CORE-2949: Add BannerRegion, a Banner that announces itself - #160
Merged
Merged
Conversation
RoyEJohnson
force-pushed
the
CORE-2949-banner-region
branch
from
October 6, 2026 20:22
8908864 to
4ee7cc5
Compare
RoyEJohnson
force-pushed
the
CORE-2949-banner-region
branch
from
October 6, 2026 21:25
56d2ec6 to
80166e4
Compare
RoyEJohnson
marked this pull request as ready for review
October 6, 2026 21:59
Banner renders a plain div with no role or aria-live, so any caller that
wants one announced has to build its own live region and know that the
region must already exist before the banner appears. Exactly one call site
in the org does that today.
The role cannot go on Banner itself: every call site renders it as
{messages.length ? <Banner/> : null}, so the region and its content would
arrive in the same tick, which AT reads as initial state rather than a
change. BannerRegion moves the conditional inside the library instead,
mirroring ToastContainer, where the container is the live region and the
individual toasts render into it.
Plain Banner is unchanged and stays the right choice for banners that are
page furniture rather than events.
https://openstax.atlassian.net/browse/CORE-2949
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The spread was handing className to Banner as well, where it goes nowhere — Banner does not forward it to StyledBanner. Destructure it out so the prop lands only on the element it is for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…docs and stories The Banner stories show errors and warnings, which read as results of async work, but each is static and already on the page when it loads. Nothing said so, and nothing pointed from Banner to BannerRegion. The rule is now stated the same way in both doc comments: use BannerRegion when something the user or the app did causes the banner to appear or change, such as a failed request, and plain Banner for one that is already on the page when it loads. The Banner stories carry captions saying they are static, and the Error story says that an error like it normally comes from a failed request and belongs in a BannerRegion. The BannerRegion stories replace the blank Empty story with ComparedWithBanner, where the same message appears in a plain Banner and in a BannerRegion side by side, so the difference can be checked with a screen reader. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
BannerRegion always renders its wrapper, so that the live region is already in the page when its content arrives. But an empty div is still a flex or grid item: in a flex row with a 2rem gap the empty region made the gap 4rem, and in a grid it took a cell and pushed the next item to a new row. While it is empty, the wrapper is now position: absolute. That removes it from layout but not from the accessibility tree, where Chrome still reports it as a polite, atomic status region before and after content arrives. display: contents fixes the layout too, but has a history of dropping elements from the accessibility tree in Safari, which is where this component matters most. With a banner in it the wrapper is an ordinary block, and className styles it. The rule keys off :empty, so a test checks that the wrapper has no children until there is something to announce. A story shows the region in a flex row with a gap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… fix a phrase A BannerRegion mounted with messages already in it puts the region and its content into the page together, which assistive technology reads as initial state rather than a change. That is deliberate: messages present when the page loads are page content, read in order, and announcing them on every load would repeat a message the user did nothing to cause. The doc comment now says so, and says to keep the region mounted rather than rendering it only when there is something to show. A test pins it: the banner must be in the first render's output, so a server render, which runs no effects, contains it. Deferring the first render through an effect would fail it. Also "because something the user or the app did" is missing a word, in the Banner doc comment and two story captions: "because of something". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RoyEJohnson
force-pushed
the
CORE-2949-banner-region
branch
from
October 8, 2026 19:55
cc5e195 to
19c489d
Compare
P-Gill97
approved these changes
Oct 9, 2026
This branch was successfully deployed
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
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.
Jira: CORE-2949 — follows up on CORE-2947, where the question came up of whether the live region should be built into
Banneritself.What this is
Bannerrenders a plain styled<div>with noroleoraria-live. A caller that wants a banner announced has to build its own live region and know a non-obvious rule to get it right: the region has to already be in the accessibility tree before the banner appears. Today exactly one call site in the org does that — the wrapper #533 just added in assignments. This moves that wrapper into the library so the constraint is written down where the next person will look for it.Why not
aria-liveonBannerEvery call site uses the shape
{messages.length ? <Banner …/> : null}. IfBannercarried the role itself, the region and its content would enter the DOM in the same tick and AT would read that as initial state rather than a change — Chrome + NVDA usually announces it anyway, Safari + VoiceOver reliably does not. It would look right in review, pass snapshot tests, and silently fail for a subset of users, while removing the incentive for callers to wrap it properly.So the conditional moves inside the library, behind a container that is always rendered. This mirrors
ToastContainer(src/components/ToastContainer.tsx:37), where the container is the live region and the individual toasts render into it — Banner was the only announcement-shaped component here without a container equivalent.Deliberate choices
role="status"for every severity. The obvious mapping —error→role="alert"— would make a banner rendered on page load interrupt whatever the user was doing. Severity describes how loud the banner looks, not how urgently it needs to reach someone. If a caller turns up that genuinely needs assertive, add an explicit opt-in prop then.Banneris unchanged apart from a doc comment, and stays the right choice for a banner that is already on the page when it loads. UseBannerRegionwhen something the user or the app did causes the banner to appear or change, such as a failed request. Steady-state information inside a live region announces itself again on every remount, for a change the user never made.messages, as the doc comment says. A test pins that the banner is in the first render's output rather than added after an effect.gapto 40px in Chrome and took a grid cell. So while it is empty (:empty) it isposition: absolute, which takes it out of layout but not out of the accessibility tree.display: contentsalso fixes the layout, but has a history of dropping elements from the accessibility tree in Safari, which is where this component matters most. With a banner in it the wrapper is an ordinary block, andclassNamestyles it.Tests
The Banner specs are snapshot-based, but a snapshot can't express the property that matters. The spec asserts the region exists while there is nothing to announce, and that the element holding the banner afterwards is the same element — the case AT misses is a region that arrives together with its content. A test checks the wrapper is
:emptyuntil there is something to announce, since the layout rule depends on it; a story,InAFlexContainer, shows the region in a flex row with a gap. The stories are there for checking with a real screen reader.ComparedWithBannershows the same message appearing in a plainBannerand in aBannerRegionside by side. The existingBannerstories are static, so each banner is already on the page when it loads; they now carry captions saying so, and theErrorstory notes that an error like it normally comes from a failed request and belongs in aBannerRegion.Draft, because
The version bump and
./scripts/publish.bashare a human step, and nothing consumes this yet. Once it's released, CORE-2949 covers replacing the hand-written wrapper in assignments'SelectScopeContentwith it and auditing the remaining call sites.🤖 Generated with Claude Code