From eafc48328e36293dc849c9a9ec46077668e66b9a Mon Sep 17 00:00:00 2001 From: istarkov Date: Sat, 19 Oct 2024 19:43:01 +0000 Subject: [PATCH 1/4] Prevent tooltip on focus --- packages/design-system/src/components/tooltip.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/design-system/src/components/tooltip.tsx b/packages/design-system/src/components/tooltip.tsx index ae916f2aa95c..044063eb8cf8 100644 --- a/packages/design-system/src/components/tooltip.tsx +++ b/packages/design-system/src/components/tooltip.tsx @@ -99,7 +99,15 @@ export const Tooltip = forwardRef( delayDuration={delayDuration} disableHoverableContent={disableHoverableContent} > - + { + // Prevent tooltip from opening on focus + event.preventDefault(); + triggerProps?.onFocus?.(event); + }} + > {children} {content != null && ( From 15e1842decac73742fa6e0b4d787cfac65f99c47 Mon Sep 17 00:00:00 2001 From: istarkov Date: Sat, 19 Oct 2024 19:56:25 +0000 Subject: [PATCH 2/4] Add comment --- packages/design-system/src/components/tooltip.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/design-system/src/components/tooltip.tsx b/packages/design-system/src/components/tooltip.tsx index 044063eb8cf8..f8ee10e30393 100644 --- a/packages/design-system/src/components/tooltip.tsx +++ b/packages/design-system/src/components/tooltip.tsx @@ -103,7 +103,11 @@ export const Tooltip = forwardRef( asChild {...triggerProps} onFocus={(event) => { - // Prevent tooltip from opening on focus + // Prevent the tooltip from being shown on focus, even though this goes against accessibility guidelines. + // The only valid use case for showing tooltips on focus is for users who cannot use a mouse. + // However, since the editor cannot be used without a mouse, it is better to have a working button + // than to attempt to support this scenario, which introduces issues like breaking canvas scrolling or displaying + // the tooltip when closing a dialog/popover. event.preventDefault(); triggerProps?.onFocus?.(event); }} From cbe0a118f007ab06a5487dd5e3d5504ad70013a9 Mon Sep 17 00:00:00 2001 From: istarkov Date: Mon, 21 Oct 2024 14:25:06 +0000 Subject: [PATCH 3/4] Final fix --- packages/design-system/package.json | 1 + .../design-system/src/components/tooltip.tsx | 44 ++++++++----------- pnpm-lock.yaml | 3 ++ 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/packages/design-system/package.json b/packages/design-system/package.json index 797284d1f2bb..ea4a6e9d1c02 100644 --- a/packages/design-system/package.json +++ b/packages/design-system/package.json @@ -32,6 +32,7 @@ "@atlaskit/pragmatic-drag-and-drop-hitbox": "^1.0.3", "@floating-ui/dom": "^1.6.5", "@radix-ui/colors": "^3.0.0", + "@radix-ui/primitive": "^1.1.0", "@radix-ui/react-accessible-icon": "^1.1.0", "@radix-ui/react-avatar": "^1.1.0", "@radix-ui/react-checkbox": "^1.1.1", diff --git a/packages/design-system/src/components/tooltip.tsx b/packages/design-system/src/components/tooltip.tsx index f8ee10e30393..a8c98fe559ee 100644 --- a/packages/design-system/src/components/tooltip.tsx +++ b/packages/design-system/src/components/tooltip.tsx @@ -13,7 +13,7 @@ import { Box } from "./box"; import { Text } from "./text"; import type { CSS } from "../stitches.config"; import { theme } from "../stitches.config"; -import { disableCanvasPointerEvents } from "../utilities"; +import { composeEventHandlers } from "@radix-ui/primitive"; export const TooltipProvider = TooltipPrimitive.TooltipProvider; @@ -78,18 +78,21 @@ export const Tooltip = forwardRef( }); /** - * When the mouse leaves Tooltip.Content and moves over an iframe, the Radix Tooltip stays open. - * This happens because Radix's internal grace area relies on the pointermove event, which isn't triggered over iframes. - * The current workaround is to set pointer-events: none on the canvas when the tooltip is open. - **/ - useEffect(() => { - if (open) { - const enableCanvasPointerEvents = disableCanvasPointerEvents(); - return () => { - enableCanvasPointerEvents?.(); - }; - } - }, [open]); + * When the mouse leaves Tooltip.Content and hovers over an iframe, the Radix Tooltip stays open. + * This occurs because Radix's grace area depends on the pointermove event, which iframes don't trigger. + * + * Two possible workarounds: + * 1. Set pointer-events: none on the canvas when the tooltip is open and content is hovered. + * (This doesn't work well in Chrome, as scrolling stops working on elements hovered with pointer-events: none, + * even after removing pointer-events.) + * 2. Close the tooltip on onMouseLeave. + * (This breaks some grace area behavior, such as closing the tooltip when moving the mouse from the content to the trigger.) + * + * The simpler solution with fewer side effects is to close the tooltip on mouse leave. + */ + const handleMouseEnterComposed = composeEventHandlers(() => { + setOpen(false); + }, props.onMouseLeave); return ( - { - // Prevent the tooltip from being shown on focus, even though this goes against accessibility guidelines. - // The only valid use case for showing tooltips on focus is for users who cannot use a mouse. - // However, since the editor cannot be used without a mouse, it is better to have a working button - // than to attempt to support this scenario, which introduces issues like breaking canvas scrolling or displaying - // the tooltip when closing a dialog/popover. - event.preventDefault(); - triggerProps?.onFocus?.(event); - }} - > + {children} {content != null && ( @@ -124,6 +115,7 @@ export const Tooltip = forwardRef( collisionPadding={8} arrowPadding={8} {...props} + onMouseLeave={handleMouseEnterComposed} > {typeof content === "string" ? {content} : content} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d94a66bf5125..5402f78741d6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1318,6 +1318,9 @@ importers: '@radix-ui/colors': specifier: ^3.0.0 version: 3.0.0 + '@radix-ui/primitive': + specifier: ^1.1.0 + version: 1.1.0 '@radix-ui/react-accessible-icon': specifier: ^1.1.0 version: 1.1.0(@types/react-dom@18.2.25)(@types/react@18.2.79)(react-dom@18.3.0-canary-14898b6a9-20240318(react@18.3.0-canary-14898b6a9-20240318))(react@18.3.0-canary-14898b6a9-20240318) From 412e7618ae671994b60530d688e3fa8119cd2f48 Mon Sep 17 00:00:00 2001 From: istarkov Date: Mon, 21 Oct 2024 15:01:54 +0000 Subject: [PATCH 4/4] Remove --- packages/design-system/package.json | 1 - packages/design-system/src/components/tooltip.tsx | 8 +++++--- pnpm-lock.yaml | 3 --- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/design-system/package.json b/packages/design-system/package.json index ea4a6e9d1c02..797284d1f2bb 100644 --- a/packages/design-system/package.json +++ b/packages/design-system/package.json @@ -32,7 +32,6 @@ "@atlaskit/pragmatic-drag-and-drop-hitbox": "^1.0.3", "@floating-ui/dom": "^1.6.5", "@radix-ui/colors": "^3.0.0", - "@radix-ui/primitive": "^1.1.0", "@radix-ui/react-accessible-icon": "^1.1.0", "@radix-ui/react-avatar": "^1.1.0", "@radix-ui/react-checkbox": "^1.1.1", diff --git a/packages/design-system/src/components/tooltip.tsx b/packages/design-system/src/components/tooltip.tsx index a8c98fe559ee..7f1e72e6c5e1 100644 --- a/packages/design-system/src/components/tooltip.tsx +++ b/packages/design-system/src/components/tooltip.tsx @@ -13,7 +13,6 @@ import { Box } from "./box"; import { Text } from "./text"; import type { CSS } from "../stitches.config"; import { theme } from "../stitches.config"; -import { composeEventHandlers } from "@radix-ui/primitive"; export const TooltipProvider = TooltipPrimitive.TooltipProvider; @@ -90,9 +89,12 @@ export const Tooltip = forwardRef( * * The simpler solution with fewer side effects is to close the tooltip on mouse leave. */ - const handleMouseEnterComposed = composeEventHandlers(() => { + const handleMouseEnterComposed: React.MouseEventHandler = ( + event + ) => { setOpen(false); - }, props.onMouseLeave); + props.onMouseLeave?.(event); + }; return (