fix: return focus to legend item on Tab when tooltip is visible#217
fix: return focus to legend item on Tab when tooltip is visible#217albadra2 wants to merge 3 commits into
Conversation
…abStopNavigationProvider scope
|
|
||
| function onKeyDown(event: React.KeyboardEvent<HTMLElement>) { | ||
| if (event.keyCode === KeyCode.tab && !event.shiftKey && tooltipRef.current) { | ||
| if (focusFirstInteractiveInTooltip(tooltipRef.current)) { |
There was a problem hiding this comment.
Can we avoid capturing the tab keypress? This impl depends on the component types and might work inconsistently across browsers.
I recommend trying an alternative approach: when a legend item is focused (and also when hovered - why not? - we can generalise it for when it is highlighted and tooltip exists), we can render an invisible <div tabIndex={0} /> - to capture the Tab press and forward it to the tooltip. The tooltip content can be wrapped in a focus lock, that already support auto-focus. We can consider adding this component to the toolkit, or else an similar implementation can be used - utilising getAllFocusables to find the first focusable element in the tooltip, or maybe better just focusing the dismissButtonRef.current.focus() - provided the dismiss button is always there.
|
Good work, @albadra2 - the UX changes to the legend tooltip significantly improve UX and accessibility of the legend item tooltips! |
| onMouseLeave={() => hideTooltip()} | ||
| onBlur={(event) => { | ||
| const trigger = elementsByIdRef.current[tooltipTarget.id]; | ||
| if (focusStaysInTooltipScope(event.relatedTarget, tooltipRef.current, trigger)) { |
There was a problem hiding this comment.
Can we use exit tab trap instead? There is already a tab trap (an invisible element with tabIndex=0) before the tooltip - we can also add one after it, so that every time it gets focused - the focus is goes to back to the dismiss button.
| <div | ||
| ref={tabTrapRef} | ||
| tabIndex={0} | ||
| onFocus={() => tooltipRef.current?.querySelector<HTMLElement>("button")?.focus()} |
There was a problem hiding this comment.
Let's use getAllFocusables()[0]?.focus() instead - to make the code better general and better readable at the same time.
| ref={tabTrapRef} | ||
| tabIndex={0} | ||
| onFocus={() => tooltipRef.current?.querySelector<HTMLElement>("button")?.focus()} | ||
| style={{ position: "absolute", width: 0, height: 0, overflow: "hidden" }} |
|
|
||
| // Hide tooltip and clear highlight unless focus moves inside tooltip; | ||
| if (tooltipRef.current && event.relatedTarget && !tooltipRef.current.contains(event.relatedTarget)) { | ||
| // Hide tooltip and clear highlight unless focus moves inside tooltip or to the tab trap; |
There was a problem hiding this comment.
Can we wrap the tooltip and its tab traps in a div and check that div instead?
<div ref={tooltipWrapperRef}>
<div tabIndex={0} />
<Tooltip ... />
<div tabIndex={0} />
</div>
| // Remove this once InternalChartTooltip supports a `disableAutoFocus` prop. | ||
| useEffect(() => { | ||
| if (tooltipItemId) { | ||
| elementsByIdRef.current[tooltipItemId]?.focus({ preventScroll: true }); |
There was a problem hiding this comment.
This is shaky as the auto-focus can theoretically happen after we re-focus the item (e.g. in certain browsers). Also, some screen-readers might be quick enough to announce the dismiss button if it was focused for a split-second before the focus was restored. A proper change, as per the comments, is to add the ability to disable the built-in auto-focus. Let's do it.
There was a problem hiding this comment.
PR for the tooltip: cloudscape-design/components#4510
Description
When a chart legend tooltip is visible and the user presses Tab, focus falls to
<body>instead of returning to the legend item that triggered the tooltip. This is a WCAG 2.4.3 (Focus Order) violation.This revision implements the approach suggested in review: an unconditional dismiss button with an invisible tab-trap element to forward focus into the tooltip, rather than capturing Tab keypresses.
Changes
1. Unconditional dismiss button.
The tooltip now always renders a dismiss (X) button — both on hover and on keyboard focus. This engages the popover's built-in focus trap unconditionally, so Tab always cycles through the tooltip's interactive content. Activating the dismiss button (Enter/Space) closes the tooltip and returns focus to the legend item.
2. Invisible tab-trap
<div tabIndex={0}>.An invisible focusable element is rendered between the legend list and the tooltip. When the user Tabs past the legend items, the browser naturally lands on this element. Its
onFocushandler forwards focus to the dismiss button inside the tooltip. This avoids capturing Tab keypresses and works consistently across browsers.3. Tightened
<SingleTabStopNavigationProvider>scope.Previously,
<SingleTabStopNavigationProvider>wrapped both the legend item list AND the tooltip. Because Cloudscape'sButton,Link, and other interactive components consumeuseSingleTabStopNavigationfrom the context, every focusable element inside the tooltip receivedtabindex="-1"— making them keyboard-unreachable.The provider now wraps only the legend item list:
This aligns with the pattern used by Cloudscape's other navigable components — Tabs (
tab-header-bar.tsx), NavigableGroup, TreeView, and Table all scope<SingleTabStopNavigationProvider>to the navigable region only.Behavior
Implementation note
A focus-reclaim
useEffectre-focuses the legend trigger afterPopoverBodyauto-focuses the dismiss button on mount. This relies on React's child-before-parent effect ordering. It could be removed ifInternalChartTooltipsupported adisableAutoFocusprop in the future.Related links, issue #, if available: Blocking accessibility compliance reports (ACRs):
How has this been tested?
chart-components's own dev pages (pages/03-core/core-legend.page.tsx) usingnpm start.tabindex="-1"after provider restructure.Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md. ✓ Backward-compatible. Tooltip interactive content is now keyboard-reachable — a strict accessibility improvement.CONTRIBUTING.md. ✓ Uses existing toolkit utilities (KeyCode,SingleTabStopNavigationProvider).Security
checkSafeUrlfunction. N/A — no URL handling.Testing
chart-components'snpm startdev environment.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.