From 61302cf50e3db230686284bf65442de8322d656d Mon Sep 17 00:00:00 2001 From: Chris Lorenzo Date: Fri, 21 Aug 2026 23:08:20 -0400 Subject: [PATCH] fix(spatialHandleNavigation): don't reset selection at grid edges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a cross-axis move (up/down in a row-wrapped container, left/right in a column-wrapped one) found no child in an adjacent row/column, the handler still called selectChild with closestIdx === -1. selectChild sets el.selected = -1 for an invalid index, so the failed move wiped the current selection. On the next key press the guard at the top of the handler saw selected === -1, treated it as "no selection", and fell back to the first focusable child — focus jumped to the top-left instead of staying put. Return false when no adjacent row/column exists so the current selection survives. The in-flex-direction branch already did this. Adds a regression test that drives a 3x2 wrapped grid through the focus manager with real key events. Co-Authored-By: Claude Opus 5 --- src/primitives/utils/handleNavigation.ts | 4 + tests/spatialNavigation.test.tsx | 112 +++++++++++++++++++++++ 2 files changed, 116 insertions(+) create mode 100644 tests/spatialNavigation.test.tsx diff --git a/src/primitives/utils/handleNavigation.ts b/src/primitives/utils/handleNavigation.ts index 587377d..f998f51 100644 --- a/src/primitives/utils/handleNavigation.ts +++ b/src/primitives/utils/handleNavigation.ts @@ -377,6 +377,10 @@ export const spatialHandleNavigation: lng.KeyHandler = function (e) { closestIdx = i; } + // No child in an adjacent column/row - keep the current selection so the + // next key press resumes from here instead of resetting to the first child. + if (closestIdx === -1) return false; + return selectChild(this as lngp.NavigableElement, closestIdx); } diff --git a/tests/spatialNavigation.test.tsx b/tests/spatialNavigation.test.tsx new file mode 100644 index 0000000..3d7b3cb --- /dev/null +++ b/tests/spatialNavigation.test.tsx @@ -0,0 +1,112 @@ +import * as v from 'vitest'; +import * as lng from '@solidtv/solid'; +import { + useFocusManager, + spatialForwardFocus, + spatialHandleNavigation, +} from '@solidtv/solid/primitives'; +import { renderer, waitForUpdate } from './setup.js'; + +const key = (k: string) => + document.dispatchEvent(new KeyboardEvent('keydown', { key: k })); + +// 3x2 wrapped grid: +// 0 1 2 +// 3 4 +let grid!: lng.ElementNode; + +v.beforeAll(async () => { + renderer.render(() => { + useFocusManager(); + return ( + + {Array.from({ length: 5 }, () => ( + + ))} + + ); + }); + await waitForUpdate(); +}); + +// Each test starts from an explicit child so it doesn't inherit the previous +// test's selection. +function focusChild(index: number) { + grid.selected = index; + (grid.children[index] as lng.ElementNode).setFocus(); +} + +v.describe('spatialHandleNavigation', () => { + v.test('lays out the fixture as a wrapped 3x2 grid', () => { + v.assert.deepEqual( + grid.children.map((c) => [ + (c as lng.ElementNode).x, + (c as lng.ElementNode).y, + ]), + [ + [0, 0], + [100, 0], + [200, 0], + [0, 100], + [100, 100], + ], + ); + }); + + v.test('keeps the selection when there is no row below', () => { + focusChild(0); + + key('ArrowDown'); // 0 -> 3 + v.assert.equal(grid.selected, 3); + + key('ArrowDown'); // no row below - selection must stay put + v.assert.equal(grid.selected, 3); + + key('ArrowDown'); // still 3, not reset to the first child + v.assert.equal(grid.selected, 3); + + key('ArrowUp'); // back up to 0 + v.assert.equal(grid.selected, 0); + }); + + v.test('keeps the selection when there is no row above', () => { + focusChild(1); + + key('ArrowUp'); // no row above - selection must stay put + v.assert.equal(grid.selected, 1); + + key('ArrowUp'); // still 1, not reset to the first child + v.assert.equal(grid.selected, 1); + + key('ArrowDown'); // 1 -> 4 + v.assert.equal(grid.selected, 4); + }); + + v.test('keeps the selection at the row edges', () => { + focusChild(0); + + key('ArrowLeft'); // already leftmost + v.assert.equal(grid.selected, 0); + + focusChild(2); + + key('ArrowRight'); // end of the row, must not wrap or reset + v.assert.equal(grid.selected, 2); + + key('ArrowLeft'); // 2 -> 1 + v.assert.equal(grid.selected, 1); + }); +});