From 11b34ae18c9c277326002301c7f18274bcdea95a Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Sat, 26 Sep 2026 13:55:01 -0400 Subject: [PATCH 1/2] fix(v2): center the row the wrap-grid nav moves to Moving up the platforms/collections grid scrolled with block "nearest", which parks the row at the viewport top edge, under the fixed top bar and pinned toolbar, so focus ended up hidden. Vertical moves and focus restoration now center the target row, matching useGridNav. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../composables/useWrapGridNav/index.test.ts | 126 ++++++++++++++++++ .../v2/composables/useWrapGridNav/index.ts | 30 +++-- 2 files changed, 147 insertions(+), 9 deletions(-) create mode 100644 frontend/src/v2/composables/useWrapGridNav/index.test.ts diff --git a/frontend/src/v2/composables/useWrapGridNav/index.test.ts b/frontend/src/v2/composables/useWrapGridNav/index.test.ts new file mode 100644 index 000000000..85ef5b203 --- /dev/null +++ b/frontend/src/v2/composables/useWrapGridNav/index.test.ts @@ -0,0 +1,126 @@ +import { mount } from "@vue/test-utils"; +import { createPinia, setActivePinia } from "pinia"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { defineComponent, h, ref } from "vue"; +import { useInputModality } from "@/v2/composables/useInputModality"; +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(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, + }), + ), + ), + ); + }, +}); + +describe("useWrapGridNav", () => { + const { setModality } = useInputModality(); + const scrollIntoView = vi.fn(); + let wrapper: ReturnType | null = null; + const restores: (() => void)[] = []; + + function stub( + 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)[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" }), + ); + }); +}); diff --git a/frontend/src/v2/composables/useWrapGridNav/index.ts b/frontend/src/v2/composables/useWrapGridNav/index.ts index 70aef1560..da4a6d444 100644 --- a/frontend/src/v2/composables/useWrapGridNav/index.ts +++ b/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( @@ -155,7 +159,12 @@ export function useWrapGridNav( target.setAttribute("tabindex", "0"); target.focus({ preventScroll: true }); - target.scrollIntoView({ block: "nearest", behavior: "smooth" }); + // "nearest" would stop at the viewport edge, which on index pages sits + // under the fixed top bar and pinned toolbar, hiding the row above. + target.scrollIntoView({ + block: opts.verticalJump ? "center" : "nearest", + behavior: "smooth", + }); } function focusFirst() { @@ -168,7 +177,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 +186,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 +213,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 +227,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; From e877ec71ee77e0207bd92e6d1c22b1a681cfe9f7 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Sat, 26 Sep 2026 15:55:42 -0400 Subject: [PATCH 2/2] fix(v2): clear the fixed bars when centring index tiles Addresses Greptile review on #4811: - IndexShell gives focus-keyed tiles a scroll-margin matching the top bar, pinned toolbar and bottom tab bar, so a centred tile taller than the uncovered band no longer lands under the chrome on short viewports. - Cover the saved-focus restore path with a test. - Drop the comparison with "nearest" from the scroll comment. Co-Authored-By: Claude Opus 5.5 (1M context) --- frontend/src/v2/components/shared/IndexShell.vue | 8 ++++++++ .../v2/composables/useWrapGridNav/index.test.ts | 16 +++++++++++++++- .../src/v2/composables/useWrapGridNav/index.ts | 3 +-- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/frontend/src/v2/components/shared/IndexShell.vue b/frontend/src/v2/components/shared/IndexShell.vue index 32ac0354c..309528a83 100644 --- a/frontend/src/v2/components/shared/IndexShell.vue +++ b/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 { diff --git a/frontend/src/v2/composables/useWrapGridNav/index.test.ts b/frontend/src/v2/composables/useWrapGridNav/index.test.ts index 85ef5b203..bc7794bd9 100644 --- a/frontend/src/v2/composables/useWrapGridNav/index.test.ts +++ b/frontend/src/v2/composables/useWrapGridNav/index.test.ts @@ -1,8 +1,9 @@ import { mount } from "@vue/test-utils"; import { createPinia, setActivePinia } from "pinia"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { defineComponent, h, ref } from "vue"; +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", () => ({ @@ -27,6 +28,7 @@ const Grid = defineComponent({ class: "cell", "data-row": row, "data-col": col, + "data-focus-key": `${row}-${col}`, }), ), ), @@ -123,4 +125,16 @@ describe("useWrapGridNav", () => { 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" }), + ); + }); }); diff --git a/frontend/src/v2/composables/useWrapGridNav/index.ts b/frontend/src/v2/composables/useWrapGridNav/index.ts index da4a6d444..2bcc197c5 100644 --- a/frontend/src/v2/composables/useWrapGridNav/index.ts +++ b/frontend/src/v2/composables/useWrapGridNav/index.ts @@ -159,8 +159,7 @@ export function useWrapGridNav( target.setAttribute("tabindex", "0"); target.focus({ preventScroll: true }); - // "nearest" would stop at the viewport edge, which on index pages sits - // under the fixed top bar and pinned toolbar, hiding the row above. + // Rows centre on vertical moves so the fixed bars never cover them. target.scrollIntoView({ block: opts.verticalJump ? "center" : "nearest", behavior: "smooth",