From ee0574aee5c66a521e7348ec33eb322e1655f8c0 Mon Sep 17 00:00:00 2001 From: Ihor Romanchuk Date: Tue, 29 Sep 2026 15:43:46 +0200 Subject: [PATCH 1/5] feat(design-system): make DsPopover and DsTooltip controllable and composable [AR-75956] - DsTooltip: controlled open / defaultOpen / onOpenChange; forward ref and asChild-injected props; share the trigger id with an outer trigger. - DsPopover.Trigger / .Anchor: forward ref and injected props so a tooltip and a popover can share one trigger in either nesting order. - Add DsPopover.CloseTrigger and a DsPopover.Header actions slot. - openOn="hover": a click pins the panel until Escape, outside click, close button or another trigger click; any close cancels a pending hover open. - Hoist DsTreeRowTriggerProps into a shared DsAsChildTriggerProps type (aliased, public API unchanged). --- .changeset/popover-tooltip-composition.md | 10 + cspell/local-words.txt | 1 + .../__snapshots__/ds-popover.docs.snap | 93 +++++- .../__tests__/ds-popover.browser.test.tsx | 290 +++++++++++++++++- .../popover-trigger/popover-trigger.tsx | 25 +- .../ds-popover/ds-popover.hover-intent.ts | 84 ++++- .../ds-popover/ds-popover.module.scss | 8 + .../ds-popover/ds-popover.stories.tsx | 61 +++- .../src/components/ds-popover/ds-popover.tsx | 69 ++++- .../components/ds-popover/ds-popover.types.ts | 36 ++- .../__snapshots__/ds-tooltip.docs.snap | 48 ++- .../__tests__/ds-tooltip.browser.test.tsx | 96 ++++++ .../ds-tooltip/ds-tooltip.stories.tsx | 31 ++ .../src/components/ds-tooltip/ds-tooltip.tsx | 48 ++- .../components/ds-tooltip/ds-tooltip.types.ts | 19 +- .../src/components/ds-tree/ds-tree.types.ts | 30 +- .../src/utils/as-child-trigger-props.ts | 35 +++ 17 files changed, 915 insertions(+), 69 deletions(-) create mode 100644 .changeset/popover-tooltip-composition.md create mode 100644 packages/design-system/src/utils/as-child-trigger-props.ts diff --git a/.changeset/popover-tooltip-composition.md b/.changeset/popover-tooltip-composition.md new file mode 100644 index 000000000..55e83bbe5 --- /dev/null +++ b/.changeset/popover-tooltip-composition.md @@ -0,0 +1,10 @@ +--- +'@drivenets/design-system': minor +--- + +Make `DsPopover` and `DsTooltip` controllable and composable on a shared trigger: + +- `DsTooltip`: add controlled `open` / `defaultOpen` / `onOpenChange(open)`. +- `DsTooltip`, `DsPopover.Trigger` and `DsPopover.Anchor` forward `ref` and props injected by an outer `asChild` trigger, so a tooltip and a popover can share one trigger element (either nesting order; `DsPopover.Trigger > DsTooltip > element` is recommended). +- Add `DsPopover.CloseTrigger` (icon-only close button, `locale={{ close }}`, default `'Close'`) and a `DsPopover.Header` `actions` trailing slot to place it. +- `DsPopover.Root` `openOn="hover"`: a click pins the panel — clicking a hover-opened panel keeps it open, and a click-opened panel survives the pointer leaving until Escape, an outside click, the close button, or another trigger click. diff --git a/cspell/local-words.txt b/cspell/local-words.txt index 7ca34ce55..002bbd773 100644 --- a/cspell/local-words.txt +++ b/cspell/local-words.txt @@ -11,6 +11,7 @@ clickability controlify cooldown cursoragents +describedby deslop domcontentloaded drivenets diff --git a/packages/design-system/src/components/ds-popover/__tests__/__snapshots__/ds-popover.docs.snap b/packages/design-system/src/components/ds-popover/__tests__/__snapshots__/ds-popover.docs.snap index 5c6329bf8..9ea13e5fb 100644 --- a/packages/design-system/src/components/ds-popover/__tests__/__snapshots__/ds-popover.docs.snap +++ b/packages/design-system/src/components/ds-popover/__tests__/__snapshots__/ds-popover.docs.snap @@ -16,7 +16,10 @@ - }> + } + icon={} + > Release lock @@ -70,7 +73,9 @@ const WithContentItemsAndCTA = () => Release lock - }>Release lock + } + actions={}>Release lock }>Releases a physical lock on a device or asset, granting immediate access to the selected inventory @@ -376,4 +381,86 @@ const CustomAnchor = () => -; \ No newline at end of file +; + +## With Tooltip + +### Show code +{ + parameters: { + docs: { + source: { + type: 'code' + } + } + }, + render: function Render(args) { + const [open, setOpen] = useState(false); + return + + + + Provision edge + + + + + } actions={}> + Provision edge + + + }> + Provisions a new edge device and attaches it to the selected site, including the full + description that the preview truncates. + + + 2.3.4 (latest) + + + + Open in catalog + + + + ; + } +} + +### MCP manifest +const WithTooltip = () => { + const [open, setOpen] = useState(false); + + return ( + + + + Provision edge + + + + + } + actions={}>Provision edge + + + }>Provisions a new edge device and attaches it to the selected site, including the full + description that the preview truncates. + + + 2.3.4 (latest) + + + Open in catalog + + + + + ); +}; \ No newline at end of file diff --git a/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx b/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx index e8d70bd2f..ee6c4a395 100644 --- a/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx +++ b/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx @@ -1,6 +1,7 @@ -import { useRef } from 'react'; +import { createRef, useRef, type ReactNode } from 'react'; import { describe, expect, it, vi } from 'vitest'; import { page, userEvent } from 'vitest/browser'; +import { DsTooltip } from '../../ds-tooltip'; import { DsPopover } from '../ds-popover'; import type { DsPopoverRootProps } from '../ds-popover.types'; import styles from '../ds-popover.stories.module.scss'; @@ -338,6 +339,94 @@ describe('DsPopover openOn="hover"', () => { await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); }); + it('keeps a hover-opened panel open when the trigger is clicked', async () => { + await renderHoverExample(); + + await getTrigger().hover(); + await expect.element(getPanel()).toBeVisible(); + + await getTrigger().click(); + await wait(OPEN_DELAY); + + await expect.element(getPanel()).toBeVisible(); + }); + + it('keeps a click-opened panel open after the pointer leaves', async () => { + await renderHoverExample(); + + await getTrigger().click(); + await expect.element(getPanel()).toBeVisible(); + + await getTrigger().unhover(); + await wait(CLOSE_DELAY + OPEN_DELAY); + + await expect.element(getPanel()).toBeVisible(); + }); + + it('closes a hover-opened panel once the pointer leaves the trigger', async () => { + await renderHoverExample(); + + await getTrigger().hover(); + await expect.element(getPanel()).toBeVisible(); + + await getTrigger().unhover(); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + }); + + it('keeps a pinned panel open when the pointer enters and leaves it', async () => { + await renderHoverExample(); + + await getTrigger().hover(); + await expect.element(getPanel()).toBeVisible(); + await getTrigger().click(); + + await getPanel().hover(); + await getPanel().unhover(); + await wait(CLOSE_DELAY + OPEN_DELAY); + + await expect.element(getPanel()).toBeVisible(); + }); + + it('closes a pinned panel on a second trigger click', async () => { + await renderHoverExample(); + + await getTrigger().hover(); + await expect.element(getPanel()).toBeVisible(); + + await getTrigger().click(); + await getTrigger().click(); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + }); + + it('does not carry a pin over to the next hover-opened session', async () => { + await renderHoverExample(); + + await getTrigger().click(); + await expect.element(getPanel()).toBeVisible(); + await userEvent.keyboard('{Escape}'); + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + + await getTrigger().unhover(); + await getTrigger().hover(); + await expect.element(getPanel()).toBeVisible(); + await getTrigger().unhover(); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + }); + + it('stays closed after a quick open-close click pair, before the hover delay elapses', async () => { + await renderHoverExample(); + + // The pointer enters and schedules a hover open; clicking must drop that pending open. + await getTrigger().click(); + await getTrigger().click(); + await wait(OPEN_DELAY * 2); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + }); + it('ignores touch pointers so a tap stays a plain click', async () => { await renderHoverExample(); @@ -714,3 +803,202 @@ describe('DsPopover.Panel id', () => { .toHaveAttribute('id', 'device-panel'); }); }); + +type CompositionProps = Pick; + +const PanelBody = () => ( + + Device details + + Edge router is online. + + +); + +const TooltipAroundTrigger = (props: CompositionProps) => ( + + + + + + + + +); + +const TooltipInsideTrigger = (props: CompositionProps) => ( + + + + + + + + +); + +const expectPanelBelowTrigger = () => + expect + .poll(() => { + const trigger = getTrigger().element().getBoundingClientRect(); + const panel = getPanel().element().getBoundingClientRect(); + + // Default side is bottom with an 8px gutter; a panel with no reference element sits at the viewport origin. + return Math.round(panel.top - trigger.bottom); + }) + .toBe(8); + +describe.each([ + ['DsTooltip > DsPopover.Trigger', TooltipAroundTrigger], + ['DsPopover.Trigger > DsTooltip', TooltipInsideTrigger], +])('DsPopover composed with DsTooltip (%s)', (_, Composition) => { + it('shows the tooltip on hover and swaps it for the panel on click', async () => { + await page.render(); + + await getTrigger().hover(); + await expect.element(page.getByRole('tooltip', { name: 'Preview' })).toBeVisible(); + + await getTrigger().click(); + + await expect.element(getPanel()).toBeVisible(); + await expect.element(page.getByRole('tooltip')).not.toBeInTheDocument(); + await expect.element(getTrigger()).toHaveAttribute('aria-haspopup', 'dialog'); + await expect.element(getTrigger()).toHaveAttribute('data-state', 'open'); + }); + + it('closes on a second trigger click and positions the panel against the trigger', async () => { + const onOpenChange = vi.fn(); + await page.render(); + + await getTrigger().click(); + await expect.element(getPanel()).toBeVisible(); + await expectPanelBelowTrigger(); + + // The trigger must stay excluded from outside-click dismissal, or this click closes then reopens. + await getTrigger().click(); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + expect(onOpenChange.mock.calls).toEqual([[true], [false]]); + }); + + it('positions and dismisses a panel that is open on mount', async () => { + await page.render(); + + await expect.element(getPanel()).toBeVisible(); + await expectPanelBelowTrigger(); + + await getTrigger().click(); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + }); + + it('returns focus to the trigger when Escape closes the panel', async () => { + await page.render(); + + await getTrigger().click(); + await expect.element(getPanel()).toBeVisible(); + + await userEvent.keyboard('{Escape}'); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + await expect.element(getTrigger()).toHaveFocus(); + }); +}); + +describe('DsPopover ref forwarding', () => { + it('forwards a ref through DsPopover.Trigger to the trigger element', async () => { + const ref = createRef(); + + await page.render( + + + + + + , + ); + + expect(ref.current).toBe(getTrigger().element()); + }); + + it('forwards a ref through DsPopover.Anchor while still positioning against it', async () => { + const ref = createRef(); + + await page.render( + + +
+ Field + + + +
+ + + , + ); + + const anchor = page.getByTestId('anchor'); + expect(ref.current).toBe(anchor.element()); + + await expect + .poll(() => Math.round(getPanel().element().getBoundingClientRect().left)) + .toBe(Math.round(anchor.element().getBoundingClientRect().left)); + }); +}); + +const CloseExample = ({ + actions = , + ...props +}: Pick & { actions?: ReactNode }) => ( + + + + + + Device details + + Edge router is online. + + + +); + +describe('DsPopover.CloseTrigger', () => { + it('closes the panel and returns focus to the trigger', async () => { + await page.render(); + + await getTrigger().click(); + await expect.element(getPanel()).toBeVisible(); + + await getPanel().getByRole('button', { name: 'Close' }).click(); + + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + await expect.element(getTrigger()).toHaveFocus(); + }); + + it('uses the locale for its accessible name and is not a toggle button', async () => { + await page.render(} />); + + await getTrigger().click(); + + const close = getPanel().getByRole('button', { name: 'Dismiss' }); + await expect.element(close).toBeVisible(); + await expect.element(close).not.toHaveAttribute('aria-pressed'); + }); + + it('asks a controlled parent to close', async () => { + const onOpenChange = vi.fn(); + await page.render(); + + await getPanel().getByRole('button', { name: 'Close' }).click(); + + expect(onOpenChange).toHaveBeenCalledWith(false); + }); + + it('keeps header actions out of the panel accessible name', async () => { + await page.render(); + + await expect.element(page.getByRole('dialog', { name: 'Device details', exact: true })).toBeVisible(); + }); +}); diff --git a/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx b/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx index ecf62a8ec..65c6221ae 100644 --- a/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx +++ b/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx @@ -1,12 +1,33 @@ +import type { Ref } from 'react'; import { Popover } from '@ark-ui/react/popover'; +import { mergeProps } from '@ark-ui/react/utils'; import { useHoverTriggerProps } from '../../ds-popover.hover-intent'; import type { DsPopoverTriggerProps } from '../../ds-popover.types'; -export const PopoverTrigger = ({ children, className }: DsPopoverTriggerProps) => { +export const PopoverTrigger = ({ + children, + className, + style, + ref, + ...injectedProps +}: DsPopoverTriggerProps) => { const hoverProps = useHoverTriggerProps(); return ( - + >(hoverProps, injectedProps)} + // Keep the popover's markers when an outer `asChild` trigger (e.g. DsTooltip) injects its + // own: zag falls back to finding the trigger by them once that wrapper's `id` wins. + data-scope={undefined} + data-part={undefined} + data-state={undefined} + className={className} + style={style} + ref={ref as Ref} + > {children} ); diff --git a/packages/design-system/src/components/ds-popover/ds-popover.hover-intent.ts b/packages/design-system/src/components/ds-popover/ds-popover.hover-intent.ts index f1b73117e..86297fa49 100644 --- a/packages/design-system/src/components/ds-popover/ds-popover.hover-intent.ts +++ b/packages/design-system/src/components/ds-popover/ds-popover.hover-intent.ts @@ -1,4 +1,4 @@ -import type { FocusEvent, PointerEvent } from 'react'; +import type { FocusEvent, MouseEvent, PointerEvent } from 'react'; import { usePopoverContext } from '@ark-ui/react/popover'; import { useDsPopoverContext } from './ds-popover.context'; import { isHoverPointer } from './ds-popover.utils'; @@ -14,10 +14,21 @@ export interface HoverIntent { consumeFocusRestore: () => boolean; /** Owns one shared timer */ schedule: (action: () => void, delay: number) => void; + cancel: () => void; + /** A pinned panel was opened, or kept open, by a click — pointer leave no longer closes it. */ + isPinned: () => boolean; + setPinned: (pinned: boolean) => void; } const useHoverIntent = () => useDsPopoverContext().hoverIntent; +const scheduleClose = (intent: HoverIntent, setOpen: (open: boolean) => void) => + intent.schedule(() => { + if (!intent.isPinned()) { + setOpen(false); + } + }, intent.closeDelay); + export const useHoverIntentProps = () => { const intent = useHoverIntent(); const { setOpen } = usePopoverContext(); @@ -26,39 +37,80 @@ export const useHoverIntentProps = () => { return undefined; } - const onPointerIntent = (event: PointerEvent, open: boolean, delay: number) => { - if (!isHoverPointer(event.pointerType)) { - return; - } - - intent.schedule(() => setOpen(open), delay); - }; - return { - onPointerEnter: (event: PointerEvent) => onPointerIntent(event, true, intent.openDelay), - onPointerLeave: (event: PointerEvent) => onPointerIntent(event, false, intent.closeDelay), + // Reaching the panel only cancels a pending close; it never (re)opens it. + onPointerEnter: (event: PointerEvent) => { + if (isHoverPointer(event.pointerType)) { + intent.cancel(); + } + }, + onPointerLeave: (event: PointerEvent) => { + if (isHoverPointer(event.pointerType)) { + scheduleClose(intent, setOpen); + } + }, }; }; export const useHoverTriggerProps = () => { - const pointerProps = useHoverIntentProps(); const intent = useHoverIntent(); - const { setOpen } = usePopoverContext(); + const { open, setOpen } = usePopoverContext(); if (!intent) { return undefined; } + const openUnpinned = () => { + intent.setPinned(false); + setOpen(true); + }; + return { - ...pointerProps, + onPointerEnter: (event: PointerEvent) => { + if (!isHoverPointer(event.pointerType)) { + return; + } + + if (open) { + intent.cancel(); + return; + } + + intent.schedule(openUnpinned, intent.openDelay); + }, + onPointerLeave: (event: PointerEvent) => { + if (isHoverPointer(event.pointerType)) { + scheduleClose(intent, setOpen); + } + }, onFocus: (event: FocusEvent) => { // Closing with focus inside the panel makes Ark re-focus the trigger if (intent.consumeFocusRestore()) { return; } - if (event.target.matches(':focus-visible')) { - setOpen(true); + if (!event.target.matches(':focus-visible')) { + return; + } + + intent.cancel(); + + if (!open) { + openUnpinned(); + } + }, + // Runs before Ark's toggle, which skips a default-prevented click. + onClick: (event: MouseEvent) => { + intent.cancel(); + + if (!open) { + intent.setPinned(true); + return; + } + + if (!intent.isPinned()) { + event.preventDefault(); + intent.setPinned(true); } }, }; diff --git a/packages/design-system/src/components/ds-popover/ds-popover.module.scss b/packages/design-system/src/components/ds-popover/ds-popover.module.scss index 686e9a812..91c6d5934 100644 --- a/packages/design-system/src/components/ds-popover/ds-popover.module.scss +++ b/packages/design-system/src/components/ds-popover/ds-popover.module.scss @@ -38,6 +38,14 @@ margin: 0; } +.headerActions { + display: inline-flex; + align-items: center; + flex-shrink: 0; + gap: var(--3xs); + margin-inline-start: auto; +} + .content { display: flex; flex-direction: column; diff --git a/packages/design-system/src/components/ds-popover/ds-popover.stories.tsx b/packages/design-system/src/components/ds-popover/ds-popover.stories.tsx index f43483e39..6e3f94c37 100644 --- a/packages/design-system/src/components/ds-popover/ds-popover.stories.tsx +++ b/packages/design-system/src/components/ds-popover/ds-popover.stories.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react'; import type { Meta, StoryObj } from '@storybook/react-vite'; import { DsButtonV3 } from '../ds-button-v3'; import { DsIcon } from '../ds-icon'; @@ -5,6 +6,7 @@ import { DsAvatar } from '../ds-avatar'; import { DsDivider } from '../ds-divider'; import { DsStatusBadgeV2 } from '../ds-status-badge-v2'; import { DsStack } from '../ds-stack'; +import { DsTooltip } from '../ds-tooltip'; import { DsTypography } from '../ds-typography'; import { DsPopover } from './ds-popover'; import { popoverAligns, popoverOpenTriggers, popoverSides } from './ds-popover.types'; @@ -63,7 +65,10 @@ export const WithContentItemsAndCTA: Story = { Release lock - }> + } + actions={} + > Release lock @@ -142,8 +147,12 @@ export const Legacy: Story = { /** * `openOn="hover"` layers pointer intent on top of the click behavior: the panel * opens after `openDelay`, survives the pointer crossing the `gutter` gap onto the - * panel itself, and closes `closeDelay` after the pointer leaves both. Click and - * keyboard activation still toggle, so touch devices keep working. + * panel itself, and closes `closeDelay` after the pointer leaves both. + * + * A click pins the panel: clicking the trigger of a hover-opened panel keeps it open, + * and a click-opened panel survives the pointer leaving. It then closes on Escape, an + * outside click, `DsPopover.CloseTrigger`, or another trigger click. Touch devices + * never hover, so a tap opens and a second tap closes. * * Under `openOn="hover"` the panel deliberately does not take focus on open — tab * from the trigger to reach the links inside. @@ -237,3 +246,49 @@ export const CustomAnchor: Story = { ), }; + +/** + * A tooltip and a popover can share one trigger: nest `DsTooltip` inside + * `DsPopover.Trigger`, around the trigger element. Hover shows a short preview; a click + * opens the full panel. Disable the tooltip while the panel is open so it does not + * reappear over it, and give the panel a `DsPopover.CloseTrigger` in the header `actions`. + */ +export const WithTooltip: Story = { + parameters: { docs: { source: { type: 'code' } } }, + render: function Render(args) { + const [open, setOpen] = useState(false); + + return ( + + + + + Provision edge + + + + + } + actions={} + > + Provision edge + + + }> + Provisions a new edge device and attaches it to the selected site, including the full + description that the preview truncates. + + + 2.3.4 (latest) + + + + Open in catalog + + + + + ); + }, +}; diff --git a/packages/design-system/src/components/ds-popover/ds-popover.tsx b/packages/design-system/src/components/ds-popover/ds-popover.tsx index 5571c1c6f..fd0b85f5f 100644 --- a/packages/design-system/src/components/ds-popover/ds-popover.tsx +++ b/packages/design-system/src/components/ds-popover/ds-popover.tsx @@ -1,7 +1,8 @@ -import { useEffect, useLayoutEffect, useRef, useState, type FocusEvent } from 'react'; +import { useEffect, useLayoutEffect, useRef, useState, type FocusEvent, type Ref } from 'react'; import { Popover, type PopoverRootProps } from '@ark-ui/react/popover'; import { Portal } from '@ark-ui/react/portal'; import classNames from 'classnames'; +import { DsButtonV3 } from '../ds-button-v3'; import { DsStack } from '../ds-stack'; import { DsTypography } from '../ds-typography'; import { PopoverTrigger } from './components/popover-trigger'; @@ -12,9 +13,11 @@ import { useHoverIntentProps, } from './ds-popover.hover-intent'; import { invokeCloseAutoFocus, toFocusEl, toPlacement } from './ds-popover.utils'; +import { mergeRefs } from '../../utils/merge-refs'; import styles from './ds-popover.module.scss'; import type { DsPopoverAnchorProps, + DsPopoverCloseTriggerProps, DsPopoverContentItemProps, DsPopoverContentProps, DsPopoverFooterProps, @@ -26,6 +29,7 @@ import type { const DEFAULT_PANEL_WIDTH = 400; const MATCHED_PANEL_WIDTH = 'var(--reference-width)'; +const DEFAULT_CLOSE_TRIGGER_LOCALE = Object.freeze({ close: 'Close' }); const DsPopoverRoot = ({ open, @@ -49,16 +53,25 @@ const DsPopoverRoot = ({ const timer = useRef | undefined>(undefined); const anchorRef = useRef(null); const restoringFocus = useRef(false); + const pinned = useRef(false); const [contentId, setContentId] = useState(); const [focusInPanel, setFocusInPanel] = useState(false); useEffect(() => () => clearTimeout(timer.current), []); + const cancel = () => clearTimeout(timer.current); + const schedule = (action: () => void, delay: number) => { - clearTimeout(timer.current); + cancel(); timer.current = setTimeout(action, delay); }; + const isPinned = () => pinned.current; + + const setPinned = (next: boolean) => { + pinned.current = next; + }; + const isHover = openOn === 'hover'; const consumeFocusRestore = () => { @@ -86,7 +99,16 @@ const DsPopoverRoot = ({ registerAnchor, registerContentId: setContentId, hoverIntent: isHover - ? { openDelay, closeDelay, schedule, setFocusInPanel, consumeFocusRestore } + ? { + openDelay, + closeDelay, + schedule, + cancel, + isPinned, + setPinned, + setFocusInPanel, + consumeFocusRestore, + } : null, }} > @@ -103,7 +125,9 @@ const DsPopoverRoot = ({ placement: toPlacement(side, align), gutter, sameWidth: matchAnchorWidth, - ...(getAnchorElement ? { getAnchorElement } : {}), + // Position from the registered Anchor element rather than zag's id lookup: an outer + // `asChild` wrapper around `DsPopover.Anchor` replaces that id. `null` falls back to the trigger. + getAnchorElement: () => getAnchorElement?.() ?? anchorRef.current, }} initialFocusEl={initialFocusEl} finalFocusEl={finalFocusEl} @@ -121,6 +145,12 @@ const DsPopoverRoot = ({ : undefined } onOpenChange={(details) => { + if (!details.open) { + // Any close ends the pin and drops a pending hover open, so it cannot reopen the panel. + cancel(); + setPinned(false); + } + if (!details.open && focusInPanel) { restoringFocus.current = true; } @@ -138,11 +168,17 @@ const DsPopoverRoot = ({ ); }; -const DsPopoverAnchor = ({ children, className }: DsPopoverAnchorProps) => { +const DsPopoverAnchor = ({ children, className, style, ref, ...injectedProps }: DsPopoverAnchorProps) => { const { registerAnchor } = useDsPopoverContext(); return ( - + (registerAnchor, ref as Ref)} + > {children} ); @@ -193,7 +229,7 @@ const DsPopoverPanel = ({ ); }; -const DsPopoverHeader = ({ icon, className, style, children }: DsPopoverHeaderProps) => ( +const DsPopoverHeader = ({ icon, actions, className, style, children }: DsPopoverHeaderProps) => (
{icon && {icon}} @@ -201,9 +237,26 @@ const DsPopoverHeader = ({ icon, className, style, children }: DsPopoverHeaderPr {children} + {actions &&
{actions}
}
); +const DsPopoverCloseTrigger = ({ className, style, ref, locale }: DsPopoverCloseTriggerProps) => ( + + + +); + const DsPopoverContent = ({ className, style, children }: DsPopoverContentProps) => (
{children} @@ -273,6 +326,7 @@ DsPopoverRoot.displayName = 'DsPopover.Root'; DsPopoverAnchor.displayName = 'DsPopover.Anchor'; DsPopoverPanel.displayName = 'DsPopover.Panel'; DsPopoverHeader.displayName = 'DsPopover.Header'; +DsPopoverCloseTrigger.displayName = 'DsPopover.CloseTrigger'; DsPopoverContent.displayName = 'DsPopover.Content'; DsPopoverContentItem.displayName = 'DsPopover.ContentItem'; DsPopoverFooter.displayName = 'DsPopover.Footer'; @@ -283,6 +337,7 @@ export const DsPopover = Object.assign(DsPopoverLegacy, { Anchor: DsPopoverAnchor, Panel: DsPopoverPanel, Header: DsPopoverHeader, + CloseTrigger: DsPopoverCloseTrigger, Content: DsPopoverContent, ContentItem: DsPopoverContentItem, Footer: DsPopoverFooter, diff --git a/packages/design-system/src/components/ds-popover/ds-popover.types.ts b/packages/design-system/src/components/ds-popover/ds-popover.types.ts index 2fe467a48..4174cc3bb 100644 --- a/packages/design-system/src/components/ds-popover/ds-popover.types.ts +++ b/packages/design-system/src/components/ds-popover/ds-popover.types.ts @@ -1,4 +1,5 @@ import type { CSSProperties, ReactNode, Ref } from 'react'; +import type { DsAsChildTriggerProps } from '../../utils/as-child-trigger-props'; export const popoverSides = ['top', 'right', 'bottom', 'left'] as const; export type DsPopoverSide = (typeof popoverSides)[number]; @@ -46,7 +47,9 @@ export interface DsPopoverRootProps { */ restoreFocus?: boolean; /** - * Trigger open method - click / hover + * Trigger open method - click / hover. + * Under `'hover'`, a click pins the panel: it stays open after the pointer leaves + * until Escape, an outside click, `DsPopover.CloseTrigger`, or another trigger click. * @default 'click' */ openOn?: DsPopoverOpenTrigger; @@ -94,16 +97,25 @@ export interface DsPopoverRootProps { onOpenChange?: (open: boolean) => void; } -export interface DsPopoverTriggerProps { +/** + * Also forwards `ref` and props injected by an outer `asChild` trigger (e.g. `DsTooltip`), + * so the trigger element can be shared. Prefer `DsPopover.Trigger > DsTooltip > element`. + */ +export interface DsPopoverTriggerProps extends DsAsChildTriggerProps { /** Single focusable element that toggles the popover. */ children: ReactNode; className?: string; + style?: CSSProperties; } -export interface DsPopoverAnchorProps { +/** + * Also forwards `ref` and props injected by an outer `asChild` trigger. + */ +export interface DsPopoverAnchorProps extends DsAsChildTriggerProps { /** Single element the panel positions against. Wraps the trigger when they share a field. */ children: ReactNode; className?: string; + style?: CSSProperties; } export interface DsPopoverPanelProps { @@ -123,12 +135,30 @@ export interface DsPopoverPanelProps { export interface DsPopoverHeaderProps { /** Leading visual (e.g. a colored task-type icon) rendered before the title. */ icon?: ReactNode; + /** + * Trailing controls after the title, e.g. ``. + * Kept out of the title, so they are not part of the popover's accessible name. + */ + actions?: ReactNode; className?: string; style?: CSSProperties; /** Title content, exposed as the popover's accessible name. */ children: ReactNode; } +export interface DsPopoverCloseTriggerProps { + className?: string; + style?: CSSProperties; + ref?: Ref; + locale?: { + /** + * Accessible name of the icon-only close button. + * @default 'Close' + */ + close?: string; + }; +} + export interface DsPopoverContentProps { className?: string; style?: CSSProperties; diff --git a/packages/design-system/src/components/ds-tooltip/__tests__/__snapshots__/ds-tooltip.docs.snap b/packages/design-system/src/components/ds-tooltip/__tests__/__snapshots__/ds-tooltip.docs.snap index 2e08f220b..71d7959dd 100644 --- a/packages/design-system/src/components/ds-tooltip/__tests__/__snapshots__/ds-tooltip.docs.snap +++ b/packages/design-system/src/components/ds-tooltip/__tests__/__snapshots__/ds-tooltip.docs.snap @@ -110,4 +110,50 @@ const CustomWidthWithEllipsis = () => ; \ No newline at end of file + }}>; + +## Controlled + +### Show code +{ + args: { + content: 'Opened from outside the trigger.' + }, + parameters: { + docs: { + source: { + type: 'code' + } + } + }, + render: function Render(args) { + const [open, setOpen] = useState(false); + return + setOpen(!open)}> + {open ? 'Hide tooltip' : 'Show tooltip'} + + + + + ; + } +} + +### MCP manifest +const Controlled = () => { + const [open, setOpen] = useState(false); + + return ( + + setOpen(!open)}> + {open ? 'Hide tooltip' : 'Show tooltip'} + + + + + + ); +}; \ No newline at end of file diff --git a/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx b/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx index 728fcd923..0f1bdbaaa 100644 --- a/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx +++ b/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx @@ -1,6 +1,8 @@ +import { createRef } from 'react'; import { describe, expect, it, vi } from 'vitest'; import { page } from 'vitest/browser'; import DsTooltip from '../ds-tooltip'; +import { DsPopover } from '../../ds-popover'; describe('DsTooltip', () => { it('should show tooltip on hover', async () => { @@ -167,3 +169,97 @@ describe('DsTooltip', () => { expect(onOpen).toHaveBeenCalledOnce(); }); }); + +describe('DsTooltip controlled state', () => { + it('shows the tooltip from a controlled open prop without hover', async () => { + await page.render( + + + , + ); + + await expect.element(page.getByRole('tooltip')).toBeVisible(); + }); + + it('reports hover through onOpenChange but stays hidden while controlled closed', async () => { + const onOpenChange = vi.fn(); + + await page.render( + + + , + ); + + await page.getByRole('button', { name: 'Trigger' }).hover(); + + await vi.waitFor(() => expect(onOpenChange).toHaveBeenCalledWith(true)); + await expect.element(page.getByRole('tooltip')).not.toBeInTheDocument(); + }); + + it('fires onOpenChange on hover and unhover when uncontrolled', async () => { + const onOpenChange = vi.fn(); + + await page.render( + + + , + ); + + const trigger = page.getByRole('button', { name: 'Trigger' }); + await trigger.hover(); + await vi.waitFor(() => expect(onOpenChange).toHaveBeenCalledWith(true)); + + await trigger.unhover(); + await vi.waitFor(() => expect(onOpenChange).toHaveBeenLastCalledWith(false)); + }); + + it('opens on mount with defaultOpen', async () => { + await page.render( + + + , + ); + + await expect.element(page.getByRole('tooltip')).toBeVisible(); + }); +}); + +describe('DsTooltip forwarding', () => { + it.each([ + ['with content', 'Tooltip text'], + ['without content', undefined], + ])('forwards ref to the trigger element %s', async (_, content) => { + const ref = createRef(); + + await page.render( + + + , + ); + + expect(ref.current).toBe(page.getByRole('button', { name: 'Trigger' }).element()); + }); + + it('forwards props injected by an outer trigger when content is undefined', async () => { + const { container } = await page.render( + + + + + + + + Details + + , + ); + + const trigger = page.getByRole('button', { name: 'Trigger' }); + await expect.element(trigger).toHaveAttribute('aria-haspopup', 'dialog'); + // No wrapper element: the button is the trigger itself. + expect(container.firstElementChild).toBe(trigger.element()); + + await trigger.click(); + await expect.element(page.getByRole('dialog', { name: 'Details' })).toBeVisible(); + }); +}); diff --git a/packages/design-system/src/components/ds-tooltip/ds-tooltip.stories.tsx b/packages/design-system/src/components/ds-tooltip/ds-tooltip.stories.tsx index 32ebc6ddf..5d956d517 100644 --- a/packages/design-system/src/components/ds-tooltip/ds-tooltip.stories.tsx +++ b/packages/design-system/src/components/ds-tooltip/ds-tooltip.stories.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react'; import type { Meta, StoryObj } from '@storybook/react-vite'; import DsTooltip from './ds-tooltip'; import { tooltipPlacements } from './ds-tooltip.types'; @@ -37,6 +38,10 @@ const meta: Meta = { control: 'object', description: 'Element that triggers the tooltip on hover', }, + open: { table: { disable: true } }, + defaultOpen: { control: 'boolean' }, + onOpenChange: { table: { disable: true } }, + ref: { table: { disable: true } }, }, }; @@ -124,3 +129,29 @@ export const CustomWidthWithEllipsis: Story = { }, }, }; + +/** + * Drive the tooltip from your own state with `open` + `onOpenChange` — for example to + * reveal it from elsewhere, or to keep it closed while the trigger is being dragged. + * Hover and focus still report their intent through `onOpenChange`. + */ +export const Controlled: Story = { + args: { + content: 'Opened from outside the trigger.', + }, + parameters: { docs: { source: { type: 'code' } } }, + render: function Render(args) { + const [open, setOpen] = useState(false); + + return ( + + setOpen(!open)}> + {open ? 'Hide tooltip' : 'Show tooltip'} + + + + + + ); + }, +}; diff --git a/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx b/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx index f9757bc7d..8d6162042 100644 --- a/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx +++ b/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx @@ -1,4 +1,5 @@ -import { type FC, isValidElement } from 'react'; +import { isValidElement, type Ref } from 'react'; +import { ark } from '@ark-ui/react/factory'; import { Tooltip } from '@ark-ui/react/tooltip'; import { Portal } from '@ark-ui/react/portal'; import classNames from 'classnames'; @@ -9,7 +10,21 @@ const OPEN_DELAY_MS = 200; const CLOSE_DELAY_MS = 0; const TOOLTIP_GUTTER_PX = 0; -const DsTooltip: FC = ({ +// Ark types the trigger ref as a button; the slotted element may be any HTMLElement. +type TriggerRef = Ref; + +/** + * The id the trigger element ends up with. The child's own `id` wins Ark's `asChild` + * merge, then an id injected by an outer trigger. Sharing it keeps both machines + * pointed at the same element. + */ +const getTriggerId = (children: DsTooltipProps['children'], injectedId: string | undefined) => { + const childId = isValidElement<{ id?: string }>(children) ? children.props.id : undefined; + + return childId ?? injectedId; +}; + +const DsTooltip = ({ content, children, placement = 'top', @@ -18,14 +33,34 @@ const DsTooltip: FC = ({ openDelay = OPEN_DELAY_MS, closeDelay = CLOSE_DELAY_MS, getAnchorRect, + open, + defaultOpen, slotProps, -}) => { + onOpenChange, + ref, + ...triggerProps +}: DsTooltipProps) => { if (content === undefined) { - return children; + // Still a transparent slot, so props from an outer `asChild` trigger reach the element. + return isValidElement(children) ? ( + + {children} + + ) : ( + children + ); } + const triggerId = getTriggerId(children, triggerProps.id); + return ( = ({ positioning={{ placement, gutter: TOOLTIP_GUTTER_PX, getAnchorRect: getAnchorRect ?? undefined }} lazyMount unmountOnExit + onOpenChange={(details) => onOpenChange?.(details.open)} > - {children} + + {children} + TooltipAnchorRect | null; + /** + * Controlled open state. Pair with `onOpenChange`. + */ + open?: boolean; + /** + * Initial open state when uncontrolled. + */ + defaultOpen?: boolean; /** * Props forwarded to nested sub-components. */ @@ -78,4 +91,8 @@ export interface DsTooltipProps { style?: CSSProperties; }; }; + /** + * Fires when the tooltip requests to open or close (hover, focus, Escape, trigger click). + */ + onOpenChange?: (open: boolean) => void; } diff --git a/packages/design-system/src/components/ds-tree/ds-tree.types.ts b/packages/design-system/src/components/ds-tree/ds-tree.types.ts index 6998eab07..bdf041389 100644 --- a/packages/design-system/src/components/ds-tree/ds-tree.types.ts +++ b/packages/design-system/src/components/ds-tree/ds-tree.types.ts @@ -1,17 +1,8 @@ -import type { - AriaAttributes, - CSSProperties, - FocusEventHandler, - KeyboardEventHandler, - MouseEvent, - MouseEventHandler, - PointerEventHandler, - ReactNode, - Ref, -} from 'react'; +import type { CSSProperties, MouseEvent, ReactNode, Ref } from 'react'; import type { TreeView as ArkTreeView } from '@ark-ui/react/tree-view'; import type { IconType } from '../ds-icon'; import type { FilterStatus } from '../ds-filter-status-icon'; +import type { DsAsChildTriggerProps } from '../../utils/as-child-trigger-props'; export interface DsTreeNode { /** @@ -163,22 +154,7 @@ export type DsTreeTreeProps = DsTreeBasePropsWithChildren; * the element it wraps. Row parts forward these so wrapping a row actually wires up * instead of silently doing nothing. */ -export interface DsTreeRowTriggerProps { - id?: string; - ref?: Ref; - tabIndex?: number; - 'aria-haspopup'?: AriaAttributes['aria-haspopup']; - 'aria-expanded'?: AriaAttributes['aria-expanded']; - 'aria-controls'?: string; - 'data-state'?: string; - onClick?: MouseEventHandler; - onPointerDown?: PointerEventHandler; - onPointerEnter?: PointerEventHandler; - onPointerLeave?: PointerEventHandler; - onFocus?: FocusEventHandler; - onBlur?: FocusEventHandler; - onKeyDown?: KeyboardEventHandler; -} +export type DsTreeRowTriggerProps = DsAsChildTriggerProps; export type DsTreeBranchProps = DsTreeBasePropsWithChildren & DsTreeRowTriggerProps; diff --git a/packages/design-system/src/utils/as-child-trigger-props.ts b/packages/design-system/src/utils/as-child-trigger-props.ts new file mode 100644 index 000000000..553e572ca --- /dev/null +++ b/packages/design-system/src/utils/as-child-trigger-props.ts @@ -0,0 +1,35 @@ +import type { + AriaAttributes, + FocusEventHandler, + KeyboardEventHandler, + MouseEventHandler, + PointerEventHandler, + Ref, +} from 'react'; + +/** + * Props a wrapping `asChild` trigger (`DsPopover.Trigger`, `DsTooltip`, `DsDropdownMenu.Trigger`) + * injects onto the element it wraps. Components that sit inside such a trigger forward these + * so the wrapper actually wires up instead of silently doing nothing. + */ +export interface DsAsChildTriggerProps { + id?: string; + ref?: Ref; + dir?: 'ltr' | 'rtl'; + tabIndex?: number; + 'aria-haspopup'?: AriaAttributes['aria-haspopup']; + 'aria-expanded'?: AriaAttributes['aria-expanded']; + 'aria-controls'?: string; + 'aria-describedby'?: string; + 'data-state'?: string; + onClick?: MouseEventHandler; + onPointerDown?: PointerEventHandler; + onPointerEnter?: PointerEventHandler; + onPointerLeave?: PointerEventHandler; + onPointerMove?: PointerEventHandler; + onPointerOver?: PointerEventHandler; + onPointerCancel?: PointerEventHandler; + onFocus?: FocusEventHandler; + onBlur?: FocusEventHandler; + onKeyDown?: KeyboardEventHandler; +} From 43208990d9605b8fd24a9c70b2c351f69670704d Mon Sep 17 00:00:00 2001 From: Ihor Romanchuk Date: Tue, 29 Sep 2026 16:26:39 +0200 Subject: [PATCH 2/5] chore(design-system): shorten AR-75956 changeset --- .changeset/popover-tooltip-composition.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.changeset/popover-tooltip-composition.md b/.changeset/popover-tooltip-composition.md index 55e83bbe5..d1dbdc5b9 100644 --- a/.changeset/popover-tooltip-composition.md +++ b/.changeset/popover-tooltip-composition.md @@ -2,9 +2,4 @@ '@drivenets/design-system': minor --- -Make `DsPopover` and `DsTooltip` controllable and composable on a shared trigger: - -- `DsTooltip`: add controlled `open` / `defaultOpen` / `onOpenChange(open)`. -- `DsTooltip`, `DsPopover.Trigger` and `DsPopover.Anchor` forward `ref` and props injected by an outer `asChild` trigger, so a tooltip and a popover can share one trigger element (either nesting order; `DsPopover.Trigger > DsTooltip > element` is recommended). -- Add `DsPopover.CloseTrigger` (icon-only close button, `locale={{ close }}`, default `'Close'`) and a `DsPopover.Header` `actions` trailing slot to place it. -- `DsPopover.Root` `openOn="hover"`: a click pins the panel — clicking a hover-opened panel keeps it open, and a click-opened panel survives the pointer leaving until Escape, an outside click, the close button, or another trigger click. +Make `DsPopover` and `DsTooltip` controllable and composable on a shared trigger From 6d18b1ce8bf4b9e9a15237861c70bfbf8ba509b0 Mon Sep 17 00:00:00 2001 From: Ihor Romanchuk Date: Wed, 30 Sep 2026 12:18:40 +0200 Subject: [PATCH 3/5] fix(design-system): let DsTooltip reopen on first hover after being disabled [AR-75956] Zag's disabled prop also drops pointer leave, so a tooltip disabled under the pointer kept its opened-by-pointer flag and ignored the next hover. Treat disabled as a controlled close so the machine keeps tracking the pointer. --- .../__tests__/ds-popover.browser.test.tsx | 40 ++++++++++++++++++- .../src/components/ds-tooltip/ds-tooltip.tsx | 16 ++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx b/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx index c5b686767..78a4b7bf2 100644 --- a/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx +++ b/packages/design-system/src/components/ds-popover/__tests__/ds-popover.browser.test.tsx @@ -1,4 +1,4 @@ -import { createRef, useRef, type ReactNode } from 'react'; +import { createRef, useRef, useState, type ReactNode } from 'react'; import { describe, expect, it, vi } from 'vitest'; import { page, userEvent } from 'vitest/browser'; import { DsTooltip } from '../../ds-tooltip'; @@ -857,6 +857,44 @@ const expectPanelBelowTrigger = () => }) .toBe(8); +// The WithTooltip story recipe: the tooltip is disabled while the panel is open. +const TooltipDisabledWhileOpen = () => { + const [open, setOpen] = useState(false); + + return ( +
+ + + + + + + + + +
+ ); +}; + +describe('DsPopover with a tooltip disabled while open', () => { + it('shows the tooltip on the first hover after the panel closes', async () => { + await page.render(); + + await getTrigger().hover(); + await expect.element(page.getByRole('tooltip', { name: 'Preview' })).toBeVisible(); + + await getTrigger().click(); + await expect.element(getPanel()).toBeVisible(); + + // The pointer leaves the trigger while the tooltip is disabled. + await page.getByRole('button', { name: 'Outside' }).click(); + await expect.element(page.getByText(/edge router is online/i)).not.toBeVisible(); + + await getTrigger().hover(); + await expect.element(page.getByRole('tooltip', { name: 'Preview' })).toBeVisible(); + }); +}); + describe.each([ ['DsTooltip > DsPopover.Trigger', TooltipAroundTrigger], ['DsPopover.Trigger > DsTooltip', TooltipInsideTrigger], diff --git a/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx b/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx index 8d6162042..32bf7a12b 100644 --- a/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx +++ b/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx @@ -53,22 +53,30 @@ const DsTooltip = ({ const triggerId = getTriggerId(children, triggerProps.id); + // Zag's own `disabled` also ignores pointer leave, so a tooltip disabled under the pointer + // keeps its "opened by this pointer" flag and skips the next hover. Treat `disabled` as a + // controlled close instead, so the machine keeps tracking the pointer. + const resolvedOpen = disabled ? false : open; + return ( onOpenChange?.(details.open)} + onOpenChange={(details) => { + if (!disabled) { + onOpenChange?.(details.open); + } + }} > {children} From 6d3428991a47c6b9d6e4ca053b6b91a6dc281bd8 Mon Sep 17 00:00:00 2001 From: Ihor Romanchuk Date: Wed, 30 Sep 2026 12:52:55 +0200 Subject: [PATCH 4/5] fix(design-system): find a tooltip-wrapped DsPopover trigger on older zag [AR-75956] zag 1.41.2 (allowed by the ^1.42 / ark ^5.37.2 ranges) matches data-ownedby exactly and does not merge it, so the fallback lookup cannot find a trigger whose id an outer DsTooltip replaced. Resolve ids.trigger lazily from an id the trigger registers in a layout effect, which runs before the Root machine's effects. --- .../popover-trigger/popover-trigger.tsx | 15 ++++++++++++--- .../components/ds-popover/ds-popover.context.ts | 3 +++ .../src/components/ds-popover/ds-popover.tsx | 14 ++++++++++++-- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx b/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx index 65c6221ae..47bf1d45a 100644 --- a/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx +++ b/packages/design-system/src/components/ds-popover/components/popover-trigger/popover-trigger.tsx @@ -1,6 +1,7 @@ -import type { Ref } from 'react'; +import { useLayoutEffect, type Ref } from 'react'; import { Popover } from '@ark-ui/react/popover'; import { mergeProps } from '@ark-ui/react/utils'; +import { useDsPopoverContext } from '../../ds-popover.context'; import { useHoverTriggerProps } from '../../ds-popover.hover-intent'; import type { DsPopoverTriggerProps } from '../../ds-popover.types'; @@ -12,6 +13,15 @@ export const PopoverTrigger = ({ ...injectedProps }: DsPopoverTriggerProps) => { const hoverProps = useHoverTriggerProps(); + const { registerTriggerId } = useDsPopoverContext(); + const injectedId = injectedProps.id; + + // Runs before the Root machine's own layout effect, so even a panel open on mount finds it. + useLayoutEffect(() => { + registerTriggerId(injectedId); + + return () => registerTriggerId(undefined); + }, [injectedId, registerTriggerId]); return ( >(hoverProps, injectedProps)} - // Keep the popover's markers when an outer `asChild` trigger (e.g. DsTooltip) injects its - // own: zag falls back to finding the trigger by them once that wrapper's `id` wins. + // Keep the popover's markers when an outer `asChild` trigger (e.g. DsTooltip) injects its own. data-scope={undefined} data-part={undefined} data-state={undefined} diff --git a/packages/design-system/src/components/ds-popover/ds-popover.context.ts b/packages/design-system/src/components/ds-popover/ds-popover.context.ts index c24cf6372..459128046 100644 --- a/packages/design-system/src/components/ds-popover/ds-popover.context.ts +++ b/packages/design-system/src/components/ds-popover/ds-popover.context.ts @@ -5,6 +5,8 @@ export interface DsPopoverContextValue { matchAnchorWidth: boolean; registerAnchor: (el: HTMLElement | null) => void; registerContentId: (id: string | undefined) => void; + /** An id an outer `asChild` wrapper put on the trigger element; zag looks the trigger up by it. */ + registerTriggerId: (id: string | undefined) => void; hoverIntent: HoverIntent | null; } @@ -12,6 +14,7 @@ export const DsPopoverContext = createContext({ matchAnchorWidth: false, registerAnchor: () => undefined, registerContentId: () => undefined, + registerTriggerId: () => undefined, hoverIntent: null, }); diff --git a/packages/design-system/src/components/ds-popover/ds-popover.tsx b/packages/design-system/src/components/ds-popover/ds-popover.tsx index 1702a639e..e5c41e5ff 100644 --- a/packages/design-system/src/components/ds-popover/ds-popover.tsx +++ b/packages/design-system/src/components/ds-popover/ds-popover.tsx @@ -1,4 +1,4 @@ -import { useEffect, useLayoutEffect, useRef, useState, type FocusEvent, type Ref } from 'react'; +import { useEffect, useId, useLayoutEffect, useRef, useState, type FocusEvent, type Ref } from 'react'; import { Popover, type PopoverRootProps } from '@ark-ui/react/popover'; import { Portal } from '@ark-ui/react/portal'; import classNames from 'classnames'; @@ -55,6 +55,8 @@ const DsPopoverRoot = ({ const restoringFocus = useRef(false); const pinned = useRef(false); const [contentId, setContentId] = useState(); + const ownTriggerId = useId(); + const injectedTriggerId = useRef(undefined); const [focusInPanel, setFocusInPanel] = useState(false); useEffect(() => () => clearTimeout(timer.current), []); @@ -98,6 +100,9 @@ const DsPopoverRoot = ({ matchAnchorWidth, registerAnchor, registerContentId: setContentId, + registerTriggerId: (id) => { + injectedTriggerId.current = id; + }, hoverIntent: isHover ? { openDelay, @@ -116,7 +121,12 @@ const DsPopoverRoot = ({ open={open} defaultOpen={defaultOpen} modal={modal} - ids={contentId ? { content: contentId } : undefined} + ids={{ + // Resolved on every lookup, so an id an outer wrapper (e.g. DsTooltip) puts on the + // trigger is found from the first effect, including a panel open on mount. + trigger: () => injectedTriggerId.current ?? ownTriggerId, + ...(contentId ? { content: contentId } : {}), + }} // Trigger focus is lost when open on hover // eslint-disable-next-line jsx-a11y/no-autofocus autoFocus={!isHover} From c0858eaecd5e3a7ec5328c3b973e1e78f045b4bc Mon Sep 17 00:00:00 2001 From: Ihor Romanchuk Date: Wed, 30 Sep 2026 14:16:47 +0200 Subject: [PATCH 5/5] fix(design-system): keep zag disabled on DsTooltip and replay the missed pointer leave [AR-75956] Forcing a controlled open={false} while disabled could leave the machine stuck in closing after an instant open, holding the shared visible-tooltip id. Pass zag's disabled through again and, once re-enabled, replay a pointer leave that happened while disabled so the next hover still opens. --- .../__tests__/ds-tooltip.browser.test.tsx | 36 +++++ .../src/components/ds-tooltip/ds-tooltip.tsx | 151 +++++++++++++----- 2 files changed, 146 insertions(+), 41 deletions(-) diff --git a/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx b/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx index 0f1bdbaaa..312ee7715 100644 --- a/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx +++ b/packages/design-system/src/components/ds-tooltip/__tests__/ds-tooltip.browser.test.tsx @@ -263,3 +263,39 @@ describe('DsTooltip forwarding', () => { await expect.element(page.getByRole('dialog', { name: 'Details' })).toBeVisible(); }); }); + +const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +describe('DsTooltip disabled', () => { + it('does not hold the shared visible-tooltip slot after the pointer passes over it', async () => { + const OPEN_DELAY = 600; + + await page.render( +
+ + + + + + +
, + ); + + const first = page.getByRole('button', { name: 'First' }); + await first.hover(); + await expect.element(page.getByRole('tooltip', { name: 'First' })).toBeVisible(); + + // Another tooltip is visible, so zag takes its instant-open path for the disabled one. + await page.getByRole('button', { name: 'Second' }).hover(); + await page.getByRole('button', { name: 'Second' }).unhover(); + await expect.element(page.getByRole('tooltip')).not.toBeInTheDocument(); + + // With the slot free, the next hover waits for the open delay again. + await first.hover(); + await wait(OPEN_DELAY / 3); + await expect + .element(page.getByRole('tooltip', { name: 'First' }), { timeout: 0 }) + .not.toBeInTheDocument(); + await expect.element(page.getByRole('tooltip', { name: 'First' })).toBeVisible(); + }); +}); diff --git a/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx b/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx index 32bf7a12b..b447b507a 100644 --- a/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx +++ b/packages/design-system/src/components/ds-tooltip/ds-tooltip.tsx @@ -1,9 +1,11 @@ -import { isValidElement, type Ref } from 'react'; +import { isValidElement, useEffect, useRef, type PointerEvent, type Ref } from 'react'; import { ark } from '@ark-ui/react/factory'; -import { Tooltip } from '@ark-ui/react/tooltip'; +import { Tooltip, useTooltip } from '@ark-ui/react/tooltip'; +import { mergeProps } from '@ark-ui/react/utils'; import { Portal } from '@ark-ui/react/portal'; import classNames from 'classnames'; import styles from './ds-tooltip.module.scss'; +import type { DsAsChildTriggerProps } from '../../utils/as-child-trigger-props'; import type { DsTooltipProps } from './ds-tooltip.types'; const OPEN_DELAY_MS = 200; @@ -24,61 +26,80 @@ const getTriggerId = (children: DsTooltipProps['children'], injectedId: string | return childId ?? injectedId; }; -const DsTooltip = ({ +type TooltipWithContentProps = Omit & { + ref: DsTooltipProps['ref']; + triggerProps: Omit; +}; + +const TooltipWithContent = ({ content, children, - placement = 'top', - disabled = false, - interactive = false, - openDelay = OPEN_DELAY_MS, - closeDelay = CLOSE_DELAY_MS, + placement, + disabled, + interactive, + openDelay, + closeDelay, getAnchorRect, open, defaultOpen, slotProps, onOpenChange, ref, - ...triggerProps -}: DsTooltipProps) => { - if (content === undefined) { - // Still a transparent slot, so props from an outer `asChild` trigger reach the element. - return isValidElement(children) ? ( - - {children} - - ) : ( - children - ); - } - + triggerProps, +}: TooltipWithContentProps) => { const triggerId = getTriggerId(children, triggerProps.id); + const leftWhileDisabled = useRef(false); + + const tooltip = useTooltip({ + ids: triggerId ? { trigger: triggerId } : undefined, + open, + defaultOpen, + disabled, + interactive, + openDelay, + closeDelay, + positioning: { placement, gutter: TOOLTIP_GUTTER_PX, getAnchorRect: getAnchorRect ?? undefined }, + onOpenChange: (details) => onOpenChange?.(details.open), + }); + + // Zag ignores pointer leave while disabled, so a tooltip disabled under the pointer keeps its + // "opened by this pointer" flag and would skip the next hover. Replay the missed leave once enabled. + const replayPointerLeave = tooltip.getTriggerProps().onPointerLeave; + + useEffect(() => { + if (disabled || !leftWhileDisabled.current) { + return; + } + + leftWhileDisabled.current = false; + replayPointerLeave?.({} as PointerEvent); + }, [disabled, replayPointerLeave]); - // Zag's own `disabled` also ignores pointer leave, so a tooltip disabled under the pointer - // keeps its "opened by this pointer" flag and skips the next hover. Treat `disabled` as a - // controlled close instead, so the machine keeps tracking the pointer. - const resolvedOpen = disabled ? false : open; + const trackDisabledLeave = { + onPointerEnter: () => { + leftWhileDisabled.current = false; + }, + onPointerLeave: () => { + if (disabled) { + leftWhileDisabled.current = true; + } + }, + }; return ( - { - if (!disabled) { - onOpenChange?.(details.open); - } - }} > - + >(trackDisabledLeave, triggerProps)} + > {children} @@ -94,7 +115,55 @@ const DsTooltip = ({ - + + ); +}; + +const DsTooltip = ({ + content, + children, + placement = 'top', + disabled = false, + interactive = false, + openDelay = OPEN_DELAY_MS, + closeDelay = CLOSE_DELAY_MS, + getAnchorRect, + open, + defaultOpen, + slotProps, + onOpenChange, + ref, + ...triggerProps +}: DsTooltipProps) => { + if (content === undefined) { + // Still a transparent slot, so props from an outer `asChild` trigger reach the element. + return isValidElement(children) ? ( + + {children} + + ) : ( + children + ); + } + + return ( + + {children} + ); };