Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prevent tooltip on focus #4318

Merged
merged 4 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/design-system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
44 changes: 18 additions & 26 deletions packages/design-system/src/components/tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
istarkov marked this conversation as resolved.
Show resolved Hide resolved

export const TooltipProvider = TooltipPrimitive.TooltipProvider;

Expand Down Expand Up @@ -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);
istarkov marked this conversation as resolved.
Show resolved Hide resolved

return (
<TooltipPrimitive.Root
Expand All @@ -99,19 +102,7 @@ export const Tooltip = forwardRef(
delayDuration={delayDuration}
disableHoverableContent={disableHoverableContent}
>
<TooltipPrimitive.Trigger
asChild
{...triggerProps}
onFocus={(event) => {
// 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);
}}
>
<TooltipPrimitive.Trigger asChild {...triggerProps}>
{children}
</TooltipPrimitive.Trigger>
{content != null && (
Expand All @@ -124,6 +115,7 @@ export const Tooltip = forwardRef(
collisionPadding={8}
arrowPadding={8}
{...props}
onMouseLeave={handleMouseEnterComposed}
>
{typeof content === "string" ? <Text>{content}</Text> : content}
<Box css={{ color: theme.colors.transparentExtreme }}>
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading