Skip to content

fix(banner): make hidden content inert and fix live region semantics - #5056

Open
lukemorawski wants to merge 8 commits into
callstack:mainfrom
lukemorawski:feat/banner-a11y-inert-when-hidden
Open

lukemorawski wants to merge 8 commits into
callstack:mainfrom
lukemorawski:feat/banner-a11y-inert-when-hidden

Conversation

@lukemorawski

@lukemorawski lukemorawski commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Banner was only visually hidden but content stayed mounted, screen readers could still reach it, and action buttons remained tabbable. The live region also had conflicting semantics and simply didn't work on Android.

  • Hidden content was still reachable. The banner was just translated off-screen and clipped - no aria-hidden, pointerEvents guard or inert. With visible={false}, both actions still had tabIndex: 0 and accepted focus in the browser a11y tree.
  • role="alert" + aria-live="polite" didn't agree. alert implies assertive + atomic, while Chrome ended up with alert atomic live="polite".
  • The live region was dead on Android. RN maps aria-live → accessibilityLiveRegion on View, not Text, so the prop was ignored by the native text node. In practice it only worked on web.
  • iOS has no live regions, so nothing announced the message there either.
  • actions was unbounded, and animation callbacks fired on mount and on theme.animation.scale changes even when visibility didn't change. Two existing tests already called this out as probably a bug.

Related issue

Fixes #5055
Part of #4990

Changes

Inertness. Content becomes aria-hidden + pointerEvents="none" + inert on web as soon as hiding starts, then unmounts after the exit animation - basically the same lifecycle Snackbar already uses. There's still one inert measuring pass when mounting hidden so the spacer gets the right height. Layout stays the same.

Live region. Moved from Text to a View, which makes it actually work on Android. It's also scoped to the message only, so action labels don't re-announce the whole banner. role and aria-live now match.

New urgent prop.

<Banner visible urgent actions={[...]}>
  Your payment failed.
</Banner>
  • false by default: role="status" + aria-live="polite"
  • true: role="alert" + aria-live="assertive"

On iOS the message is announced explicitly with announceForAccessibilityWithOptions({ queue: !urgent }): queued for normal banners, interrupting for urgent ones. This is iOS-only to avoid double announcements elsewhere. Interpolated children like <Banner>Hello {name}</Banner> are flattened before announcing.

Actions. Limited to 2, with a development warning for extras. They can now sit inline with the message when there's enough room instead of always dropping below it.

Focus. If a focused action disappears, focus moves to the nearest remaining action, or the message region if there are none. During hiding the content is inert, so focus is simply released. Restoring it to whatever opened the banner needs consumer-owned API/state and is out of scope here.

Callbacks. onShowAnimationFinished / onHideAnimationFinished now only run after actual visible transitions - not on mount, animation-scale changes, or interrupted animations.

Breaking changes

  • Banner children unmount once hidden, so local child state is lost.
  • More than 2 actions are ignored, with a dev warning.
  • Actions may render inline instead of always below the message.

Test plan

  • yarn typecheck
  • yarn lint
  • yarn test - 55 suites, 760 tests. Banner coverage went from 13 to 41 tests; the two "probably a bug" tests were inverted rather than removed.
  • Chrome browser accessibility tree (expo start --web)
  • Mobile Safari (iOS 18.3 simulator) + Chrome (Android 16 emulator)
  • Android native + TalkBack
  • iOS native + VoiceOver

Hidden state, browser a11y tree. Before, both actions were still focusable:

{"totalTabbablesOnPage":60,
 "bannerActionsStillTabbable":[
  {"label":"Set custom theme","tabIndex":0,"receivedFocus":true,"insideInert":false,"insideAriaHidden":false},
  {"label":"Fix it","tabIndex":0,"receivedFocus":true,"insideInert":false,"insideAriaHidden":false}]}

Afterwards the message and actions disappear from the tree completely. The page tab count drops by exactly those two buttons (from 31 to 29), then returns when the banner is shown again.

Visible state. alert atomic live="polite" becomes status atomic live="polite". The live region contains only the message; buttons are siblings.

Android / TalkBack. The native a11y tree contains banner-content, message and actions while visible, and none of them while hidden.

iOS / VoiceOver. Full show > hide > show cycle gives message + actions > [] > message + actions again. The urgent announcement was also checked.

@MikitasK MikitasK left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks pretty solid 👍
just a few points to address before merge:

