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 5 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
…bleDismissAutoFocus
| </SingleTabStopNavigationProvider> | ||
| {tooltipContent && ( | ||
| <div ref={tooltipWrapperRef}> | ||
| {/* [1] skips FocusLock's leading TabTrap to target the dismiss button. */} |
There was a problem hiding this comment.
This is a hack that we better avoid. Besides, we don't really need focus traps around the tooltip because tooltip itself has some. The only issue is that when the tooltip's first focus trap gets focused - the focus goes to the last element. I believe we can fix it in the tooltip itself - let me check it.
There was a problem hiding this comment.
This should address it: cloudscape-design/components#4518
| onMouseEnter={() => showTooltip(tooltipTarget.id)} | ||
| onMouseLeave={() => hideTooltip()} | ||
| onBlur={() => hideTooltip()} | ||
| onBlur={(event) => { |
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 invisible tab-trap elements 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}>elements.Two invisible focusable elements are rendered — one before and one after the tooltip (inside a wrapper div). The entry trap forwards focus into the tooltip when tabbing from the legend; the exit trap cycles focus back to the dismiss button when tabbing past the tooltip's last element. This replaces the need for a custom
onBlurfocus-return 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
Uses
disableDismissAutoFocus={true}(added in cloudscape-design/components#4510) to prevent the tooltip's built-in FocusLock from stealing focus from the legend item on mount.Tab traps use
getAllFocusables(tooltipRef.current)[1]rather than[0]because index 0 is FocusLock's internal leading TabTrap — focusing it would triggerfocusLastinstead of targeting the dismiss button.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.