diff --git a/.changeset/thin-places-check.md b/.changeset/thin-places-check.md new file mode 100644 index 000000000..8b4371715 --- /dev/null +++ b/.changeset/thin-places-check.md @@ -0,0 +1,5 @@ +--- +"bits-ui": patch +--- + +fix(Tooltip): dont eagerly start timer diff --git a/docs/src/routes/(docs)/sink/+page.svelte b/docs/src/routes/(docs)/sink/+page.svelte index a0e669472..6da4ddcff 100644 --- a/docs/src/routes/(docs)/sink/+page.svelte +++ b/docs/src/routes/(docs)/sink/+page.svelte @@ -1,5 +1,38 @@ - + + + (open = true)} + > + Hover Me & Then Click + + +
+ Tooltip Content +
+ + + + + + +

Dialog Content

+

+ Click "Close" to close dialog and hover tooltip again. The tooltip will not + appear. +

+ Close + + + + diff --git a/packages/bits-ui/src/lib/bits/tooltip/tooltip.svelte.ts b/packages/bits-ui/src/lib/bits/tooltip/tooltip.svelte.ts index d67f31bc4..6847594d3 100644 --- a/packages/bits-ui/src/lib/bits/tooltip/tooltip.svelte.ts +++ b/packages/bits-ui/src/lib/bits/tooltip/tooltip.svelte.ts @@ -46,13 +46,9 @@ export class TooltipProviderState { constructor(opts: TooltipProviderStateOpts) { this.opts = opts; - this.#timerFn = new TimeoutFn( - () => { - this.isOpenDelayed = true; - }, - this.opts.skipDelayDuration.current, - { immediate: false } - ); + this.#timerFn = new TimeoutFn(() => { + this.isOpenDelayed = true; + }, this.opts.skipDelayDuration.current); } #startTimer = () => { @@ -143,14 +139,10 @@ export class TooltipRootState { constructor(opts: TooltipRootStateOpts, provider: TooltipProviderState) { this.opts = opts; this.provider = provider; - this.#timerFn = new TimeoutFn( - () => { - this.#wasOpenDelayed = true; - this.opts.open.current = true; - }, - this.delayDuration ?? 0, - { immediate: false } - ); + this.#timerFn = new TimeoutFn(() => { + this.#wasOpenDelayed = true; + this.opts.open.current = true; + }, this.delayDuration ?? 0); new OpenChangeComplete({ open: this.opts.open, @@ -164,14 +156,10 @@ export class TooltipRootState { () => this.delayDuration, () => { if (this.delayDuration === undefined) return; - this.#timerFn = new TimeoutFn( - () => { - this.#wasOpenDelayed = true; - this.opts.open.current = true; - }, - this.delayDuration, - { immediate: false } - ); + this.#timerFn = new TimeoutFn(() => { + this.#wasOpenDelayed = true; + this.opts.open.current = true; + }, this.delayDuration); } ); @@ -183,7 +171,8 @@ export class TooltipRootState { } else { this.provider.onClose(this); } - } + }, + { lazy: true } ); } diff --git a/packages/bits-ui/src/lib/internal/timeout-fn.ts b/packages/bits-ui/src/lib/internal/timeout-fn.ts index 854ae89cd..8d5b1aa1a 100644 --- a/packages/bits-ui/src/lib/internal/timeout-fn.ts +++ b/packages/bits-ui/src/lib/internal/timeout-fn.ts @@ -1,38 +1,18 @@ import { onDestroyEffect } from "svelte-toolbelt"; import type { AnyFn } from "./types.js"; -import { BROWSER } from "esm-env"; - -type TimeoutFnOptions = { - /** - * Start the timer immediate after calling this function - * - * @default true - */ - immediate?: boolean; -}; - -const defaultOpts: TimeoutFnOptions = { - immediate: true, -}; export class TimeoutFn { - readonly #opts: TimeoutFnOptions; readonly #interval: number; readonly #cb: T; #timer: number | null = null; - constructor(cb: T, interval: number, opts: TimeoutFnOptions = {}) { + constructor(cb: T, interval: number) { this.#cb = cb; this.#interval = interval; - this.#opts = { ...defaultOpts, ...opts }; this.stop = this.stop.bind(this); this.start = this.start.bind(this); - if (this.#opts.immediate && BROWSER) { - this.start(); - } - onDestroyEffect(this.stop); } @@ -44,10 +24,12 @@ export class TimeoutFn { } stop() { + console.log("stopping timeout"); this.#clear(); } start(...args: Parameters | []) { + console.log("starting timeout"); this.#clear(); this.#timer = window.setTimeout(() => { this.#timer = null; diff --git a/tests/src/tests/dialog/dialog-integration-test.svelte b/tests/src/tests/dialog/dialog-integration-test.svelte new file mode 100644 index 000000000..1f25a1279 --- /dev/null +++ b/tests/src/tests/dialog/dialog-integration-test.svelte @@ -0,0 +1,30 @@ + + +
+ + open + + + + open + + + item + + + + + open + + content + + + content + + + +
diff --git a/tests/src/tests/dialog/dialog-tooltip-test.svelte b/tests/src/tests/dialog/dialog-tooltip-test.svelte new file mode 100644 index 000000000..9ef392444 --- /dev/null +++ b/tests/src/tests/dialog/dialog-tooltip-test.svelte @@ -0,0 +1,29 @@ + + + + + (open = true)}> + Hover Me & Then Click + + +
Tooltip Content
+ + + + + + +

Dialog Content

+

+ Click "Close" to close dialog and hover tooltip again. The tooltip will not + appear. +

+ Close + + + + diff --git a/tests/src/tests/dialog/dialog.browser.test.ts b/tests/src/tests/dialog/dialog.browser.test.ts index 7c3ac71b6..2872c5df5 100644 --- a/tests/src/tests/dialog/dialog.browser.test.ts +++ b/tests/src/tests/dialog/dialog.browser.test.ts @@ -7,6 +7,8 @@ import DialogTest, { type DialogTestProps } from "./dialog-test.svelte"; import DialogNestedTest from "./dialog-nested-test.svelte"; import { expectExists, expectNotExists, setupBrowserUserEvents } from "../browser-utils"; import DialogForceMountTest from "./dialog-force-mount-test.svelte"; +import DialogIntegrationTest from "./dialog-integration-test.svelte"; +import DialogTooltipTest from "./dialog-tooltip-test.svelte"; const kbd = getTestKbd(); @@ -384,3 +386,39 @@ describe("Nested Dialogs", () => { await expect.element(page.getByTestId("second-open")).toHaveFocus(); }); }); + +describe("Integration with other components", () => { + it("should allow opening nested floating components within the dialog", async () => { + render(DialogIntegrationTest); + await page.getByTestId("dialog-trigger").click(); + await expectExists(page.getByTestId("dialog-content")); + await page.getByTestId("dropdown-trigger").click(); + await expectExists(page.getByTestId("dropdown-content")); + await userEvent.keyboard(kbd.ESCAPE); + await expectNotExists(page.getByTestId("dropdown-content")); + await expectExists(page.getByTestId("dialog-content")); + await page.getByTestId("popover-trigger").click(); + await expectExists(page.getByTestId("popover-content")); + await userEvent.keyboard(kbd.ESCAPE); + await expectNotExists(page.getByTestId("popover-content")); + await expectExists(page.getByTestId("dialog-content")); + await userEvent.keyboard(kbd.ESCAPE); + await expectNotExists(page.getByTestId("dialog-content")); + }); + + it("should not break tooltip when opened from tooltip trigger and disableCloseOnTriggerClick is true", async () => { + // https://github.com/huntabyte/bits-ui/issues/1666 + render(DialogTooltipTest); + const trigger = page.getByTestId("trigger"); + await trigger.hover(); + await expectExists(page.getByTestId("tooltip-content")); + await trigger.click(); + await expectExists(page.getByTestId("dialog-content")); + await expectNotExists(page.getByTestId("tooltip-content")); + await page.getByTestId("dialog-close").click(); + + await expectNotExists(page.getByTestId("dialog-content")); + await trigger.hover(); + await expectExists(page.getByTestId("tooltip-content")); + }); +}); diff --git a/tests/src/tests/popover/popover-multiple-triggers-test.svelte b/tests/src/tests/popover/popover-multiple-triggers-test.svelte new file mode 100644 index 000000000..1009c5a0d --- /dev/null +++ b/tests/src/tests/popover/popover-multiple-triggers-test.svelte @@ -0,0 +1,12 @@ + + + + trigger-1 + trigger-2 + trigger-3 + + content + + diff --git a/tests/src/tests/popover/popover.browser.test.ts b/tests/src/tests/popover/popover.browser.test.ts index 708d7cf31..21efff611 100644 --- a/tests/src/tests/popover/popover.browser.test.ts +++ b/tests/src/tests/popover/popover.browser.test.ts @@ -9,6 +9,7 @@ import PopoverForceMountTest, { import PopoverSiblingsTest from "./popover-siblings-test.svelte"; import { expectExists, expectNotExists } from "../browser-utils"; import { page, userEvent } from "@vitest/browser/context"; +import PopoverMultipleTriggersTest from "./popover-multiple-triggers-test.svelte"; const kbd = getTestKbd(); @@ -198,3 +199,21 @@ it("should correctly handle focus when closing one popover by clicking another p await t.getByTestId("close-3").click(); await expectNotExists(t.getByTestId("content-3")); }); + +it("should restore focus to the trigger that opened the popover", async () => { + render(PopoverMultipleTriggersTest); + await page.getByTestId("trigger-1").click(); + await expectExists(page.getByTestId("content")); + await userEvent.keyboard(kbd.ESCAPE); + await expect.element(page.getByTestId("trigger-1")).toHaveFocus(); + + await page.getByTestId("trigger-2").click(); + await expectExists(page.getByTestId("content")); + await userEvent.keyboard(kbd.ESCAPE); + await expect.element(page.getByTestId("trigger-2")).toHaveFocus(); + + await page.getByTestId("trigger-3").click(); + await expectExists(page.getByTestId("content")); + await userEvent.keyboard(kbd.ESCAPE); + await expect.element(page.getByTestId("trigger-3")).toHaveFocus(); +});