Comment thread src/components/Banner.tsx
Comment thread src/components/Banner.tsx
Comment thread src/components/Banner.tsx Outdated
Comment thread src/components/Banner.tsx Outdated
Comment thread src/components/Banner.tsx Outdated
Comment thread src/components/Banner.tsx Outdated
Comment thread src/components/Banner.tsx Outdated
Comment thread src/components/Banner.tsx
Comment on lines +423 to +429
<View
testID={`${testID ?? 'banner'}-icon`}
style={styles.icon}
aria-hidden={!iconAccessibilityLabel}
accessible={iconAccessibilityLabel ? true : undefined}
aria-label={iconAccessibilityLabel}
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could we add role="img" when iconAccessibilityLabel is provided?

this View renders as a generic <div> on web
react native web doesn't implement accessible prop (source) and a generic element cannot have an accessible name. that's why wrapper’s aria-label isn't guaranteed to label nested image ((source)

Suggested change
<View
testID={`${testID ?? 'banner'}-icon`}
style={styles.icon}
aria-hidden={!iconAccessibilityLabel}
accessible={iconAccessibilityLabel ? true : undefined}
aria-label={iconAccessibilityLabel}
>
<View
testID={`${testID ?? 'banner'}-icon`}
style={styles.icon}
aria-hidden={!iconAccessibilityLabel}
accessible={iconAccessibilityLabel ? true : undefined}
aria-label={iconAccessibilityLabel}
role={iconAccessibilityLabel ? 'img' : undefined}
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

thanks for addressing 🙏

