Merge pull request #4811 from rommapp/fix/wrap-grid-nav-scroll-up

fix(v2): keep the focused platform tile in view when moving up the grid

frontend/src/v2/components/shared/IndexShell.vue

@@ -89,6 +89,14 @@ html[data-bp~="xs"] .r-v2-idx-shell {
   z-index: 3;
 }
 
+/* Tiles scrolled into view by keyboard or pad nav land clear of the fixed bars. */
+.r-v2-idx-shell :deep([data-focus-key]) {
+  scroll-margin-top: calc(var(--r-nav-h) + var(--r-v2-idx-shell-toolbar-h));
+  scroll-margin-bottom: calc(
+    var(--r-bottom-nav-h) + env(safe-area-inset-bottom)
+  );
+}
+
 /* List mode: the column header and the rows run to the screen edges and keep
    the shell's gutter as padding, so only the separators and row fill move. */
 .r-v2-idx-shell {

frontend/src/v2/composables/useWrapGridNav/index.test.ts

@@ -0,0 +1,140 @@
+import { mount } from "@vue/test-utils";
+import { createPinia, setActivePinia } from "pinia";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { defineComponent, h, nextTick, ref } from "vue";
+import { useInputModality } from "@/v2/composables/useInputModality";
+import storeFocusRestoration from "@/v2/stores/focusRestoration";
+import { useWrapGridNav } from "./index";
+
+vi.mock("vue-router", () => ({
+  useRoute: () => ({ fullPath: "/platforms" }),
+}));
+
+const TILE = 100;
+
+// Two rows of two tiles, laid out by the data attributes since jsdom has no
+// layout engine.
+const Grid = defineComponent({
+  setup() {
+    const root = ref<HTMLElement | null>(null);
+    useWrapGridNav(root, { cellSelector: ".cell" });
+    return () =>
+      h(
+        "div",
+        { ref: root },
+        [0, 1].flatMap((row) =>
+          [0, 1].map((col) =>
+            h("button", {
+              class: "cell",
+              "data-row": row,
+              "data-col": col,
+              "data-focus-key": `${row}-${col}`,
+            }),
+          ),
+        ),
+      );
+  },
+});
+
+describe("useWrapGridNav", () => {
+  const { setModality } = useInputModality();
+  const scrollIntoView = vi.fn();
+  let wrapper: ReturnType<typeof mount> | null = null;
+  const restores: (() => void)[] = [];
+
+  function stub<K extends keyof HTMLElement>(
+    key: K,
+    descriptor: PropertyDescriptor,
+  ) {
+    const original = Object.getOwnPropertyDescriptor(
+      HTMLElement.prototype,
+      key,
+    );
+    Object.defineProperty(HTMLElement.prototype, key, {
+      configurable: true,
+      ...descriptor,
+    });
+    restores.push(() => {
+      if (original) Object.defineProperty(HTMLElement.prototype, key, original);
+      else delete (HTMLElement.prototype as Partial<HTMLElement>)[key];
+    });
+  }
+
+  function cell(row: number, col: number): HTMLElement {
+    return wrapper!.find(`[data-row="${row}"][data-col="${col}"]`)
+      .element as HTMLElement;
+  }
+
+  function press(key: string) {
+    document.activeElement!.dispatchEvent(
+      new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true }),
+    );
+  }
+
+  beforeEach(() => {
+    setActivePinia(createPinia());
+    setModality("mouse");
+    scrollIntoView.mockClear();
+    stub("offsetParent", {
+      get(this: HTMLElement) {
+        return this.parentElement;
+      },
+    });
+    stub("getBoundingClientRect", {
+      value(this: HTMLElement) {
+        const top = Number(this.dataset.row ?? 0) * TILE;
+        const left = Number(this.dataset.col ?? 0) * TILE;
+        return new DOMRect(left, top, TILE, TILE);
+      },
+    });
+    stub("scrollIntoView", { value: scrollIntoView });
+    wrapper = mount(Grid, { attachTo: document.body });
+  });
+
+  afterEach(() => {
+    wrapper?.unmount();
+    wrapper = null;
+    restores.splice(0).forEach((restore) => restore());
+  });
+
+  it("centres the row it moves to, so it clears the pinned top chrome", () => {
+    cell(1, 0).focus();
+
+    press("ArrowUp");
+
+    expect(document.activeElement).toBe(cell(0, 0));
+    expect(scrollIntoView).toHaveBeenLastCalledWith(
+      expect.objectContaining({ block: "center" }),
+    );
+
+    press("ArrowDown");
+
+    expect(document.activeElement).toBe(cell(1, 0));
+    expect(scrollIntoView).toHaveBeenLastCalledWith(
+      expect.objectContaining({ block: "center" }),
+    );
+  });
+
+  it("keeps the page still when moving along a row", () => {
+    cell(0, 0).focus();
+
+    press("ArrowRight");
+
+    expect(document.activeElement).toBe(cell(0, 1));
+    expect(scrollIntoView).toHaveBeenLastCalledWith(
+      expect.objectContaining({ block: "nearest" }),
+    );
+  });
+
+  it("centres the restored tile when the pad takes over", async () => {
+    storeFocusRestoration().save("/platforms", "1-1");
+
+    setModality("pad");
+    await nextTick();
+
+    expect(document.activeElement).toBe(cell(1, 1));
+    expect(scrollIntoView).toHaveBeenLastCalledWith(
+      expect.objectContaining({ block: "center" }),
+    );
+  });
+});

