From bbd7e1e1642f3040db02e5088a998cf829ca3873 Mon Sep 17 00:00:00 2001 From: Cameron Tangney Date: Sat, 7 Jun 2025 02:11:09 -0700 Subject: [PATCH 1/3] fix: attempt to debounce popover toggling logic to simplify and fix on safari --- src/lib/utils/Popper.svelte | 83 ++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 29 deletions(-) diff --git a/src/lib/utils/Popper.svelte b/src/lib/utils/Popper.svelte index 1a08d9124..820bce03f 100644 --- a/src/lib/utils/Popper.svelte +++ b/src/lib/utils/Popper.svelte @@ -49,32 +49,27 @@ }); } - let isTriggered: boolean = false; - + // called in response to ui events that attempt to open the popover (e.g. + // clicks, hover, focusin, etc.) async function open_popover(ev: Event) { - // throttle - isTriggered = true; - await new Promise((resolve) => setTimeout(resolve, triggerDelay)); - if (!isTriggered) { - return; - } - - ev.preventDefault(); - if (ev.target !== invoker && triggerEls.includes(ev.target as HTMLElement)) { invoker = ev.target as HTMLElement; // if (invoker) invoker.popoverTargetElement = popover; - isOpen = false; - await new Promise((resolve) => setTimeout(resolve, triggerDelay)); } + start_change(true, true); + } - if (ev.type === "mousedown") { - isOpen = !isOpen; - } else { - isOpen = true; + // called in response to ui events (e.g. clicks) that attempt to toggle the + // popover, such as clicking the invoker repeatedly. + async function toggle_popover(ev: Event) { + if (ev.target !== invoker && triggerEls.includes(ev.target as HTMLElement)) { + invoker = ev.target as HTMLElement; } + start_change(!isOpen, true); } + // called in response to ui events (e.g. focusout, mouseout, click-outside) + // that attempt to close the popover. async function close_popover(ev: Event) { // For click triggers, don't close on focusout events from inside the popover if (trigger === "click" && ev.type === "focusout") { @@ -91,12 +86,6 @@ } } - isTriggered = false; - await new Promise((resolve) => setTimeout(resolve, triggerDelay)); - if (isTriggered) { - return; - } - // if popover has focus don't close when leaving the invoker if (ev?.type === "mouseleave" && popover?.contains(popover.ownerDocument.activeElement)) { return; @@ -105,7 +94,7 @@ return; } - isOpen = false; + start_change(false, true); } let autoUpdateDestroy = () => {}; @@ -132,8 +121,7 @@ function on_toggle(ev: ToggleEvent) { if (!invoker) return; - // Update isOpen value when popover state changes through other means - isOpen = ev.newState === "open"; + start_change(ev.newState === "open", false); (ev as TriggeredToggleEvent).trigger = invoker; _ontoggle?.(ev as TriggeredToggleEvent); @@ -143,7 +131,7 @@ const events: [string, any, boolean][] = [ ["focusin", open_popover, focusable], ["focusout", close_popover, focusable], - ["mousedown", open_popover, clickable], + ["mousedown", toggle_popover, clickable], ["mouseenter", open_popover, hoverable], ["mouseleave", close_popover, hoverable] ]; @@ -176,7 +164,7 @@ function closeOnEscape(event: KeyboardEvent) { if (event.key === "Escape") { - isOpen = false; + start_change(false, true); } } @@ -193,9 +181,46 @@ // Only close if click is outside both popover and trigger elements if (!isClickInsidePopover && !isClickOnTrigger) { close_popover(event); - isOpen = false; } } + + interface ChangeContext { + nextOpen: boolean; + interactive: boolean; + } + let context: ChangeContext | undefined = $state(undefined); + let timeout: NodeJS.Timeout | undefined = $state(undefined); + + // start_change debounces calls that attempt to open or close the popover. + // callers must specify whether this request is on behalf of ui interactivity + // (e.g. clicks, hovers). non-interactive invocations are assumed to be due + // to changing value of isOpen. + // NOTE: start_change prioritizes non-interactive changes over interactive + // ones, so that binding to isOpen (i.e. for things like programmatically + // controlled dropdowns) always results in the popover being toggled. + function start_change(nextOpen: boolean, interactive: boolean) { + // ignore redundant requests + if (!context && nextOpen == isOpen) return; + if (context && context.nextOpen == nextOpen) return; + + // ignore interactive requests while we're in the middle of a programmatic + // one. e.g. if a button is clicked which updates a bound isOpen prop, we + // should obey the value of isOpen and ignore any interactive events + // (click, hover, etc.) that arrive until we finish handling (triggerDelay + // ms). + if (context && !context.interactive && interactive) return; + + context = { interactive, nextOpen }; + if (timeout) clearTimeout(timeout); + timeout = setTimeout(finish_change, triggerDelay); + } + + // finish_change is called after debouncing calls to start_change. + function finish_change() { + if (!context) return; + isOpen = context.nextOpen; + context = undefined; + } From 3f50fc272b03d1a39f507b28afd58d6b089222c3 Mon Sep 17 00:00:00 2001 From: Cameron Tangney Date: Sat, 7 Jun 2025 11:18:42 -0700 Subject: [PATCH 2/3] user more portable type for timeout and clear timeout in finish_change (AI suggestions) --- src/lib/utils/Popper.svelte | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/utils/Popper.svelte b/src/lib/utils/Popper.svelte index 820bce03f..459bbc78b 100644 --- a/src/lib/utils/Popper.svelte +++ b/src/lib/utils/Popper.svelte @@ -189,7 +189,7 @@ interactive: boolean; } let context: ChangeContext | undefined = $state(undefined); - let timeout: NodeJS.Timeout | undefined = $state(undefined); + let timeout: ReturnType | undefined = $state(undefined); // start_change debounces calls that attempt to open or close the popover. // callers must specify whether this request is on behalf of ui interactivity @@ -220,6 +220,7 @@ if (!context) return; isOpen = context.nextOpen; context = undefined; + timeout = undefined; } From 29cd5857c899e793702aba14f6da36359366d785 Mon Sep 17 00:00:00 2001 From: Cameron Tangney Date: Sat, 7 Jun 2025 11:33:08 -0700 Subject: [PATCH 3/3] add timeout/context cleanup (ai suggestion) --- src/lib/utils/Popper.svelte | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/lib/utils/Popper.svelte b/src/lib/utils/Popper.svelte index 459bbc78b..91d8e3833 100644 --- a/src/lib/utils/Popper.svelte +++ b/src/lib/utils/Popper.svelte @@ -215,6 +215,16 @@ timeout = setTimeout(finish_change, triggerDelay); } + $effect(() => { + return () => { + if (timeout) { + clearTimeout(timeout); + timeout = undefined; + } + context = undefined; + }; + }); + // finish_change is called after debouncing calls to start_change. function finish_change() { if (!context) return;