Comment thread src/components/Banner.tsx Outdated
maxFontSizeMultiplier={maxFontSizeMultiplier}
{exited ? null : (
<Animated.View
testID={`${testID ?? 'banner'}-content`}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only derive these when testID is set - the snapshots show banner-content, banner-row and banner-message on banners that were given no testID. Default test ids conflict with the user's own (#4998).

Comment thread src/components/Banner.tsx
const hasLiveRegion = () => Platform.OS === 'android' || Platform.OS === 'web';

// a nested <Text> would otherwise be dropped from the announcement
const extractText = (node: React.ReactNode): string =>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

child.props.children breaks on any child that doesn't render its own children, so <Banner><Message /></Banner> announces nothing on iOS. React.Children is the composition problem v6 is meant to remove (#4954) - what's the alternative here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yep, and it looks like it's not just iOS. Both the android and web live regions will go empty too. Checked it: zero announceForAccessibility calls on iOS, empty announcer on Android, visible text renders fine. dead silent on every platform. Thanks for bringing that up!

i see it liek that:

  • add messageAccessibilityLabel, matching iconAccessibilityLabel in this PR
  • keep extraction as the default so plain text keeps working
  • warn in dev when extraction is empty but children are not

React.Children stays as best effort with an opt out, not as main mechanism. but i'm happy to drop extraction entirely or park this for v6 instead. any preference?

Comment thread src/components/Banner.tsx Outdated
React.useEffect(() => {
const focused = focusedAction.current;

if (focused === null || focused < actionCount) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

focusedAction.current isn't cleared on hide - focused < actionCount returns first. Hide, then show with actions={[]}, and focus jumps to the message. The test that covers this keeps one action, so it short-circuits too.

Comment thread src/components/Banner.tsx Outdated
textColor={colors.primary}
theme={theme}
{...others}
touchableRef={mergeRefs(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

mergeRefs returns a new callback every render, so React detaches and reattaches each action ref on every commit — a consumer's touchableRef callback gets null then the node each time. Maybe try to memoize it.

@MikitasK MikitasK left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM 👌

Comment thread src/components/Banner.tsx
Comment on lines +423 to +429
<View
testID={`${testID ?? 'banner'}-icon`}
style={styles.icon}
aria-hidden={!iconAccessibilityLabel}
accessible={iconAccessibilityLabel ? true : undefined}
aria-label={iconAccessibilityLabel}
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

thanks for addressing 🙏

@satya164
satya164 requested a balanced review from Copilot September 15, 2026 20:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@satya164

Copy link
Copy Markdown
Member

Review by Claude

Web

A banner without actions loses its bottom spacing and the icon moves to the vertical center.

Steps: render <Banner visible icon={...}> with a message that wraps to 3 or more lines and no actions.

What happens: the text sits 8px above the bottom edge (it was 16px on the base branch), so the top spacing (24px) and the bottom spacing no longer match. The icon is centered on the text block instead of aligned with the first lines.

What should happen: the same spacing and icon position as before, since the PR description does not list this as a change.

Base branch:

Web, base branch: banner without actions

This PR:

Web, this PR: banner without actions

(via Claude, verify if it makes sense)

Android

The same spacing and icon change for a banner without actions.

Steps: the same as on web.

What happens: the last line of text is 8px from the bottom edge, and the icon is centered on the text.

What should happen: 16px bottom spacing, as on the base branch.

Android, this PR: banner without actions

(via Claude, verify if it makes sense)

@satya164 satya164 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review by Claude

  • extractText (line 38) still reads child.props.children through React.Children.toArray, and the open thread on it waits for a decision. The project rule is to avoid the Children API and element inspection, so the extraction should go. On Android and web the live region can render {children} itself instead of a string, so it announces whatever the children render, including <Message />. iOS needs a string for announceForAccessibilityWithOptions, so take it from an explicit prop there, such as the messageAccessibilityLabel proposed in the thread.
  • The branch conflicts with main in Banner.tsx, the example, the tests and the snapshot. main already moved the example banner below the app bar (#5061), so the rebase should drop the onLayout padding and the absolute position from the example. (via Claude, verify if it makes sense)

Comment thread src/components/Banner.tsx
</Text>
</View>
</View>
{visibleActions.length ? (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A banner without actions now ends 8px below the text instead of 16px. The empty actions row used to add 8px of margin, and nothing replaces it now. Multi-line messages also get the icon centered on the text because of alignItems: 'center' on body (line 537). Before, the icon sat at the top. Web and Android screenshots are in this comment.

Web, this PR: banner without actions

Add 8px of bottom margin to content when there are no actions, and keep the icon at the top unless the change is intended. The PR description does not list either change. (via Claude, verify if it makes sense)

Comment thread src/components/Banner.tsx
// and the tab order. native ignores the unknown prop
const inertProps: { inert?: boolean } = visible ? {} : { inert: true };

const stacked = !row.measured || row.width < COMPACT_BREAKPOINT;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On wide screens the first render always stacks the actions, because row is not measured yet. The next render moves them inline. On web at 1280px wide the banner is 126px tall for 2 frames and then shrinks to 72px, so the content below it jumps on every mount.

The comment on COMPACT_BREAKPOINT cites the md3 window size class, which uses the window width. useWindowDimensions gives that value in the first render and removes the useLayout state and the extra render:

Suggested change
const stacked = !row.measured || row.width < COMPACT_BREAKPOINT;
const { width: windowWidth } = useWindowDimensions();
const stacked = windowWidth < COMPACT_BREAKPOINT;

This also needs the useWindowDimensions import and drops onRowLayout. If the layout must follow the banner's own width, keep the row hidden until it is measured instead. (via Claude, verify if it makes sense)

Comment thread src/components/Banner.tsx
Comment on lines +396 to +397
const messageFocusProps: Pick<ViewProps, 'tabIndex' | 'accessible'> =
Platform.OS === 'web' ? { tabIndex: -1 } : { accessible: true };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On Android, accessible: true makes the message View focusable, so a hardware keyboard Tab stops on the message text, which is not interactive.

The focus move for a removed action also does not reach keyboard users on Android. Steps: give the banner 2 actions where the second one removes itself, Tab to it with a hardware keyboard and press Enter. Keyboard focus goes to the first focusable view on the screen (here, the message of another banner) instead of the remaining action. setAccessibilityFocus in focusNode moves screen reader focus, not keyboard focus.

Leave accessible unset on native, since the Text is already one accessibility element. Then either move keyboard focus on native too, or limit the focus handling to web, where .focus() works and findNodeHandle is not needed. (via Claude, verify if it makes sense)

Comment thread src/components/Banner.tsx
]}
>
<View
testID={subTestID('row')}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

content, row and announcer are internal wrappers, and their test IDs exist only so the tests can reach them. The reflow tests then assert flexDirection on banner-row, which is an internal style. The project test rules ask for public output instead: query the live region by role (status or alert), and the message and actions by text or their public IDs. (via Claude, verify if it makes sense)

Comment on lines +56 to +60
<FAB
icon="alert"
style={styles.urgentFab}
onPress={() => setUrgent(!urgent)}
/>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This FAB has no label, so TalkBack and VoiceOver read it as an unlabelled button. This toggle exists to test screen reader output, so give it a label that also says what it does:

Suggested change
<FAB
icon="alert"
style={styles.urgentFab}
onPress={() => setUrgent(!urgent)}
/>
<FAB
icon="alert"
aria-label={urgent ? 'Make banner not urgent' : 'Make banner urgent'}
style={styles.urgentFab}
onPress={() => setUrgent(!urgent)}
/>

(via Claude, verify if it makes sense)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(banner): make hidden content inert and fix live region semantics

5 participants