Merge pull request #602 from PR0M3TH3AN/codex/add-focuswithoutscroll-helper-to-popoverengine

Preserve scroll position when focusing popovers

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 */
       }

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);
     }
   }
 

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 () => {

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",
+  );
+});