frontend/src/v2/composables/useWrapGridNav/index.ts

@@ -1,6 +1,6 @@
 // useWrapGridNav
 //
-// 2D arrow-key navigation for wrapping CSS grids — the layout
+// 2D arrow-key navigation for wrapping CSS grids, the layout
 // PlatformsIndex / CollectionsIndex use:
 //
 //   .grid {
@@ -10,7 +10,7 @@
 //
 // There are no per-row DOM containers; tiles wrap visually based on the
 // viewport width. Rows are reconstructed at navigation time from each
-// cell's `getBoundingClientRect().top` — cells with (approximately) the
+// cell's `getBoundingClientRect().top`: cells with (approximately) the
 // same top live on the same visual row. This handles:
 //   * variable column count at different breakpoints
 //   * multiple grouped grids in a single view (each group's grid sits
@@ -46,7 +46,7 @@ const FOCUSABLE_SELECTOR = [
 const ROW_TOP_TOLERANCE = 2;
 
 export interface UseWrapGridNavOptions {
-  /** CSS selector for the focusable tiles within the root. Required —
+  /** CSS selector for the focusable tiles within the root. Required:
    *  the composable can't guess what counts as a cell (the grid may also
    *  contain headings, panels, dividers, etc). */
   cellSelector: string;
@@ -132,7 +132,11 @@ export function useWrapGridNav(
     return null;
   }
 
-  function focusAt(rowIdx: number, colIdx: number) {
+  function focusAt(
+    rowIdx: number,
+    colIdx: number,
+    opts: { verticalJump?: boolean } = {},
+  ) {
     const rs = rows();
     const row = rs[rowIdx];
     if (!row) return;
@@ -141,7 +145,7 @@ export function useWrapGridNav(
     const cell = row[clamped];
     const target = focusableIn(cell);
 
-    // Roving tabindex — only the current cell is a tab stop. Mirrors
+    // Roving tabindex: only the current cell is a tab stop. Mirrors
     // useGridNav's approach so Tab from outside the grid lands on the
     // last-focused tile.
     const previous = rootRef.value?.querySelectorAll<HTMLElement>(
@@ -155,7 +159,11 @@ export function useWrapGridNav(
     target.setAttribute("tabindex", "0");
 
     target.focus({ preventScroll: true });
-    target.scrollIntoView({ block: "nearest", behavior: "smooth" });
+    // Rows centre on vertical moves so the fixed bars never cover them.
+    target.scrollIntoView({
+      block: opts.verticalJump ? "center" : "nearest",
+      behavior: "smooth",
+    });
   }
 
   function focusFirst() {
@@ -168,7 +176,7 @@ export function useWrapGridNav(
   // Restore focus to whichever tile carried the saved `[data-focus-key]`
   // on this route the last time the user was here. Returns false when the
   // saved key has no match in the current view (filter changed, item
-  // removed) — the caller falls back to `focusFirst`.
+  // removed); the caller falls back to `focusFirst`.
   function focusSaved(): boolean {
     const savedKey = focusStore.restore(route.fullPath);
     if (!savedKey) return false;
@@ -177,7 +185,7 @@ export function useWrapGridNav(
       for (let c = 0; c < rs[r].length; c++) {
         if (focusKeyOf(rs[r][c]) === savedKey) {
           preferredCol = c;
-          focusAt(r, c);
+          focusAt(r, c, { verticalJump: true });
           return true;
         }
       }
@@ -204,6 +212,7 @@ export function useWrapGridNav(
     const rs = rows();
     let { rowIdx, colIdx } = cur;
     const rowCells = rs[rowIdx];
+    let verticalJump = false;
 
     if (e.key === "ArrowLeft") {
       if (colIdx === 0) return;
@@ -217,14 +226,16 @@ export function useWrapGridNav(
       if (rowIdx === 0) return;
       rowIdx -= 1;
       colIdx = preferredCol;
+      verticalJump = true;
     } else if (e.key === "ArrowDown") {
       if (rowIdx === rs.length - 1) return;
       rowIdx += 1;
       colIdx = preferredCol;
+      verticalJump = true;
     }
 
     e.preventDefault();
-    focusAt(rowIdx, colIdx);
+    focusAt(rowIdx, colIdx, { verticalJump });
   }
 
   let observer: MutationObserver | null = null;