| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
looks pretty solid 👍
just a few points to address before merge:
Sorry, something went wrong.
| }).start((result) => { | ||
| if (result.finished) { | ||
| setExited(true); | ||
| } | ||
| hideCallback(result); | ||
| }); |
There was a problem hiding this comment.
maybe we should invoke hideCallback only when result.finished === true ?
| }).start((result) => { | |
| if (result.finished) { | |
| setExited(true); | |
| } | |
| hideCallback(result); | |
| }); | |
| }).start((result) => { | |
| if (result.finished) { | |
| setExited(true); | |
| hideCallback(result); | |
| } | |
| }); |
Sorry, something went wrong.
| <View | ||
| ref={messageRef} | ||
| testID={`${testID ?? 'banner'}-message`} | ||
| style={styles.message} | ||
| role={urgent ? 'alert' : 'status'} | ||
| aria-live={visible ? liveRegion : 'off'} | ||
| > |
There was a problem hiding this comment.
could we make message focusable on web?
when last action is removed, we call .focus() on this View, but role="status" doesn't make rendered <div> focusable
maybe we can add tabIndex={-1} on web:
| <View | |
| ref={messageRef} | |
| testID={`${testID ?? 'banner'}-message`} | |
| style={styles.message} | |
| role={urgent ? 'alert' : 'status'} | |
| aria-live={visible ? liveRegion : 'off'} | |
| > | |
| <View | |
| ref={messageRef} | |
| testID={`${testID ?? 'banner'}-message`} | |
| style={styles.message} | |
| role={urgent ? 'alert' : 'status'} | |
| aria-live={visible ? liveRegion : 'off'} | |
| {...(Platform.OS === 'web' ? { tabIndex: -1 } : { accessible: true })} | |
| > |
also it'd be great to verify this behavior in Banner.text.tsx
Sorry, something went wrong.
| const message = React.Children.toArray(children) | ||
| .filter((child) => typeof child === 'string' || typeof child === 'number') | ||
| .join(''); |
There was a problem hiding this comment.
what about nested text elements? current code only collects direct string & number children, so messages like that would announce only "Payment " on iOS:
<Banner visible> Payment <Text>failed</Text> </Banner>
maybe we can recursively collect text from nested children? or provide explicit announcement label?
Sorry, something went wrong.
| textColor={colors.primary} | ||
| theme={theme} | ||
| {...others} | ||
| touchableRef={actionRefs.current[i]} |
There was a problem hiding this comment.
could we preserve touchableRef passed through an action? internal touchableRef currently overrides the consumer’s ref. maybe we can merge both refs instead:
| touchableRef={actionRefs.current[i]} | |
| touchableRef={mergeRefs(actionRefs.current[i], touchableRef)} |
Sorry, something went wrong.
| const actionRefs = React.useRef<Array<React.RefObject<View>>>([]); | ||
| for (let i = 0; i < MAX_ACTIONS; i++) { | ||
| // Button types touchableRef as non-nullable, but a ref always starts null | ||
| actionRefs.current[i] ??= React.createRef<View>() as React.RefObject<View>; |
There was a problem hiding this comment.
Let's try not to use as casts if possible
Sorry, something went wrong.
| } | ||
|
|
||
| // rnw resolves the ref to the dom node, which takes focus directly | ||
| (node as unknown as { focus?: () => void }).focus?.(); |
There was a problem hiding this comment.
Same here
Sorry, something went wrong.
| ); | ||
| // rnw forwards `inert` to the dom, which drops the subtree from the a11y | ||
| // tree and the tab order. native ignores the unknown prop | ||
| const inertProps = visible ? null : ({ inert: true } as object); |
There was a problem hiding this comment.
Same here
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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.
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.
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
Test plan
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.