From eab4153b97d3b2bdc27bdda7febb7703484b08d4 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 20:31:43 +0200 Subject: [PATCH] fix(stammbaum): order keyboard tab stops by visual layout, not DB order (#718) Person nodes rendered in `nodes` array order (backend/DB row order), so Tab focus hopped between nodes unrelated to their on-screen position, failing WCAG 2.4.3 Focus Order (Level A). Render the node loop in reading order instead: sort by layout y (top generation first) then x (left-to-right within a row), via a `nodesInReadingOrder` derived. Nodes without a layout position sort last (mirroring the `{#if pos}` guard); node.id is the final tie-break for a total, deterministic comparator. Shift+Tab and reload-stability fall out for free (reversed render order; x/y independent of backend order). Co-Authored-By: Claude Sonnet 4.6 --- .../lib/person/genealogy/StammbaumTree.svelte | 29 ++++- .../genealogy/StammbaumTree.svelte.test.ts | 120 ++++++++++++++++++ 2 files changed, 147 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/person/genealogy/StammbaumTree.svelte b/frontend/src/lib/person/genealogy/StammbaumTree.svelte index 53ccf02b..ccc5ed27 100644 --- a/frontend/src/lib/person/genealogy/StammbaumTree.svelte +++ b/frontend/src/lib/person/genealogy/StammbaumTree.svelte @@ -70,6 +70,31 @@ let { const layout = $derived.by(() => buildLayout(nodes, edges)); +// Keyboard tab order follows DOM source order (#718). Render the nodes in +// reading order — top generation first (by layout y), left-to-right within a +// row (by layout x) — so focus sweeps the visual grid instead of the backend +// row order in which `nodes` arrives. Nodes without a layout position sort last +// (mirroring the `{#if pos}` render guard); node.id is the final tie-break for a +// total, deterministic comparator. +// +// LOAD-BEARING: relies on every node in a generation sharing the same y +// (buildLayout assigns y = g * (NODE_H + ROW_GAP)). If the layout is ever +// changed to stagger nodes vertically within a generation, this y-then-x sort +// would silently regress tab order — revisit here. +const nodesInReadingOrder = $derived.by(() => + [...nodes].sort((a, b) => { + const pa = layout.positions.get(a.id); + const pb = layout.positions.get(b.id); + const ay = pa ? pa.y : Infinity; + const by = pb ? pb.y : Infinity; + if (ay !== by) return ay - by; + const ax = pa ? pa.x : Infinity; + const bx = pb ? pb.x : Infinity; + if (ax !== bx) return ax - bx; + return a.id.localeCompare(b.id); + }) +); + // Lineage highlight (#703). The adjacency index is rebuilt only when the edges // change; the cheap walk re-runs whenever the selection changes. A null // highlight (no selection) means full strength everywhere — nothing dims. @@ -312,8 +337,8 @@ function handleCanvasKey(event: KeyboardEvent) { isConnectorActive={isConnectorActive} /> - - {#each nodes as node (node.id)} + + {#each nodesInReadingOrder as node (node.id)} {@const pos = layout.positions.get(node.id)} {#if pos} { expect(dimmedConnectorCount()).toBe(1); }); }); + +describe('StammbaumTree keyboard tab order (#718)', () => { + // Tab order follows DOM source order, so the node elements + // must render in reading order — top generation first, left-to-right within a + // row — regardless of the order nodes arrive in the `nodes` array (which is + // backend/DB order). Scope to in-SVG role="button" elements: the canvas and + // the controls/side panel are also focusable, so a whole-page Tab count would + // be brittle (see the issue's testing note). + const WALTER = '00000000-0000-0000-0000-0000000000a1'; + const EUGENIE = '00000000-0000-0000-0000-0000000000a2'; + const CLARA = '00000000-0000-0000-0000-0000000000a3'; + const HANS = '00000000-0000-0000-0000-0000000000a4'; + + // Walter ↔ Eugenie (gen 0); their children Clara + Hans (gen 1). buildLayout + // sorts each generation alphabetically, so the deterministic visual order is + // Eugenie, Walter (top row) then Clara, Hans (next row). + const FAMILY_EDGES = [ + { + id: 'sp', + personId: WALTER, + relatedPersonId: EUGENIE, + personDisplayName: 'Walter', + relatedPersonDisplayName: 'Eugenie', + relationType: 'SPOUSE_OF' + }, + { + id: 'p1', + personId: WALTER, + relatedPersonId: CLARA, + personDisplayName: 'Walter', + relatedPersonDisplayName: 'Clara', + relationType: 'PARENT_OF' + }, + { + id: 'p2', + personId: EUGENIE, + relatedPersonId: CLARA, + personDisplayName: 'Eugenie', + relatedPersonDisplayName: 'Clara', + relationType: 'PARENT_OF' + }, + { + id: 'p3', + personId: WALTER, + relatedPersonId: HANS, + personDisplayName: 'Walter', + relatedPersonDisplayName: 'Hans', + relationType: 'PARENT_OF' + }, + { + id: 'p4', + personId: EUGENIE, + relatedPersonId: HANS, + personDisplayName: 'Eugenie', + relatedPersonDisplayName: 'Hans', + relationType: 'PARENT_OF' + } + ]; + + function nodeLabelsInDomOrder(): string[] { + const svg = document.querySelector('svg')!; + return Array.from(svg.querySelectorAll('g[role="button"]')).map( + (g) => (g.getAttribute('aria-label') ?? '').split(',')[0] + ); + } + + it('renders node tab stops in reading order, not backend array order', () => { + // Deliberately scrambled (not reading order): Hans (gen 1), Eugenie + // (gen 0), Clara (gen 1), Walter (gen 0). Source order alone would tab + // gen1 → gen0 → gen1 → gen0. + render(StammbaumTree, { + nodes: [ + { id: HANS, displayName: 'Hans', familyMember: true }, + { id: EUGENIE, displayName: 'Eugenie', familyMember: true }, + { id: CLARA, displayName: 'Clara', familyMember: true }, + { id: WALTER, displayName: 'Walter', familyMember: true } + ], + edges: FAMILY_EDGES, + selectedId: null, + panZoom: { x: 0, y: 0, z: 1 }, + showGutter: false, + onSelect: () => {} + }); + + // Top generation left-to-right, then next generation left-to-right. + expect(nodeLabelsInDomOrder()).toEqual(['Eugenie', 'Walter', 'Clara', 'Hans']); + }); + + it('orders tab stops by rendered position regardless of input order', () => { + // Self-validating against the actual layout: DOM order of the node groups + // must equal the same groups sorted by (y, then x). Independent of the + // alphabetical assumption above, so it survives layout-internal changes. + render(StammbaumTree, { + nodes: [ + { id: CLARA, displayName: 'Clara', familyMember: true }, + { id: WALTER, displayName: 'Walter', familyMember: true }, + { id: HANS, displayName: 'Hans', familyMember: true }, + { id: EUGENIE, displayName: 'Eugenie', familyMember: true } + ], + edges: FAMILY_EDGES, + selectedId: null, + panZoom: { x: 0, y: 0, z: 1 }, + showGutter: false, + onSelect: () => {} + }); + + const svg = document.querySelector('svg')!; + const placed = Array.from(svg.querySelectorAll('g[role="button"]')).map((g) => { + const t = g.getAttribute('transform') ?? ''; + const m = t.match(/translate\((-?[\d.]+)[,\s]+(-?[\d.]+)\)/); + return { + label: (g.getAttribute('aria-label') ?? '').split(',')[0], + x: m ? parseFloat(m[1]) : Infinity, + y: m ? parseFloat(m[2]) : Infinity + }; + }); + const readingOrder = [...placed].sort((a, b) => a.y - b.y || a.x - b.x); + expect(placed.map((n) => n.label)).toEqual(readingOrder.map((n) => n.label)); + }); +});