From 8944ae7cf73903b7fbd13e67420a3838f13be85d Mon Sep 17 00:00:00 2001 From: Fedo Hagge-Kubat Date: Mon, 2 Mar 2026 18:33:16 +0100 Subject: [PATCH 1/3] =?UTF-8?q?fix(a11y):=20WCAG=201.4.13=20=E2=80=94=20ma?= =?UTF-8?q?ke=20hover=20content=20dismissable=20and=20hoverable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../AgGridTable/components/CustomPopover.tsx | 6 +++++ .../src/components/Tooltip.tsx | 24 ++++++++++++++++--- .../src/features/tasks/TaskPayloadPopover.tsx | 10 +++++++- .../features/tasks/TaskStackTracePopover.tsx | 10 +++++++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx index e630986bf69a..d2e404e98c24 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomPopover.tsx @@ -69,15 +69,21 @@ const CustomPopover: React.FC = ({ } }; + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') onClose(); + }; + if (isOpen) { updatePosition(); document.addEventListener('mousedown', handleClickOutside); + document.addEventListener('keydown', handleKeyDown); window.addEventListener('scroll', updatePosition); window.addEventListener('resize', updatePosition); } return () => { document.removeEventListener('mousedown', handleClickOutside); + document.removeEventListener('keydown', handleKeyDown); window.removeEventListener('scroll', updatePosition); window.removeEventListener('resize', updatePosition); }; diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx index 1eca2c1a77fe..d6b3ce7c581d 100644 --- a/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx +++ b/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx @@ -19,7 +19,7 @@ import { safeHtmlSpan } from '@superset-ui/core'; import { styled } from '@apache-superset/core/theme'; -import { ReactNode } from 'react'; +import { ReactNode, useEffect, useState } from 'react'; export type TooltipProps = { tooltip: @@ -43,7 +43,7 @@ const StyledDiv = styled.div<{ top: ${top}px; left: ${left}px; z-index: 9; - pointer-events: none; + pointer-events: auto; ${ variant === 'default' ? ` @@ -66,7 +66,25 @@ const StyledDiv = styled.div<{ export default function Tooltip(props: TooltipProps) { const { tooltip, variant = 'default' } = props; - if (typeof tooltip === 'undefined' || tooltip === null) { + const [dismissed, setDismissed] = useState(false); + + // Reset dismissed state when tooltip content changes (new hover target) + useEffect(() => { + setDismissed(false); + }, [tooltip?.x, tooltip?.y]); + + // Dismiss on Escape key + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') setDismissed(true); + }; + if (tooltip && !dismissed) { + document.addEventListener('keydown', handleKeyDown); + } + return () => document.removeEventListener('keydown', handleKeyDown); + }, [tooltip, dismissed]); + + if (typeof tooltip === 'undefined' || tooltip === null || dismissed) { return null; } diff --git a/superset-frontend/src/features/tasks/TaskPayloadPopover.tsx b/superset-frontend/src/features/tasks/TaskPayloadPopover.tsx index b8276327e577..e419919fdfb7 100644 --- a/superset-frontend/src/features/tasks/TaskPayloadPopover.tsx +++ b/superset-frontend/src/features/tasks/TaskPayloadPopover.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { useState } from 'react'; +import { useState, useEffect } from 'react'; import { styled } from '@apache-superset/core/theme'; import { Popover } from '@superset-ui/core/components'; import { Icons } from '@superset-ui/core/components/Icons'; @@ -54,6 +54,14 @@ export default function TaskPayloadPopover({ }: TaskPayloadPopoverProps) { const [visible, setVisible] = useState(false); + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') setVisible(false); + }; + if (visible) document.addEventListener('keydown', handleKeyDown); + return () => document.removeEventListener('keydown', handleKeyDown); + }, [visible]); + const content = ( {JSON.stringify(payload, null, 2)} diff --git a/superset-frontend/src/features/tasks/TaskStackTracePopover.tsx b/superset-frontend/src/features/tasks/TaskStackTracePopover.tsx index 18b7c725132c..82a2f746676c 100644 --- a/superset-frontend/src/features/tasks/TaskStackTracePopover.tsx +++ b/superset-frontend/src/features/tasks/TaskStackTracePopover.tsx @@ -17,7 +17,7 @@ * under the License. */ -import { useState, useCallback } from 'react'; +import { useState, useCallback, useEffect } from 'react'; import { t } from '@apache-superset/core/translation'; import { styled } from '@apache-superset/core/theme'; import { Popover, Tooltip } from '@superset-ui/core/components'; @@ -90,6 +90,14 @@ export default function TaskStackTracePopover({ const [copied, setCopied] = useState(false); const { addDangerToast } = useToasts(); + useEffect(() => { + const handleKeyDown = (e: KeyboardEvent) => { + if (e.key === 'Escape') setVisible(false); + }; + if (visible) document.addEventListener('keydown', handleKeyDown); + return () => document.removeEventListener('keydown', handleKeyDown); + }, [visible]); + const handleCopy = useCallback(() => { copyTextToClipboard(() => Promise.resolve(stackTrace)) .then(() => { From 0044e80724ef5c6ae18f6827e1a4db0f6953fc1b Mon Sep 17 00:00:00 2001 From: Fedo Hagge-Kubat Date: Thu, 9 Apr 2026 21:16:46 +0200 Subject: [PATCH 2/3] fix(a11y): reset dismiss only on content change, not mouse coordinates --- .../plugins/preset-chart-deckgl/src/components/Tooltip.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx index d6b3ce7c581d..29f1319d3408 100644 --- a/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx +++ b/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx @@ -68,10 +68,11 @@ export default function Tooltip(props: TooltipProps) { const { tooltip, variant = 'default' } = props; const [dismissed, setDismissed] = useState(false); - // Reset dismissed state when tooltip content changes (new hover target) + // Reset dismissed state only when tooltip content changes (new hover target), + // not on coordinate changes from mouse movement useEffect(() => { setDismissed(false); - }, [tooltip?.x, tooltip?.y]); + }, [tooltip?.content]); // Dismiss on Escape key useEffect(() => { From e8c404885e4b4f1e4b451cf9ccafd0445b569e6a Mon Sep 17 00:00:00 2001 From: Kolja Date: Thu, 7 May 2026 18:28:13 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(a11y):=20WCAG=201.4.13=20=E2=80=94=20ke?= =?UTF-8?q?ep=20deck.gl=20tooltips=20non-blocking,=20stabilise=20dismiss?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Rusackas' regression concern and three bot review items on PR #39239: - pointer-events back to `none` on deck.gl tooltips. The previous switch to `auto` was the load-bearing regression Rusackas flagged: deck.gl tooltips track the cursor across the canvas, and capturing pointer events here blocks the underlying map from receiving hover/click and causes flicker loops. WCAG 1.4.13 "hoverable" is still satisfied because the tooltip stays under the cursor while pointing at the feature. - Dismiss-state reset now also fires on the hidden-to-visible transition, not only on content change. Previously, moving off a feature and back onto a different pickable that renders the same content would leave the tooltip permanently dismissed — bot bito flagged this for repeated-value layers. - Escape keydown listener is now bound based on a stable visibility flag derived from `(tooltip && !dismissed)` instead of `[tooltip, dismissed]` directly. The old dependency array re-ran on every cursor coordinate change, churning addEventListener/removeEventListener pairs at hover rate. --- .../src/components/Tooltip.tsx | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx index 29f1319d3408..002854b9f02c 100644 --- a/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx +++ b/superset-frontend/plugins/preset-chart-deckgl/src/components/Tooltip.tsx @@ -19,7 +19,7 @@ import { safeHtmlSpan } from '@superset-ui/core'; import { styled } from '@apache-superset/core/theme'; -import { ReactNode, useEffect, useState } from 'react'; +import { ReactNode, useEffect, useRef, useState } from 'react'; export type TooltipProps = { tooltip: @@ -43,7 +43,13 @@ const StyledDiv = styled.div<{ top: ${top}px; left: ${left}px; z-index: 9; - pointer-events: auto; + /* deck.gl tooltips track the cursor across the canvas — capturing pointer + events here would block layer hover/click and create flicker loops as + the cursor enters and leaves the floating tooltip. Dismissal is via the + Escape key handler below; WCAG 1.4.13 "hoverable" is satisfied because + the tooltip remains visible under the cursor while pointing at the + feature. */ + pointer-events: none; ${ variant === 'default' ? ` @@ -67,25 +73,38 @@ const StyledDiv = styled.div<{ export default function Tooltip(props: TooltipProps) { const { tooltip, variant = 'default' } = props; const [dismissed, setDismissed] = useState(false); + const wasVisibleRef = useRef(false); - // Reset dismissed state only when tooltip content changes (new hover target), - // not on coordinate changes from mouse movement + // Reset dismissed when the tooltip transitions from hidden to visible. This + // handles the case where the cursor leaves the chart and re-enters a + // different pickable that happens to render the same content — content + // alone wouldn't trigger a reset there. + useEffect(() => { + const isVisible = !!tooltip; + if (isVisible && !wasVisibleRef.current) { + setDismissed(false); + } + wasVisibleRef.current = isVisible; + }, [tooltip]); + + // Reset on content change too (most common new-target signal). useEffect(() => { setDismissed(false); }, [tooltip?.content]); - // Dismiss on Escape key + // Bind the Escape listener once per visibility cycle. Depending on + // `tooltip` directly would re-bind on every cursor move (x/y change). + const visible = !!tooltip && !dismissed; useEffect(() => { + if (!visible) return undefined; const handleKeyDown = (e: KeyboardEvent) => { if (e.key === 'Escape') setDismissed(true); }; - if (tooltip && !dismissed) { - document.addEventListener('keydown', handleKeyDown); - } + document.addEventListener('keydown', handleKeyDown); return () => document.removeEventListener('keydown', handleKeyDown); - }, [tooltip, dismissed]); + }, [visible]); - if (typeof tooltip === 'undefined' || tooltip === null || dismissed) { + if (!tooltip || dismissed) { return null; }