From 581e99daf32a1e84688f9ce68753785629abb16e Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Thu, 16 Oct 2025 16:29:33 -0400 Subject: [PATCH] Preserve scroll position during popover focus --- js/ui/overlay/popoverEngine.js | 46 ++++++++++++++++++--------------- tests/ui/popoverEngine.test.mjs | 41 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 21 deletions(-) diff --git a/js/ui/overlay/popoverEngine.js b/js/ui/overlay/popoverEngine.js index f3764129..220f10bb 100644 --- a/js/ui/overlay/popoverEngine.js +++ b/js/ui/overlay/popoverEngine.js @@ -243,6 +243,26 @@ export function createPopover(trigger, render, options = {}) { return documentRef?.defaultView || globalThis; } + function focusWithoutScroll(node) { + if (!node || typeof node.focus !== "function") { + return; + } + + const view = documentRef?.defaultView || null; + const scrollX = view?.scrollX ?? view?.pageXOffset ?? 0; + const scrollY = view?.scrollY ?? view?.pageYOffset ?? 0; + + try { + node.focus({ preventScroll: true }); + } catch (error) { + node.focus(); + } finally { + if (view && typeof view.scrollTo === "function") { + view.scrollTo(scrollX, scrollY); + } + } + } + function resetTypeaheadBuffer() { const view = getView(); if (menuState.typeaheadTimeout) { @@ -337,11 +357,7 @@ export function createPopover(trigger, render, options = {}) { updateAriaActiveDescendant(panelElement, target); if (focus) { - try { - target.focus({ preventScroll: true }); - } catch (error) { - target.focus(); - } + focusWithoutScroll(target); } } @@ -363,11 +379,7 @@ export function createPopover(trigger, render, options = {}) { if (focusPanelFallback) { panelElement.setAttribute("tabindex", panelElement.getAttribute("tabindex") || "-1"); - try { - panelElement.focus({ preventScroll: true }); - } catch (error) { - panelElement.focus(); - } + focusWithoutScroll(panelElement); } } @@ -837,20 +849,12 @@ export function createPopover(trigger, render, options = {}) { function restoreTriggerFocus() { if (anchor && typeof anchor.focus === "function") { - try { - anchor.focus({ preventScroll: true }); - return; - } catch (error) { - anchor.focus(); - } + focusWithoutScroll(anchor); + return; } if (previousActiveElement && typeof previousActiveElement.focus === "function") { - try { - previousActiveElement.focus({ preventScroll: true }); - } catch (error) { - previousActiveElement.focus(); - } + focusWithoutScroll(previousActiveElement); } } diff --git a/tests/ui/popoverEngine.test.mjs b/tests/ui/popoverEngine.test.mjs index f6303f50..84e771c0 100644 --- a/tests/ui/popoverEngine.test.mjs +++ b/tests/ui/popoverEngine.test.mjs @@ -53,6 +53,9 @@ beforeEach(() => { global.Node = windowRef.Node; global.getComputedStyle = windowRef.getComputedStyle.bind(windowRef); global.ResizeObserver = StubObserver; + global.IntersectionObserver = StubObserver; + windowRef.IntersectionObserver = StubObserver; + windowRef.scrollTo = () => {}; if (!global.PointerEvent) { global.PointerEvent = windowRef.PointerEvent || windowRef.Event; } @@ -66,6 +69,7 @@ afterEach(() => { delete global.Node; delete global.getComputedStyle; delete global.ResizeObserver; + delete global.IntersectionObserver; if (dom) { dom.window.close(); } @@ -209,6 +213,31 @@ test("supports roving focus, home/end navigation, and typeahead", async () => { height: 40, }); + const initialScroll = { x: 64, y: 128 }; + windowRef.scrollX = initialScroll.x; + windowRef.scrollY = initialScroll.y; + + const scrollCalls = []; + const originalScrollTo = windowRef.scrollTo; + windowRef.scrollTo = (x, y) => { + scrollCalls.push({ x, y }); + windowRef.scrollX = x; + windowRef.scrollY = y; + }; + + const focusMutations = []; + const originalFocus = windowRef.HTMLElement.prototype.focus; + const registerFocusMutation = (element, mutatedX, mutatedY) => { + element.focus = function focus(options = {}) { + focusMutations.push({ id: element.id || element.textContent, options }); + windowRef.scrollX = mutatedX; + windowRef.scrollY = mutatedY; + if (typeof originalFocus === "function") { + originalFocus.call(this, options); + } + }; + }; + const popover = createPopover( trigger, ({ container }) => { @@ -239,18 +268,21 @@ test("supports roving focus, home/end navigation, and typeahead", async () => { copy.className = "menu__item"; copy.type = "button"; copy.textContent = "Copy link"; + registerFocusMutation(copy, 400, 500); list.appendChild(copy); const deleteBtn = documentRef.createElement("button"); deleteBtn.className = "menu__item"; deleteBtn.type = "button"; deleteBtn.textContent = "Delete video"; + registerFocusMutation(deleteBtn, 420, 520); list.appendChild(deleteBtn); const share = documentRef.createElement("button"); share.className = "menu__item"; share.type = "button"; share.textContent = "Share"; + registerFocusMutation(share, 440, 540); list.appendChild(share); container.appendChild(panel); @@ -307,7 +339,16 @@ test("supports roving focus, home/end navigation, and typeahead", async () => { assert.equal(documentRef.activeElement, deleteBtn); assert.equal(panel.getAttribute("aria-activedescendant"), deleteBtn.id); + assert.ok(focusMutations.length >= 1, "focus mutations should occur"); + assert.ok(scrollCalls.length >= focusMutations.length); + scrollCalls.forEach((call) => { + assert.deepEqual(call, initialScroll); + }); + assert.equal(windowRef.scrollX, initialScroll.x); + assert.equal(windowRef.scrollY, initialScroll.y); + popover.destroy(); + windowRef.scrollTo = originalScrollTo; }); test("escape closes the popover and restores trigger focus", async () => {