diff --git a/js/ui/components/VideoCard.js b/js/ui/components/VideoCard.js index a5c7ee9f..657221d3 100644 --- a/js/ui/components/VideoCard.js +++ b/js/ui/components/VideoCard.js @@ -226,8 +226,13 @@ export class VideoCard { closeMoreMenu(options = {}) { const restoreFocus = options?.restoreFocus !== false; + const trigger = this.moreMenuButton; + const wasExpanded = + typeof trigger?.getAttribute === "function" && + trigger.getAttribute("aria-expanded") === "true"; + const detail = { - trigger: this.moreMenuButton, + trigger, video: this.video, card: this, restoreFocus, @@ -247,9 +252,14 @@ export class VideoCard { } } - if (!handled && restoreFocus && this.moreMenuButton?.focus) { + if ( + !handled && + restoreFocus && + wasExpanded && + typeof trigger?.focus === "function" + ) { try { - this.moreMenuButton.focus(); + trigger.focus(); } catch (error) { /* noop */ } @@ -258,8 +268,13 @@ export class VideoCard { closeSettingsMenu(options = {}) { const restoreFocus = options?.restoreFocus !== false; + const trigger = this.settingsButton; + const wasExpanded = + typeof trigger?.getAttribute === "function" && + trigger.getAttribute("aria-expanded") === "true"; + const detail = { - trigger: this.settingsButton, + trigger, video: this.video, card: this, restoreFocus, @@ -279,9 +294,14 @@ export class VideoCard { } } - if (!handled && restoreFocus && this.settingsButton?.focus) { + if ( + !handled && + restoreFocus && + wasExpanded && + typeof trigger?.focus === "function" + ) { try { - this.settingsButton.focus(); + trigger.focus(); } catch (error) { /* noop */ } diff --git a/js/ui/overlay/popoverEngine.js b/js/ui/overlay/popoverEngine.js index f3764129..138b7d9c 100644 --- a/js/ui/overlay/popoverEngine.js +++ b/js/ui/overlay/popoverEngine.js @@ -243,6 +243,48 @@ export function createPopover(trigger, render, options = {}) { return documentRef?.defaultView || globalThis; } + function focusWithoutScroll(node) { + if (!node || typeof node.focus !== "function") { + return; + } + + const view = getView(); + const scrollX = view?.scrollX ?? view?.pageXOffset ?? 0; + const scrollY = view?.scrollY ?? view?.pageYOffset ?? 0; + + let preventScrollSupported = true; + + try { + node.focus({ preventScroll: true }); + } catch (error) { + preventScrollSupported = false; + node.focus(); + } + + if (!view || typeof view.scrollTo !== "function") { + return; + } + + const restoreScroll = () => { + view.scrollTo(scrollX, scrollY); + }; + + restoreScroll(); + + if (preventScrollSupported) { + return; + } + + if (typeof view.requestAnimationFrame === "function") { + view.requestAnimationFrame(restoreScroll); + return; + } + + if (typeof view.setTimeout === "function") { + view.setTimeout(restoreScroll, 0); + } + } + function resetTypeaheadBuffer() { const view = getView(); if (menuState.typeaheadTimeout) { @@ -337,11 +379,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 +401,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 +871,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..cc653ad1 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,50 @@ 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; + const originalRequestAnimationFrame = windowRef.requestAnimationFrame; + const rafCalls = []; + windowRef.scrollTo = (x, y) => { + scrollCalls.push({ x, y }); + windowRef.scrollX = x; + windowRef.scrollY = y; + }; + windowRef.requestAnimationFrame = (callback) => { + rafCalls.push(true); + if (typeof callback === "function") { + callback(); + } + return rafCalls.length; + }; + + const focusMutations = []; + const originalFocus = windowRef.HTMLElement.prototype.focus; + const registerFocusMutation = ( + element, + mutatedX, + mutatedY, + { supportsPreventScroll = true } = {}, + ) => { + element.focus = function focus(options = {}) { + focusMutations.push({ id: element.id || element.textContent, options }); + + if (!supportsPreventScroll && options && typeof options === "object" && "preventScroll" in options) { + throw new TypeError("preventScroll not supported"); + } + + windowRef.scrollX = mutatedX; + windowRef.scrollY = mutatedY; + if (typeof originalFocus === "function") { + originalFocus.call(this, options); + } + }; + }; + const popover = createPopover( trigger, ({ container }) => { @@ -239,18 +287,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, { supportsPreventScroll: false }); 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 +358,18 @@ 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.ok(rafCalls.length >= 1, "should schedule scroll restoration when preventScroll unsupported"); + assert.equal(windowRef.scrollX, initialScroll.x); + assert.equal(windowRef.scrollY, initialScroll.y); + popover.destroy(); + windowRef.scrollTo = originalScrollTo; + windowRef.requestAnimationFrame = originalRequestAnimationFrame; }); test("escape closes the popover and restores trigger focus", async () => { diff --git a/tests/video-card-source-visibility.test.mjs b/tests/video-card-source-visibility.test.mjs index 93e48610..e56020f4 100644 --- a/tests/video-card-source-visibility.test.mjs +++ b/tests/video-card-source-visibility.test.mjs @@ -139,3 +139,114 @@ test("VideoCard hides cards without playable sources until a healthy CDN update assert.equal(root.hidden, false, "a healthy CDN update should unhide the card"); assert.equal(root.dataset.sourceVisibility, "visible", "visibility dataset should reflect the restored state"); }); + +test("VideoCard.closeMoreMenu only restores focus when the trigger was expanded", (t) => { + const { document } = setupDom(t); + + const card = new VideoCard({ + document, + video: { + id: "event-focus", + title: "Focusable clip", + }, + formatters: { + formatTimeAgo: () => "just now", + }, + helpers: { + isMagnetSupported: () => true, + }, + }); + + const root = card.getRoot(); + document.body.appendChild(root); + + const trigger = card.moreMenuButton; + assert.ok(trigger, "the more menu trigger should exist"); + + let focusCalls = 0; + trigger.focus = () => { + focusCalls += 1; + }; + + card.onCloseMoreMenu = () => false; + + card.closeMoreMenu(); + assert.equal( + focusCalls, + 0, + "collapsed menus should not restore focus when closing", + ); + + trigger.setAttribute("aria-expanded", "true"); + card.closeMoreMenu(); + assert.equal( + focusCalls, + 1, + "expanded menus should restore focus back to the trigger", + ); + + trigger.setAttribute("aria-expanded", "true"); + card.closeMoreMenu({ restoreFocus: false }); + assert.equal( + focusCalls, + 1, + "restoreFocus=false should suppress focus restoration", + ); +}); + +test("VideoCard.closeSettingsMenu only restores focus when the trigger was expanded", (t) => { + const { document } = setupDom(t); + + const card = new VideoCard({ + document, + video: { + id: "event-settings", + title: "Editable clip", + }, + capabilities: { + canEdit: true, + }, + formatters: { + formatTimeAgo: () => "just now", + }, + helpers: { + isMagnetSupported: () => true, + }, + }); + + const root = card.getRoot(); + document.body.appendChild(root); + + const trigger = card.settingsButton; + assert.ok(trigger, "the settings trigger should exist when canEdit is true"); + + let focusCalls = 0; + trigger.focus = () => { + focusCalls += 1; + }; + + card.onCloseSettingsMenu = () => false; + + card.closeSettingsMenu(); + assert.equal( + focusCalls, + 0, + "collapsed settings menus should not restore focus when closing", + ); + + trigger.setAttribute("aria-expanded", "true"); + card.closeSettingsMenu(); + assert.equal( + focusCalls, + 1, + "expanded settings menus should restore focus back to the trigger", + ); + + trigger.setAttribute("aria-expanded", "true"); + card.closeSettingsMenu({ restoreFocus: false }); + assert.equal( + focusCalls, + 1, + "restoreFocus=false should suppress settings focus restoration", + ); +});