fix(a11y): WCAG 1.4.13 — make hover content dismissable and hoverable#39239
fix(a11y): WCAG 1.4.13 — make hover content dismissable and hoverable#39239Aitema-gmbh wants to merge 2 commits intoapache:masterfrom
Conversation
- Add Escape key handler to TaskStackTracePopover (useEffect + keydown) - Add Escape key handler to TaskPayloadPopover (useEffect + keydown) - Add Escape key handler to AG Grid CustomPopover (keydown in existing useEffect) - Fix DeckGL Tooltip: pointerEvents none->auto, add Escape dismiss with state reset Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review Agent Run #1b45a3Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
dosubot
bot
added
change:frontend
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| // Reset dismissed state when tooltip content changes (new hover target) | ||
| useEffect(() => { | ||
| setDismissed(false); | ||
| }, [tooltip?.x, tooltip?.y]); |
There was a problem hiding this comment.
Suggestion: The dismiss state is reset on every mouse-coordinate update, so pressing Escape can be immediately undone as soon as the cursor moves by a pixel while still hovering. This breaks the dismiss behavior by making the tooltip reappear without ending the hover interaction. Reset the dismissed state only after hover ends (when tooltip becomes null/undefined), so Escape dismissal remains effective until the user leaves and re-enters. [logic error]
Severity Level: Major ⚠️
- ❌ Deck.gl map tooltips reappear after Escape while hovering.
- ⚠️ Breaks WCAG 1.4.13 hover-content dismissal for deck.gl.| // Reset dismissed state when tooltip content changes (new hover target) | |
| useEffect(() => { | |
| setDismissed(false); | |
| }, [tooltip?.x, tooltip?.y]); | |
| // Reset dismissed state only after hover ends, so Escape dismissal persists while hovered | |
| useEffect(() => { | |
| if (!tooltip) { | |
| setDismissed(false); | |
| } | |
| }, [tooltip]); |
Steps of Reproduction ✅
1. Open any deck.gl visualization in Superset that uses `DeckGLContainer` (e.g., a
Screengrid chart; container implementation at
`superset-frontend/plugins/preset-chart-deckgl/src/DeckGLContainer.tsx:62-187`), so that
map interactions and tooltips are provided by `DeckGLContainer`.
2. Hover over a rendered data point so that the layer `onHover` handler calls `setTooltip`
with the mouse coordinates, for example in Screengrid: `Screengrid.tsx:170-23` sets
`setTooltip({ content: tooltipContent(enhancedInfo), x: info.x, y: info.y });`, and
`DeckGLContainer` stores this in its `tooltip` state (`DeckGLContainer.tsx:62-66`).
3. Observe the tooltip rendered via `renderTooltip` in `DeckGLContainer`
(`DeckGLContainer.tsx:113-121`), which mounts the `Tooltip` component with the current
`tooltip` prop; `Tooltip` manages a local `dismissed` state and registers an Escape key
handler to set `dismissed` to `true` when `e.key === 'Escape'` (`Tooltip.tsx:69-85`).
4. Press Escape to dismiss the tooltip (setting `dismissed` to `true` in
`Tooltip.tsx:78-80` so `Tooltip` returns `null` at `Tooltip.tsx:87-89`), then move the
mouse slightly while still over the same hovered geometry: the layer `onHover` fires again
and updates `tooltip.x`/`tooltip.y`, which causes the `useEffect` at `Tooltip.tsx:71-74`
(dependent on `[tooltip?.x, tooltip?.y]`) to reset `dismissed` back to `false`, making the
tooltip reappear immediately despite the Escape dismissal having been requested.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx
**Line:** 71:74
**Comment:**
*Logic Error: The dismiss state is reset on every mouse-coordinate update, so pressing Escape can be immediately undone as soon as the cursor moves by a pixel while still hovering. This breaks the dismiss behavior by making the tooltip reappear without ending the hover interaction. Reset the dismissed state only after hover ends (when tooltip becomes null/undefined), so Escape dismissal remains effective until the user leaves and re-enters.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
| // Reset dismissed state when tooltip content changes (new hover target) | ||
| useEffect(() => { | ||
| setDismissed(false); | ||
| }, [tooltip?.x, tooltip?.y]); |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
The Escape-dismiss state is reset purely when tooltip?.x/tooltip?.y change, so after pressing Escape, any small mouse movement that keeps the pointer over the same DeckGL pickable object causes the tooltip coordinates to update, which immediately clears dismissed and makes the tooltip reappear. As a result, Escape does not provide a stable dismissal for the current hover session.
Suggestion: Reset the dismissed state based on the tooltip lifecycle (e.g., when tooltip is cleared to null or when a stable identity/content key changes) rather than raw cursor coordinates, so that pressing Escape keeps the current tooltip hidden until the user actually leaves and re-enters its trigger.
Code Review Agent Run #810535Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
1 participant
SUMMARY
Implements WCAG 2.1 criterion 1.4.13 (Content on Hover or Focus, Level AA).
enterable: trueso they can be hoveredTESTING INSTRUCTIONS
ADDITIONAL INFORMATION
Part of a series of 16 individual WCAG 2.1 accessibility PRs. See also fix(a11y): Accessibility improvements for WCAG 2.1 Level A compliance #38342.