From dac435f599184da24e86008dcd034a0e9b6f99fd Mon Sep 17 00:00:00 2001 From: Barsnes Date: Wed, 18 Dec 2024 11:16:52 +0100 Subject: [PATCH 01/21] feat(Tooltip): use popover API --- packages/css/src/tooltip.css | 8 + .../components/Tooltip/Tooltip.stories.tsx | 2 - .../react/src/components/Tooltip/Tooltip.tsx | 197 +++++++++--------- 3 files changed, 110 insertions(+), 97 deletions(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index 20ec841dbf..e388c2883c 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -5,12 +5,15 @@ --dsc-tooltip-padding: var(--ds-spacing-1) var(--ds-spacing-2); --dsc-tooltip-arrow-size: var(--ds-spacing-2); + position: fixed; background: var(--dsc-tooltip-background); border-radius: var(--dsc-tooltip-border-radius); box-sizing: border-box; color: var(--dsc-tooltip-color); line-height: var(--ds-line-height-sm); padding: var(--dsc-tooltip-padding); + overflow: visible; + margin: 0; /* Needed to place tooltip correctly, since popover adds margin */ &::before { content: ''; @@ -24,6 +27,11 @@ translate: -50% -50%; rotate: 45deg; } + + &::backdrop { + display: none; + } + @media (forced-colors: active) { --dsc-tooltip-background: CanvasText; } diff --git a/packages/react/src/components/Tooltip/Tooltip.stories.tsx b/packages/react/src/components/Tooltip/Tooltip.stories.tsx index 73bac76fe2..02f1e288fb 100644 --- a/packages/react/src/components/Tooltip/Tooltip.stories.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.stories.tsx @@ -44,7 +44,6 @@ export const Placement: Story = { export const DefaultOpen: Story = { args: { content: 'Tooltip text', - defaultOpen: true, children: defaultChildren, }, play: async () => { @@ -58,6 +57,5 @@ export const Portal: Story = { content: 'Tooltip text', children: defaultChildren, placement: 'top', - portal: true, }, }; diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index dd7adf38fd..ac85458d70 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -1,36 +1,29 @@ import { - FloatingPortal, type MiddlewareState, autoUpdate, + computePosition, flip, offset, shift, - useDismiss, - useFloating, - useFocus, - useHover, - useInteractions, - useMergeRefs, - useRole, - useTransitionStyles, -} from '@floating-ui/react'; +} from '@floating-ui/dom'; import { Slot } from '@radix-ui/react-slot'; import cl from 'clsx/lite'; -import type { - HTMLAttributes, - MutableRefObject, - ReactElement, - RefAttributes, +import type { HTMLAttributes, ReactElement, RefAttributes } from 'react'; +import { + Fragment, + forwardRef, + useEffect, + useId, + useRef, + useState, } from 'react'; -import { Fragment, forwardRef, useState } from 'react'; -import type { DefaultProps, PortalProps } from '../../types'; +import { useMergeRefs } from '@floating-ui/react'; +import type { DefaultProps } from '../../types'; import type { MergeRight } from '../../utilities'; export type TooltipProps = MergeRight< - Omit & - PortalProps & - HTMLAttributes, + Omit & HTMLAttributes, { /** * The element or string that triggers the tooltip. @@ -56,11 +49,6 @@ export type TooltipProps = MergeRight< * This overrides the internal state of the tooltip. */ open?: boolean; - /** - * Whether the tooltip is open by default or not. - * @default false - */ - defaultOpen?: boolean; } >; @@ -79,64 +67,92 @@ export type TooltipProps = MergeRight< export const Tooltip = forwardRef( function Tooltip( { + id, children, content, placement = 'top', delay = 150, - open: userOpen, - defaultOpen = false, - portal = false, + open, className, - style, ...rest }, ref, ) { - const [isOpen, setIsOpen] = useState(defaultOpen); - const internalOpen = userOpen ?? isOpen; - - const Container = portal ? FloatingPortal : Fragment; - - const { refs, floatingStyles, context } = useFloating({ - open: internalOpen, - onOpenChange: setIsOpen, - placement, - whileElementsMounted: autoUpdate, - middleware: [ - offset((data) => { - // get pseudo element arrow size - const styles = getComputedStyle(data.elements.floating, '::before'); - return parseFloat(styles.height); - }), - flip({ - fallbackAxisSideDirection: 'start', - }), - shift(), - arrowPseudoElement, - ], - }); - - const { styles: animationStyles } = useTransitionStyles(context, { - initial: { - opacity: 0, - }, - }); - - const { getReferenceProps, getFloatingProps } = useInteractions([ - // Event listeners to change the open state - useHover(context, { move: false, delay }), - useFocus(context), - useDismiss(context), - useRole(context, { role: 'tooltip' }), - ]); - - const mergedRef = useMergeRefs([ref, refs.setFloating]); - - const childMergedRef = useMergeRefs([ - (children as ReactElement & RefAttributes) - .ref as MutableRefObject, - refs.setReference, - ]); + const randomTooltipId = useId(); + + const [internalOpen, setInternalOpen] = useState(false); + + const triggerRef = useRef(null); + const tooltipRef = useRef(null); + const mergedRefs = useMergeRefs([tooltipRef, ref]); + + const controlledOpen = open ?? internalOpen; + + // NOTE: This code is purely to add React controlled component ability to Popover API + useEffect(() => { + if (!tooltipRef.current || !triggerRef.current) return; + + const trigger = triggerRef.current; + const tooltip = tooltipRef.current; + const handleMouseover = () => { + setInternalOpen(true); + }; + + const handleMouseout = (event: MouseEvent) => { + /* if we move mosue to tooltip, don't close */ + const el = event.relatedTarget as Element | null; + if (el === tooltip) return; + + setInternalOpen(false); + }; + + const handleClick = (event: MouseEvent) => { + event.preventDefault(); // Prevent native Popover API + }; + + tooltip?.togglePopover?.(controlledOpen); + trigger.addEventListener('mouseover', handleMouseover); + document.addEventListener('mouseout', handleMouseout); + trigger.addEventListener('click', handleClick, true); + + return () => { + trigger.removeEventListener('mouseover', handleMouseover); + document.removeEventListener('mouseout', handleMouseout); + trigger.removeEventListener('click', handleClick, true); + }; + }, [controlledOpen]); + + // Position with floating-ui + useEffect(() => { + const tooltip = tooltipRef.current; + const trigger = triggerRef.current; + + if (tooltip && trigger && controlledOpen) { + return autoUpdate(trigger, tooltip, () => { + computePosition(trigger, tooltip, { + placement, + strategy: 'fixed', + middleware: [ + offset((data) => { + // get pseudo element arrow size + const styles = getComputedStyle( + data.elements.floating, + '::before', + ); + return parseFloat(styles.height) / 2; + }), + flip({ + fallbackAxisSideDirection: 'start', + }), + shift(), + arrowPseudoElement, + ], + }).then(({ x, y }) => { + tooltip.style.translate = `${x}px ${y}px`; + }); + }); + } + }, [controlledOpen, placement]); /* If children is only a string, make a span */ const ChildContainer = typeof children === 'string' ? 'span' : Slot; @@ -151,29 +167,20 @@ export const Tooltip = forwardRef( return ( <> - + {children} - {internalOpen && ( - -
- {content} -
- - )} + ); }, From 3ff1c3a956fe55b6452cb70c22d2aec08fd2704d Mon Sep 17 00:00:00 2001 From: Barsnes Date: Wed, 18 Dec 2024 11:35:48 +0100 Subject: [PATCH 02/21] add delay and focus --- packages/css/src/tooltip.css | 9 +++++---- packages/react/src/components/Tooltip/Tooltip.tsx | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index e388c2883c..641a7bfa7e 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -4,6 +4,7 @@ --dsc-tooltip-border-radius: var(--ds-border-radius-default); --dsc-tooltip-padding: var(--ds-spacing-1) var(--ds-spacing-2); --dsc-tooltip-arrow-size: var(--ds-spacing-2); + --dsc-tooltip-transition-duration: 0.2s; position: fixed; background: var(--dsc-tooltip-background); @@ -14,6 +15,10 @@ padding: var(--dsc-tooltip-padding); overflow: visible; margin: 0; /* Needed to place tooltip correctly, since popover adds margin */ + transition-property: opacity; + transition-duration: var(--dsc-tooltip-transition-duration); + transition-timing-function: ease-in-out; + opacity: 0; &::before { content: ''; @@ -28,10 +33,6 @@ rotate: 45deg; } - &::backdrop { - display: none; - } - @media (forced-colors: active) { --dsc-tooltip-background: CanvasText; } diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index ac85458d70..f31fc4011b 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -94,6 +94,7 @@ export const Tooltip = forwardRef( const trigger = triggerRef.current; const tooltip = tooltipRef.current; + const handleMouseover = () => { setInternalOpen(true); }; @@ -110,15 +111,28 @@ export const Tooltip = forwardRef( event.preventDefault(); // Prevent native Popover API }; + const handleFocusIn = () => { + setInternalOpen(true); + }; + + const handleFocusOut = () => { + setInternalOpen(false); + }; + tooltip?.togglePopover?.(controlledOpen); trigger.addEventListener('mouseover', handleMouseover); + /* We use document so user can move to tooltip contents */ document.addEventListener('mouseout', handleMouseout); trigger.addEventListener('click', handleClick, true); + trigger.addEventListener('focus', handleFocusIn); + trigger.addEventListener('focusout', handleFocusOut); return () => { trigger.removeEventListener('mouseover', handleMouseover); document.removeEventListener('mouseout', handleMouseout); trigger.removeEventListener('click', handleClick, true); + trigger.removeEventListener('focus', handleFocusIn); + trigger.removeEventListener('focusout', handleFocusOut); }; }, [controlledOpen]); @@ -126,6 +140,7 @@ export const Tooltip = forwardRef( useEffect(() => { const tooltip = tooltipRef.current; const trigger = triggerRef.current; + tooltip.style.opacity = controlledOpen ? '1' : '0'; if (tooltip && trigger && controlledOpen) { return autoUpdate(trigger, tooltip, () => { From 53f0f893a49007ffcec589d7b87b87c2fbcfc633 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Wed, 18 Dec 2024 11:42:03 +0100 Subject: [PATCH 03/21] las fixes --- packages/css/src/tooltip.css | 2 ++ .../react/src/components/Tooltip/Tooltip.mdx | 8 -------- .../components/Tooltip/Tooltip.stories.tsx | 19 ------------------- .../react/src/components/Tooltip/Tooltip.tsx | 4 ++-- 4 files changed, 4 insertions(+), 29 deletions(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index 641a7bfa7e..7be944fdbe 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -10,6 +10,8 @@ background: var(--dsc-tooltip-background); border-radius: var(--dsc-tooltip-border-radius); box-sizing: border-box; + border: none; + border-width: 0; color: var(--dsc-tooltip-color); line-height: var(--ds-line-height-sm); padding: var(--dsc-tooltip-padding); diff --git a/packages/react/src/components/Tooltip/Tooltip.mdx b/packages/react/src/components/Tooltip/Tooltip.mdx index 992cc2cdcf..b8482f1f3f 100644 --- a/packages/react/src/components/Tooltip/Tooltip.mdx +++ b/packages/react/src/components/Tooltip/Tooltip.mdx @@ -38,14 +38,6 @@ import { Tooltip } from '@digdir/designsystemet-react'; -## Åpen som standard - - - -## Portal - - - ## CSS Variabler diff --git a/packages/react/src/components/Tooltip/Tooltip.stories.tsx b/packages/react/src/components/Tooltip/Tooltip.stories.tsx index 02f1e288fb..fe3654f676 100644 --- a/packages/react/src/components/Tooltip/Tooltip.stories.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.stories.tsx @@ -40,22 +40,3 @@ export const Placement: Story = { children: defaultChildren, }, }; - -export const DefaultOpen: Story = { - args: { - content: 'Tooltip text', - children: defaultChildren, - }, - play: async () => { - // Wait 500 ms to let tooltip fade in before running tests - await new Promise((resolve) => setTimeout(resolve, 500)); - }, -}; - -export const Portal: Story = { - args: { - content: 'Tooltip text', - children: defaultChildren, - placement: 'top', - }, -}; diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index f31fc4011b..9f2eeeed3b 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -140,7 +140,7 @@ export const Tooltip = forwardRef( useEffect(() => { const tooltip = tooltipRef.current; const trigger = triggerRef.current; - tooltip.style.opacity = controlledOpen ? '1' : '0'; + if (tooltip) tooltip.style.opacity = controlledOpen ? '1' : '0'; if (tooltip && trigger && controlledOpen) { return autoUpdate(trigger, tooltip, () => { @@ -154,7 +154,7 @@ export const Tooltip = forwardRef( data.elements.floating, '::before', ); - return parseFloat(styles.height) / 2; + return parseFloat(styles.height); }), flip({ fallbackAxisSideDirection: 'start', From 1dde1512b3baebcc2423e50c40bd7a628a2969e3 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Wed, 18 Dec 2024 13:05:26 +0100 Subject: [PATCH 04/21] update tests --- packages/css/src/tooltip.css | 2 + .../components/Tooltip/Tooltip.stories.tsx | 2 +- .../src/components/Tooltip/Tooltip.test.tsx | 90 ++++++++----------- 3 files changed, 40 insertions(+), 54 deletions(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index 7be944fdbe..65bcce355f 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -5,6 +5,7 @@ --dsc-tooltip-padding: var(--ds-spacing-1) var(--ds-spacing-2); --dsc-tooltip-arrow-size: var(--ds-spacing-2); --dsc-tooltip-transition-duration: 0.2s; + --dsc-tooltip-transition-delay: 0s; position: fixed; background: var(--dsc-tooltip-background); @@ -18,6 +19,7 @@ overflow: visible; margin: 0; /* Needed to place tooltip correctly, since popover adds margin */ transition-property: opacity; + transition-delay: var(--dsc-tooltip-transition-delay); transition-duration: var(--dsc-tooltip-transition-duration); transition-timing-function: ease-in-out; opacity: 0; diff --git a/packages/react/src/components/Tooltip/Tooltip.stories.tsx b/packages/react/src/components/Tooltip/Tooltip.stories.tsx index fe3654f676..2fd38c4c9f 100644 --- a/packages/react/src/components/Tooltip/Tooltip.stories.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.stories.tsx @@ -16,7 +16,7 @@ export default { } satisfies Meta; export const Preview: StoryFn = (args) => ( - + ); diff --git a/packages/react/src/components/Tooltip/Tooltip.test.tsx b/packages/react/src/components/Tooltip/Tooltip.test.tsx index 48b420bd42..dbda5a5ca6 100644 --- a/packages/react/src/components/Tooltip/Tooltip.test.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.test.tsx @@ -9,7 +9,6 @@ const render = async (props: Partial = {}) => { const allProps: TooltipProps = { children: , content: 'Tooltip text', - delay: 0, ...props, }; /* Flush microtasks */ @@ -18,78 +17,63 @@ const render = async (props: Partial = {}) => { return { user, - ...renderRtl(), + ...renderRtl( + , + ), }; }; describe('Tooltip', () => { - describe('should render children', () => { - it('should render child', async () => { - await render(); - const tooltipTrigger = screen.getByRole('button', { name: 'My button' }); + it('should render child', async () => { + await render(); + const tooltipTrigger = screen.getByRole('button', { name: 'My button' }); - expect(tooltipTrigger).toBeInTheDocument(); - }); + expect(tooltipTrigger).toBeInTheDocument(); }); - describe('should render tooltip', () => { - it('should render tooltip on hover', async () => { - const { user } = await render(); - const tooltipTrigger = screen.getByRole('button', { name: 'My button' }); - - expect(screen.queryByRole('tooltip')).not.toBeInTheDocument(); + it('should render tooltip on hover', async () => { + const { user } = await render(); + const tooltipTrigger = screen.getByRole('button'); - await act(async () => await user.hover(tooltipTrigger)); + expect(screen.getByText('Tooltip text')).not.toBeVisible(); - const tooltip = await screen.findByText('Tooltip text'); - expect(tooltip).toBeInTheDocument(); - expect(screen.queryByRole('tooltip')).toBeInTheDocument(); - }); + await act(async () => await user.hover(tooltipTrigger)); - it('should render tooltip on focus', async () => { - const { user } = await render(); + const tooltip = await screen.findByText('Tooltip text'); + expect(tooltip).toBeVisible(); + expect(screen.getByText('Tooltip text')).toBeVisible(); + }); - expect(screen.queryByText('Tooltip text')).not.toBeInTheDocument(); - await user.click(screen.getByRole('button', { name: 'My button' })); - const tooltip = await screen.findByText('Tooltip text'); - expect(tooltip).toBeInTheDocument(); - expect(screen.queryByRole('tooltip')).toBeInTheDocument(); - }); + it('should render tooltip on focus', async () => { + const { user } = await render(); - it('should close tooltip on escape', async () => { - const { user } = await render(); - const tooltipTrigger = screen.getByRole('button', { name: 'My button' }); - - expect(screen.queryByText('Tooltip text')).not.toBeInTheDocument(); - await act(async () => { - await user.hover(tooltipTrigger); - }); - const tooltip = await screen.findByText('Tooltip text'); - expect(tooltip).toBeInTheDocument(); - await act(async () => { - await user.keyboard('[Escape]'); - }); - expect(screen.queryByText('Tooltip text')).not.toBeInTheDocument(); - }); + expect(screen.queryByText('Tooltip text')).not.toBeVisible(); + await user.click(screen.getByRole('button', { name: 'My button' })); + const tooltip = await screen.findByText('Tooltip text'); + expect(tooltip).toBeInTheDocument(); + expect(screen.queryByRole('tooltip')).toBeVisible(); }); it('should render open when we pass open prop', async () => { await render({ open: true }); - const tooltipTrigger = screen.getByRole('button', { name: 'My button' }); - expect(screen.getByRole('tooltip')).toBeInTheDocument(); - expect(tooltipTrigger).toHaveAttribute('aria-describedby'); + expect(screen.getByRole('tooltip')).toBeVisible(); }); - it('should have delay', async () => { - const { user } = await render({ delay: 300 }); - - await act(async () => await user.hover(screen.getByRole('button'))); - expect(screen.queryByRole('tooltip')).toBeNull(); - - await vi.waitFor(() => { - expect(screen.queryByRole('tooltip')).toBeVisible(); + it('should have correct id and popovertarget attributes', async () => { + await render({ + id: 'my-tooltip', }); + const trigger = screen.getByRole('button'); + const popover = screen.getByText('Tooltip text'); + + expect(trigger.getAttribute('popovertarget')).toBe(popover.id); }); it('should render span when children is a string', async () => { From 1909944d45892efc91385d1ec9761b7a6633f7d1 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Wed, 18 Dec 2024 13:06:35 +0100 Subject: [PATCH 05/21] same delay as we used to have --- packages/css/src/tooltip.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index 65bcce355f..3a1ed325ea 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -5,7 +5,7 @@ --dsc-tooltip-padding: var(--ds-spacing-1) var(--ds-spacing-2); --dsc-tooltip-arrow-size: var(--ds-spacing-2); --dsc-tooltip-transition-duration: 0.2s; - --dsc-tooltip-transition-delay: 0s; + --dsc-tooltip-transition-delay: 150ms; position: fixed; background: var(--dsc-tooltip-background); From 71804446e7deaef1fc157d5201f58f8b91461472 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Wed, 18 Dec 2024 13:09:18 +0100 Subject: [PATCH 06/21] fix showcase portal --- apps/_components/src/Showcase/Showcase.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/_components/src/Showcase/Showcase.tsx b/apps/_components/src/Showcase/Showcase.tsx index be943da93e..94a83cdcb6 100644 --- a/apps/_components/src/Showcase/Showcase.tsx +++ b/apps/_components/src/Showcase/Showcase.tsx @@ -69,7 +69,7 @@ export function Showcase({ className, ...props }: ShowcaseProps) { placeholder='ola@norge.no' className={classes.userField} /> - + Glemt passord? From 375f40dcbf686beb7fcddf902b747a0c792b0f8b Mon Sep 17 00:00:00 2001 From: Barsnes Date: Thu, 19 Dec 2024 07:36:42 +0100 Subject: [PATCH 07/21] add exc, remove preventDefault to make button action work --- .../react/src/components/Tooltip/Tooltip.tsx | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index 9f2eeeed3b..313f6a2618 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -107,10 +107,6 @@ export const Tooltip = forwardRef( setInternalOpen(false); }; - const handleClick = (event: MouseEvent) => { - event.preventDefault(); // Prevent native Popover API - }; - const handleFocusIn = () => { setInternalOpen(true); }; @@ -123,14 +119,12 @@ export const Tooltip = forwardRef( trigger.addEventListener('mouseover', handleMouseover); /* We use document so user can move to tooltip contents */ document.addEventListener('mouseout', handleMouseout); - trigger.addEventListener('click', handleClick, true); trigger.addEventListener('focus', handleFocusIn); trigger.addEventListener('focusout', handleFocusOut); return () => { trigger.removeEventListener('mouseover', handleMouseover); document.removeEventListener('mouseout', handleMouseout); - trigger.removeEventListener('click', handleClick, true); trigger.removeEventListener('focus', handleFocusIn); trigger.removeEventListener('focusout', handleFocusOut); }; @@ -169,6 +163,21 @@ export const Tooltip = forwardRef( } }, [controlledOpen, placement]); + /* Add listener for ESC to dismiss */ + useEffect(() => { + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + setInternalOpen(false); + } + }; + + window.addEventListener('keydown', handleKeyDown); + + return () => { + window.removeEventListener('keydown', handleKeyDown); + }; + }, []); + /* If children is only a string, make a span */ const ChildContainer = typeof children === 'string' ? 'span' : Slot; From e6300c7cf4ef13014c97f3918936a1b897ead50f Mon Sep 17 00:00:00 2001 From: Barsnes Date: Thu, 19 Dec 2024 07:50:48 +0100 Subject: [PATCH 08/21] correct css vars for arrow --- packages/css/src/tooltip.css | 4 ++-- packages/react/src/components/Tooltip/Tooltip.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index 3a1ed325ea..944ceefbb3 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -31,8 +31,8 @@ height: var(--dsc-tooltip-arrow-size); width: var(--dsc-tooltip-arrow-size); position: absolute; - left: var(--ds-tooltip-arrow-x); - top: var(--ds-tooltip-arrow-y); + left: var(--dsc-tooltip-arrow-x); + top: var(--dsc-tooltip-arrow-y); translate: -50% -50%; rotate: 45deg; } diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index 313f6a2618..2f8cd95493 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -237,8 +237,8 @@ const arrowPseudoElement = { break; } - elements.floating.style.setProperty('--ds-tooltip-arrow-x', arrowX); - elements.floating.style.setProperty('--ds-tooltip-arrow-y', arrowY); + elements.floating.style.setProperty('--dsc-tooltip-arrow-x', arrowX); + elements.floating.style.setProperty('--dsc-tooltip-arrow-y', arrowY); return data; }, }; From c655c591a35d764f8473342f53614f1aaaba029b Mon Sep 17 00:00:00 2001 From: Barsnes Date: Thu, 19 Dec 2024 08:21:12 +0100 Subject: [PATCH 09/21] from review --- .../react/src/components/Tooltip/Tooltip.tsx | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index 2f8cd95493..b671bbd1fa 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -95,38 +95,34 @@ export const Tooltip = forwardRef( const trigger = triggerRef.current; const tooltip = tooltipRef.current; - const handleMouseover = () => { + const setOpen = () => { setInternalOpen(true); }; - const handleMouseout = (event: MouseEvent) => { - /* if we move mosue to tooltip, don't close */ - const el = event.relatedTarget as Element | null; - if (el === tooltip) return; - + const setClose = () => { setInternalOpen(false); }; - const handleFocusIn = () => { - setInternalOpen(true); - }; + const handleMouseout = (event: MouseEvent) => { + /* if we move mouse to tooltip, don't close */ + const el = event.relatedTarget as Element | null; + if (el === tooltip) return; - const handleFocusOut = () => { - setInternalOpen(false); + setClose(); }; tooltip?.togglePopover?.(controlledOpen); - trigger.addEventListener('mouseover', handleMouseover); + trigger.addEventListener('mouseover', setOpen); /* We use document so user can move to tooltip contents */ document.addEventListener('mouseout', handleMouseout); - trigger.addEventListener('focus', handleFocusIn); - trigger.addEventListener('focusout', handleFocusOut); + trigger.addEventListener('focusin', setOpen); + trigger.addEventListener('focusout', setClose); return () => { - trigger.removeEventListener('mouseover', handleMouseover); + trigger.removeEventListener('mouseover', setOpen); document.removeEventListener('mouseout', handleMouseout); - trigger.removeEventListener('focus', handleFocusIn); - trigger.removeEventListener('focusout', handleFocusOut); + trigger.removeEventListener('focusin', setOpen); + trigger.removeEventListener('focusout', setClose); }; }, [controlledOpen]); From b07e9eb5acf788e011aba684f23c688963b4f2c7 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Thu, 19 Dec 2024 08:22:27 +0100 Subject: [PATCH 10/21] mouseenter --- packages/react/src/components/Tooltip/Tooltip.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index b671bbd1fa..dd2a467afd 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -112,14 +112,14 @@ export const Tooltip = forwardRef( }; tooltip?.togglePopover?.(controlledOpen); - trigger.addEventListener('mouseover', setOpen); + trigger.addEventListener('mouseenter', setOpen); /* We use document so user can move to tooltip contents */ document.addEventListener('mouseout', handleMouseout); trigger.addEventListener('focusin', setOpen); trigger.addEventListener('focusout', setClose); return () => { - trigger.removeEventListener('mouseover', setOpen); + trigger.removeEventListener('mouseenter', setOpen); document.removeEventListener('mouseout', handleMouseout); trigger.removeEventListener('focusin', setOpen); trigger.removeEventListener('focusout', setClose); From 53bdef8233f4f51da6c28d96ec243767e2aa1477 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Thu, 19 Dec 2024 08:50:37 +0100 Subject: [PATCH 11/21] popovertargetaction='show' --- packages/react/src/components/Tooltip/Tooltip.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index dd2a467afd..db8b4fa3de 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -187,7 +187,13 @@ export const Tooltip = forwardRef( return ( <> - + {children}
Date: Thu, 19 Dec 2024 09:47:01 +0100 Subject: [PATCH 12/21] remove delay prop --- .../react/src/components/Tooltip/Tooltip.tsx | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index db8b4fa3de..f8d77a9fb9 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -39,11 +39,6 @@ export type TooltipProps = MergeRight< * @default 'top' */ placement?: 'top' | 'right' | 'bottom' | 'left'; - /** - * Delay in milliseconds before opening. - * @default 150 - */ - delay?: number; /** * Whether the tooltip is open or not. * This overrides the internal state of the tooltip. @@ -66,16 +61,7 @@ export type TooltipProps = MergeRight< */ export const Tooltip = forwardRef( function Tooltip( - { - id, - children, - content, - placement = 'top', - delay = 150, - open, - className, - ...rest - }, + { id, children, content, placement = 'top', open, className, ...rest }, ref, ) { const randomTooltipId = useId(); From 3121b1c278ae259974e404f689822b55fcb8c0b8 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Thu, 19 Dec 2024 09:47:29 +0100 Subject: [PATCH 13/21] remove id from preview story --- packages/react/src/components/Tooltip/Tooltip.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.stories.tsx b/packages/react/src/components/Tooltip/Tooltip.stories.tsx index 2fd38c4c9f..fe3654f676 100644 --- a/packages/react/src/components/Tooltip/Tooltip.stories.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.stories.tsx @@ -16,7 +16,7 @@ export default { } satisfies Meta; export const Preview: StoryFn = (args) => ( - + ); From 6fb92a40f471317005a27e9849776f9bc1e65bd7 Mon Sep 17 00:00:00 2001 From: eirikbacker Date: Thu, 19 Dec 2024 12:34:09 +0100 Subject: [PATCH 14/21] fix(Tooltip): safari rendering --- packages/css/src/tooltip.css | 14 +++++++++++--- packages/react/src/components/Tooltip/Tooltip.tsx | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index 944ceefbb3..520cfdeb21 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -8,6 +8,7 @@ --dsc-tooltip-transition-delay: 150ms; position: fixed; + inset: 0 auto auto 0; background: var(--dsc-tooltip-background); border-radius: var(--dsc-tooltip-border-radius); box-sizing: border-box; @@ -18,11 +19,18 @@ padding: var(--dsc-tooltip-padding); overflow: visible; margin: 0; /* Needed to place tooltip correctly, since popover adds margin */ - transition-property: opacity; + transition-property: opacity, visibility; transition-delay: var(--dsc-tooltip-transition-delay); transition-duration: var(--dsc-tooltip-transition-duration); transition-timing-function: ease-in-out; opacity: 0; + display: block; + visibility: hidden; + + &:popover-open { + opacity: 1; + visibility: visible; + } &::before { content: ''; @@ -31,8 +39,8 @@ height: var(--dsc-tooltip-arrow-size); width: var(--dsc-tooltip-arrow-size); position: absolute; - left: var(--dsc-tooltip-arrow-x); - top: var(--dsc-tooltip-arrow-y); + left: var(--dsc-tooltip-arrow-x, 50%); + top: var(--dsc-tooltip-arrow-y, 100%); translate: -50% -50%; rotate: 45deg; } diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index f8d77a9fb9..c125d2c0da 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -116,8 +116,8 @@ export const Tooltip = forwardRef( useEffect(() => { const tooltip = tooltipRef.current; const trigger = triggerRef.current; - if (tooltip) tooltip.style.opacity = controlledOpen ? '1' : '0'; + tooltip?.togglePopover?.(controlledOpen); if (tooltip && trigger && controlledOpen) { return autoUpdate(trigger, tooltip, () => { computePosition(trigger, tooltip, { From ffc0b651a12b5858b9ce4230144f2bf35efe1034 Mon Sep 17 00:00:00 2001 From: Tobias Barsnes Date: Thu, 19 Dec 2024 14:13:09 +0100 Subject: [PATCH 15/21] Create red-chefs-refuse.md --- .changeset/red-chefs-refuse.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/red-chefs-refuse.md diff --git a/.changeset/red-chefs-refuse.md b/.changeset/red-chefs-refuse.md new file mode 100644 index 0000000000..947e5f8078 --- /dev/null +++ b/.changeset/red-chefs-refuse.md @@ -0,0 +1,9 @@ +--- +"@digdir/designsystemet-css": patch +"@digdir/designsystemet-react": patch +--- + +Tooltip: Use popover API +- Removes `delay`, this is now `--dsc-tooltip-transition-delay` +- Removes `defaultOpen` +- Removes `portal` From 26b1f4b83a53cdb44790bd634f544c7e72fdcd27 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Fri, 20 Dec 2024 10:27:33 +0100 Subject: [PATCH 16/21] add kjent mangler, and remove safari support --- packages/css/src/tooltip.css | 1 - packages/react/src/components/Tooltip/Tooltip.mdx | 3 +++ packages/react/src/components/Tooltip/Tooltip.tsx | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/css/src/tooltip.css b/packages/css/src/tooltip.css index 520cfdeb21..88c419e611 100644 --- a/packages/css/src/tooltip.css +++ b/packages/css/src/tooltip.css @@ -24,7 +24,6 @@ transition-duration: var(--dsc-tooltip-transition-duration); transition-timing-function: ease-in-out; opacity: 0; - display: block; visibility: hidden; &:popover-open { diff --git a/packages/react/src/components/Tooltip/Tooltip.mdx b/packages/react/src/components/Tooltip/Tooltip.mdx index 883d543f77..a03461394e 100644 --- a/packages/react/src/components/Tooltip/Tooltip.mdx +++ b/packages/react/src/components/Tooltip/Tooltip.mdx @@ -88,3 +88,6 @@ Hvis `tooltip` er knyttet til et ord i en tekst, marker ordet med en stiplet lin ### Interaksjon med touch På berøringsskjermer er `tooltip` mindre egnet, fordi de vanligvis aktiveres ved `hover` eller `fokus`, som ikke støttes på disse enhetene. `Tooltip` vises i stedet kun når brukeren trykker på elementet, og forsvinner igjen når brukeren trykker utenfor elementet. +## Kjente mangler + +I Safari fungerer ikke fade-in animasjon. diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index c125d2c0da..7bd4e87200 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -118,6 +118,7 @@ export const Tooltip = forwardRef( const trigger = triggerRef.current; tooltip?.togglePopover?.(controlledOpen); + if (tooltip) tooltip.style.opacity = controlledOpen ? '1' : '0'; if (tooltip && trigger && controlledOpen) { return autoUpdate(trigger, tooltip, () => { computePosition(trigger, tooltip, { From 19c39c7ced8070bc010682431f30c765407c510d Mon Sep 17 00:00:00 2001 From: Barsnes Date: Fri, 20 Dec 2024 11:54:48 +0100 Subject: [PATCH 17/21] always close when moving away from trigger --- packages/react/src/components/Tooltip/Tooltip.tsx | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index 7bd4e87200..1a5e0d36b3 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -89,24 +89,15 @@ export const Tooltip = forwardRef( setInternalOpen(false); }; - const handleMouseout = (event: MouseEvent) => { - /* if we move mouse to tooltip, don't close */ - const el = event.relatedTarget as Element | null; - if (el === tooltip) return; - - setClose(); - }; - tooltip?.togglePopover?.(controlledOpen); trigger.addEventListener('mouseenter', setOpen); - /* We use document so user can move to tooltip contents */ - document.addEventListener('mouseout', handleMouseout); + trigger.addEventListener('mouseout', setClose); trigger.addEventListener('focusin', setOpen); trigger.addEventListener('focusout', setClose); return () => { trigger.removeEventListener('mouseenter', setOpen); - document.removeEventListener('mouseout', handleMouseout); + trigger.removeEventListener('mouseout', setClose); trigger.removeEventListener('focusin', setOpen); trigger.removeEventListener('focusout', setClose); }; From f0af89ceaf0008da5af255122df4feb69f2df09c Mon Sep 17 00:00:00 2001 From: Barsnes Date: Fri, 20 Dec 2024 11:57:15 +0100 Subject: [PATCH 18/21] add listeners to element and not effect --- .../react/src/components/Tooltip/Tooltip.tsx | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.tsx b/packages/react/src/components/Tooltip/Tooltip.tsx index 1a5e0d36b3..4532189a45 100644 --- a/packages/react/src/components/Tooltip/Tooltip.tsx +++ b/packages/react/src/components/Tooltip/Tooltip.tsx @@ -74,34 +74,13 @@ export const Tooltip = forwardRef( const controlledOpen = open ?? internalOpen; - // NOTE: This code is purely to add React controlled component ability to Popover API - useEffect(() => { - if (!tooltipRef.current || !triggerRef.current) return; - - const trigger = triggerRef.current; - const tooltip = tooltipRef.current; - - const setOpen = () => { - setInternalOpen(true); - }; + const setOpen = () => { + setInternalOpen(true); + }; - const setClose = () => { - setInternalOpen(false); - }; - - tooltip?.togglePopover?.(controlledOpen); - trigger.addEventListener('mouseenter', setOpen); - trigger.addEventListener('mouseout', setClose); - trigger.addEventListener('focusin', setOpen); - trigger.addEventListener('focusout', setClose); - - return () => { - trigger.removeEventListener('mouseenter', setOpen); - trigger.removeEventListener('mouseout', setClose); - trigger.removeEventListener('focusin', setOpen); - trigger.removeEventListener('focusout', setClose); - }; - }, [controlledOpen]); + const setClose = () => { + setInternalOpen(false); + }; // Position with floating-ui useEffect(() => { @@ -171,6 +150,10 @@ export const Tooltip = forwardRef( // We set this to not close on click, since it should always show on hover // @ts-ignore @types/react-dom does not understand popovertargetaction yet popovertargetaction='show' + onMouseEnter={setOpen} + onMouseLeave={setClose} + onFocus={setOpen} + onBlur={setClose} > {children} From 579def030e8750f6195442b660b50748d8cf9e96 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Fri, 20 Dec 2024 12:13:10 +0100 Subject: [PATCH 19/21] update docs --- packages/react/src/components/Tooltip/Tooltip.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.mdx b/packages/react/src/components/Tooltip/Tooltip.mdx index a03461394e..fc2a1c618d 100644 --- a/packages/react/src/components/Tooltip/Tooltip.mdx +++ b/packages/react/src/components/Tooltip/Tooltip.mdx @@ -80,7 +80,7 @@ Hvis `tooltip` er knyttet til et ord i en tekst, marker ordet med en stiplet lin ## Tilgjengelighet ### Interaksjon med mus -`Tooltip` vises når et element får hover, og musepekeren kan føres over `tooltip` uten at den blir borte. Når elementet eller selve `tooltip` mister hover, blir `tooltip` borte. +`Tooltip` vises når et element får hover, og blir borte når musen forlater elementet. ### Interaksjon med tastatur `Tooltip` vises når et element får tastaturfokus, og forsvinner når fokus fjernes. From 5920355ebca04b0448dc4bb614f599277bbec051 Mon Sep 17 00:00:00 2001 From: Barsnes Date: Thu, 2 Jan 2025 10:21:26 +0100 Subject: [PATCH 20/21] update kjent mangler --- packages/react/src/components/Tooltip/Tooltip.mdx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/Tooltip/Tooltip.mdx b/packages/react/src/components/Tooltip/Tooltip.mdx index fc2a1c618d..257f9625d2 100644 --- a/packages/react/src/components/Tooltip/Tooltip.mdx +++ b/packages/react/src/components/Tooltip/Tooltip.mdx @@ -90,4 +90,5 @@ På berøringsskjermer er `tooltip` mindre egnet, fordi de vanligvis aktiveres v ## Kjente mangler -I Safari fungerer ikke fade-in animasjon. +- I Safari fungerer ikke fade-in animasjon. +- Brukeren kan ikke flytte musepeker inn i `tooltip` From 514a2a55cb7e37530cb3ec68361259a85c8b1a16 Mon Sep 17 00:00:00 2001 From: Tobias Barsnes Date: Thu, 2 Jan 2025 15:43:44 +0100 Subject: [PATCH 21/21] Update red-chefs-refuse.md --- .changeset/red-chefs-refuse.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/red-chefs-refuse.md b/.changeset/red-chefs-refuse.md index 947e5f8078..67df321729 100644 --- a/.changeset/red-chefs-refuse.md +++ b/.changeset/red-chefs-refuse.md @@ -7,3 +7,4 @@ Tooltip: Use popover API - Removes `delay`, this is now `--dsc-tooltip-transition-delay` - Removes `defaultOpen` - Removes `portal` +- Removes ability to hover to keep open