fix(stammbaum): order keyboard tab stops by visual layout, not DB order (#718)
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m26s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m40s
CI / fail2ban Regex (pull_request) Successful in 46s
CI / Semgrep Security Scan (pull_request) Successful in 22s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m26s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m40s
CI / fail2ban Regex (pull_request) Successful in 46s
CI / Semgrep Security Scan (pull_request) Successful in 22s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s
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 <noreply@anthropic.com>
This commit is contained in:
@@ -70,6 +70,31 @@ let {
|
|||||||
|
|
||||||
const layout = $derived.by<Layout>(() => buildLayout(nodes, edges));
|
const layout = $derived.by<Layout>(() => 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
|
// Lineage highlight (#703). The adjacency index is rebuilt only when the edges
|
||||||
// change; the cheap walk re-runs whenever the selection changes. A null
|
// change; the cheap walk re-runs whenever the selection changes. A null
|
||||||
// highlight (no selection) means full strength everywhere — nothing dims.
|
// highlight (no selection) means full strength everywhere — nothing dims.
|
||||||
@@ -312,8 +337,8 @@ function handleCanvasKey(event: KeyboardEvent) {
|
|||||||
isConnectorActive={isConnectorActive}
|
isConnectorActive={isConnectorActive}
|
||||||
/>
|
/>
|
||||||
|
|
||||||
<!-- Nodes -->
|
<!-- Nodes (reading order so keyboard Tab follows the visual grid, #718) -->
|
||||||
{#each nodes as node (node.id)}
|
{#each nodesInReadingOrder as node (node.id)}
|
||||||
{@const pos = layout.positions.get(node.id)}
|
{@const pos = layout.positions.get(node.id)}
|
||||||
{#if pos}
|
{#if pos}
|
||||||
<StammbaumNode
|
<StammbaumNode
|
||||||
|
|||||||
@@ -1086,3 +1086,123 @@ describe('StammbaumTree lineage highlight (#703)', () => {
|
|||||||
expect(dimmedConnectorCount()).toBe(1);
|
expect(dimmedConnectorCount()).toBe(1);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('StammbaumTree keyboard tab order (#718)', () => {
|
||||||
|
// Tab order follows DOM source order, so the node <g role="button"> 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));
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user