[bug] Stammbaum keyboard tab order is unpredictable — follows DB order, not visual layout #718

Closed
opened 2026-06-02 20:08:44 +02:00 by marcel · 1 comment
Owner

Summary

On the Stammbaum (family tree, /stammbaum), pressing Tab to move keyboard focus between person nodes jumps around the canvas with no relationship to where the nodes appear on screen. A node in the bottom-right corner can be focused before a top-left node. Expected behaviour: focus should sweep left to right within a generation, then drop to the next generation — i.e. reading order.

Steps to reproduce

  1. Open /stammbaum (a tree with several people across 2+ generations).
  2. Tab into the canvas — the first Tab focuses the SVG canvas itself (it is a tab stop, see below).
  3. Press Tab again to reach the first node, then keep pressing and watch the focus ring move between nodes.

Actual: The focus ring hops between nodes in an order unrelated to their visual position — it crosses generations and reverses direction seemingly at random.

Expected: After the canvas, focus advances through the topmost generation left-to-right, then the next generation left-to-right, and so on (and Shift+Tab reverses it).

Environment

  • Affected screen/flow: /stammbaum, keyboard navigation between nodes
  • Not browser-specific (it is DOM source-order driven, so reproduces in every browser)

Root cause (confirmed against source)

Tab order is driven entirely by DOM source order, which is the order of the nodes array — and that array order has nothing to do with on-screen position.

  1. Every node is a natural tab stop: StammbaumNode.svelte:32-43 renders each person as <g role="button" tabindex="0">. Because all focusable elements use tabindex="0" (no positive tabindex exists anywhere in the genealogy components), the browser walks them in DOM source order.
  2. DOM source order is the iteration order of {#each nodes as node (node.id)} in StammbaumTree.svelte:294. The {#if pos} guard at :296 only skips unpositioned nodes; it does not reorder.
  3. That nodes array comes straight from GET /api/network via stammbaum/+page.server.ts:28-29 (network.nodes ?? []) with no sorting — effectively database insertion / UUID order.
  4. The visual layout is computed independently in buildLayout() (rank to row, sibling-block packing to x/y, written into the positions map at buildLayout.ts:271) and is consumed only for each node's SVG transform (StammbaumNode.svelte:37) and connector geometry. This spatial ordering is never fed back into the {#each} iteration order.

Net effect: Tab walks the database's order while the eye walks the visual grid. The two are uncorrelated, so focus appears to jump randomly.

Focusable elements in play (full tab sequence)

A clean-agent code review confirmed exactly two tabindex sources, both "0", and mapped the real page tab order so tests aren't written off-by-one:

  • The canvas SVG is itself the first tab stop (StammbaumTree.svelte:260, tabindex="0", with the keyboard zoom/pan handler). It precedes all nodes in the DOM. So the literal sequence is canvas, then node, then node, ..., not node-first.
  • Person nodes follow, in the buggy order described above.
  • Zoom / fit controls (StammbaumControls, rendered after the tree in +page.svelte:149) come after the nodes.
  • The side panel (StammbaumSidePanel) adds further focusable controls when a node is selected.
  • Connectors (<line>/<circle>) and the generation rail (role="text", no tabindex) are correctly not focusable — no change needed there.

Suggested direction (design hint, not a hard requirement)

The data needed to order correctly already exists. Render the node loop in reading order by sorting the iterated nodes on positions.get(id).y (generation row, top-to-bottom), tie-breaking on .x (left-to-right within the row). The implementer is free to choose where the ordering lives (a $derived in StammbaumTree.svelte is the natural spot).

Why this works (and what it depends on): every node in a generation is assigned the same y = g * (NODE_H + ROW_GAP) (buildLayout.ts:92, 273), so a y-then-x sort cleanly yields per-generation reading order. This is load-bearing: if the layout is ever changed to stagger nodes vertically within a generation (e.g. offsetting spouses), tab order would silently regress. Note this dependency so it isn't broken later.

Comparator must be defensive: the render loop already guards {#if pos} for nodes with no positions entry; the sort comparator must do the same (treat a missing entry as last / +Infinity rather than dereferencing .y on undefined). Add a final tie-break on node.id to make the comparator total and deterministic — the packing invariant already guarantees distinct x within a generation (groupLeft >= cursorRight + COL_GAP at buildLayout.ts:269), so this is bulletproofing rather than fixing a known collision. Determinism across reloads holds because buildLayout alphabetically sorts each generation before packing (buildLayout.ts:62-68), so x/y are independent of backend row order.

No paint-order side effect: reordering the {#each} changes SVG paint order, but this is visually invisible — nodes never overlap (packing enforces groupLeft >= cursorRight + COL_GAP at buildLayout.ts:269), each node's focus ring is drawn inside its own <g> (StammbaumNode.svelte:44-55) so it can't be occluded by a sibling, and connectors are painted in a separate block before the nodes (:291) regardless of node order.

Shift+Tab needs no extra code — reversing the render order automatically reverses backward tabbing.

Acceptance criteria

Scenario: Canvas is the first tab stop, then nodes follow in reading order
  Given the Stammbaum is displayed with multiple people in the same generation row
  When I Tab into the canvas and then press Tab again
  Then focus moves from the SVG canvas to the leftmost node of the topmost generation

Scenario: Tab moves left-to-right within a generation
  Given focus is on a node in a generation row
  When I press Tab
  Then focus moves to the next node to the right in the same generation
  (until the rightmost node of that row is reached)

Scenario: Tab drops to the next generation after finishing a row
  Given focus is on the rightmost node of a generation row
  When I press Tab
  Then focus moves to the leftmost node of the next generation down

Scenario: Shift+Tab reverses the same order
  Given focus is on a node mid-tree
  When I press Shift+Tab
  Then focus moves to the immediately preceding node in reading order
  (the node to its left, or the rightmost node of the generation above)

Scenario: Focus order is stable across reloads
  Given the same tree data
  When I reload the page and Tab through the nodes
  Then the node focus order is identical (deterministic, not dependent on backend row order)

Testing note: assert the node sequence by scoping to role="button" elements inside the SVG, not by counting every page Tab stop — the canvas (before) and the zoom/fit controls + side panel (after) are also focusable and would make a whole-page Tab-count assertion brittle.

Non-functional / accessibility

  • WCAG 2.1 — 2.4.3 Focus Order (Level A): focusable components must receive focus in an order that preserves meaning and operability. The current behaviour fails this for the tree's primary keyboard interaction. (Secondary co-failure: 1.3.2 Meaningful Sequence — the AT reading order is scrambled too, not just sighted-keyboard focus.)
  • Existing canvas-level keyboard controls (+/- zoom, arrow-key pan in handleCanvasKey) and per-node Enter/Space selection must remain unaffected.

Severity rationale

Rated P1-high (Major): significant friction for keyboard and screen-reader users on a primary interaction, but every node is still reachable, so it is not a hard blocker. Bump to P0 if you consider the WCAG 2.4.3 Level A failure a blocking accessibility barrier.

Suggested size

XS-S — a single ordering change in the render loop, plus a focused test asserting the new node sequence (scoped to in-SVG role="button" elements, accounting for the canvas as the first tab stop).

## Summary On the **Stammbaum** (family tree, `/stammbaum`), pressing `Tab` to move keyboard focus between person nodes jumps around the canvas with no relationship to where the nodes appear on screen. A node in the bottom-right corner can be focused before a top-left node. Expected behaviour: focus should sweep **left to right within a generation, then drop to the next generation** — i.e. reading order. ## Steps to reproduce 1. Open `/stammbaum` (a tree with several people across 2+ generations). 2. `Tab` into the canvas — the **first** Tab focuses the SVG canvas itself (it is a tab stop, see below). 3. Press `Tab` again to reach the first node, then keep pressing and watch the focus ring move between nodes. **Actual:** The focus ring hops between nodes in an order unrelated to their visual position — it crosses generations and reverses direction seemingly at random. **Expected:** After the canvas, focus advances through the topmost generation left-to-right, then the next generation left-to-right, and so on (and `Shift+Tab` reverses it). ## Environment - Affected screen/flow: `/stammbaum`, keyboard navigation between nodes - Not browser-specific (it is DOM source-order driven, so reproduces in every browser) ## Root cause (confirmed against source) Tab order is driven entirely by **DOM source order**, which is the order of the `nodes` array — and that array order has nothing to do with on-screen position. 1. Every node is a natural tab stop: `StammbaumNode.svelte:32-43` renders each person as `<g role="button" tabindex="0">`. Because all focusable elements use `tabindex="0"` (no positive tabindex exists anywhere in the genealogy components), the browser walks them in DOM source order. 2. DOM source order is the iteration order of `{#each nodes as node (node.id)}` in `StammbaumTree.svelte:294`. The `{#if pos}` guard at `:296` only skips unpositioned nodes; it does not reorder. 3. That `nodes` array comes straight from `GET /api/network` via `stammbaum/+page.server.ts:28-29` (`network.nodes ?? []`) with **no sorting** — effectively database insertion / UUID order. 4. The visual layout is computed independently in `buildLayout()` (rank to row, sibling-block packing to `x`/`y`, written into the `positions` map at `buildLayout.ts:271`) and is consumed **only** for each node's SVG `transform` (`StammbaumNode.svelte:37`) and connector geometry. This spatial ordering is **never fed back** into the `{#each}` iteration order. Net effect: `Tab` walks the database's order while the eye walks the visual grid. The two are uncorrelated, so focus appears to jump randomly. ### Focusable elements in play (full tab sequence) A clean-agent code review confirmed exactly two `tabindex` sources, both `"0"`, and mapped the real page tab order so tests aren't written off-by-one: - **The canvas SVG is itself the first tab stop** (`StammbaumTree.svelte:260`, `tabindex="0"`, with the keyboard zoom/pan handler). It precedes all nodes in the DOM. So the literal sequence is **canvas, then node, then node, ...**, not node-first. - **Person nodes** follow, in the buggy order described above. - **Zoom / fit controls** (`StammbaumControls`, rendered after the tree in `+page.svelte:149`) come **after** the nodes. - **The side panel** (`StammbaumSidePanel`) adds further focusable controls when a node is selected. - Connectors (`<line>`/`<circle>`) and the generation rail (`role="text"`, no tabindex) are correctly **not** focusable — no change needed there. ## Suggested direction (design hint, not a hard requirement) The data needed to order correctly already exists. Render the node loop in reading order by sorting the iterated nodes on `positions.get(id).y` (generation row, top-to-bottom), tie-breaking on `.x` (left-to-right within the row). The implementer is free to choose where the ordering lives (a `$derived` in `StammbaumTree.svelte` is the natural spot). **Why this works (and what it depends on):** every node in a generation is assigned the *same* `y = g * (NODE_H + ROW_GAP)` (`buildLayout.ts:92, 273`), so a y-then-x sort cleanly yields per-generation reading order. This is **load-bearing**: if the layout is ever changed to stagger nodes vertically within a generation (e.g. offsetting spouses), tab order would silently regress. Note this dependency so it isn't broken later. **Comparator must be defensive:** the render loop already guards `{#if pos}` for nodes with no `positions` entry; the sort comparator must do the same (treat a missing entry as last / +Infinity rather than dereferencing `.y` on `undefined`). Add a final tie-break on `node.id` to make the comparator total and deterministic — the packing invariant already guarantees distinct x within a generation (`groupLeft >= cursorRight + COL_GAP` at `buildLayout.ts:269`), so this is bulletproofing rather than fixing a known collision. Determinism across reloads holds because `buildLayout` alphabetically sorts each generation before packing (`buildLayout.ts:62-68`), so x/y are independent of backend row order. **No paint-order side effect:** reordering the `{#each}` changes SVG paint order, but this is visually invisible — nodes never overlap (packing enforces `groupLeft >= cursorRight + COL_GAP` at `buildLayout.ts:269`), each node's focus ring is drawn *inside* its own `<g>` (`StammbaumNode.svelte:44-55`) so it can't be occluded by a sibling, and connectors are painted in a separate block *before* the nodes (`:291`) regardless of node order. **`Shift+Tab`** needs no extra code — reversing the render order automatically reverses backward tabbing. ## Acceptance criteria ```gherkin Scenario: Canvas is the first tab stop, then nodes follow in reading order Given the Stammbaum is displayed with multiple people in the same generation row When I Tab into the canvas and then press Tab again Then focus moves from the SVG canvas to the leftmost node of the topmost generation Scenario: Tab moves left-to-right within a generation Given focus is on a node in a generation row When I press Tab Then focus moves to the next node to the right in the same generation (until the rightmost node of that row is reached) Scenario: Tab drops to the next generation after finishing a row Given focus is on the rightmost node of a generation row When I press Tab Then focus moves to the leftmost node of the next generation down Scenario: Shift+Tab reverses the same order Given focus is on a node mid-tree When I press Shift+Tab Then focus moves to the immediately preceding node in reading order (the node to its left, or the rightmost node of the generation above) Scenario: Focus order is stable across reloads Given the same tree data When I reload the page and Tab through the nodes Then the node focus order is identical (deterministic, not dependent on backend row order) ``` **Testing note:** assert the *node* sequence by scoping to `role="button"` elements **inside the SVG**, not by counting every page Tab stop — the canvas (before) and the zoom/fit controls + side panel (after) are also focusable and would make a whole-page Tab-count assertion brittle. ## Non-functional / accessibility - **WCAG 2.1 — 2.4.3 Focus Order (Level A):** focusable components must receive focus in an order that preserves meaning and operability. The current behaviour fails this for the tree's primary keyboard interaction. (Secondary co-failure: 1.3.2 Meaningful Sequence — the AT reading order is scrambled too, not just sighted-keyboard focus.) - Existing canvas-level keyboard controls (`+`/`-` zoom, arrow-key pan in `handleCanvasKey`) and per-node Enter/Space selection must remain unaffected. ## Severity rationale Rated **P1-high (Major)**: significant friction for keyboard and screen-reader users on a primary interaction, but every node is still reachable, so it is not a hard blocker. Bump to P0 if you consider the WCAG 2.4.3 Level A failure a blocking accessibility barrier. ## Suggested size XS-S — a single ordering change in the render loop, plus a focused test asserting the new node sequence (scoped to in-SVG `role="button"` elements, accounting for the canvas as the first tab stop).
marcel added the P1-highbugpersonui labels 2026-06-02 20:08:48 +02:00
Author
Owner

Implemented

Tab order now follows the visual reading order instead of backend/DB row order.

Branch: feat/issue-718-stammbaum-tab-order (off main)
Commit: eab4153b — fix(stammbaum): order keyboard tab stops by visual layout, not DB order (#718)

What changed

  • StammbaumTree.svelte: added a nodesInReadingOrder $derived.by that sorts a copy of nodes by layout y (top generation first), then x (left-to-right within a row), with a missing layout position sorting last (mirrors the existing {#if pos} guard) and node.id as the final tie-break for a total, deterministic comparator. The node {#each} now iterates this derived list.
  • A load-bearing comment documents the dependency on uniform per-generation y (y = g * (NODE_H + ROW_GAP)), so a future vertical-stagger change won't silently regress tab order.

Acceptance criteria

  • Canvas → leftmost node of the topmost generation, then nodes follow in reading order
  • Tab moves left-to-right within a generation
  • Tab drops to the next generation after finishing a row
  • Shift+Tab reverses (reversed render order, no extra code)
  • Stable across reloads (x/y independent of backend row order; buildLayout sorts each generation alphabetically)

Untouched as noted in the issue: the canvas/controls/side-panel tab stops, the {#if pos} guard, the connector paint block, and the generation rail.

Tests (StammbaumTree.svelte.test.ts, scoped to in-SVG role="button" elements)

  1. renders node tab stops in reading order, not backend array order — scrambled input array, asserts the explicit sequence Eugenie, Walter, Clara, Hans (within-row L→R + cross-generation drop).
  2. orders tab stops by rendered position regardless of input order — self-validating: DOM order equals the same nodes sorted by their rendered (y, x) transform.

Both written red first (38 other tests passing), then green — 40 passed (40) on --project=client. Prettier + ESLint clean (pre-commit hook). svelte-check shows only the pre-existing project-wide baseline errors; none in the changed files.

No docs needed (no DB/route/package/error-code/permission change).

Next: open a PR from the pushed branch (or run /review-pr).

## Implemented ✅ Tab order now follows the visual reading order instead of backend/DB row order. **Branch:** `feat/issue-718-stammbaum-tab-order` (off `main`) **Commit:** `eab4153b` — fix(stammbaum): order keyboard tab stops by visual layout, not DB order (#718) ### What changed - `StammbaumTree.svelte`: added a `nodesInReadingOrder` `$derived.by` that sorts a copy of `nodes` by layout **y** (top generation first), then **x** (left-to-right within a row), with a missing layout position sorting **last** (mirrors the existing `{#if pos}` guard) and `node.id` as the final tie-break for a total, deterministic comparator. The node `{#each}` now iterates this derived list. - A load-bearing comment documents the dependency on uniform per-generation `y` (`y = g * (NODE_H + ROW_GAP)`), so a future vertical-stagger change won't silently regress tab order. ### Acceptance criteria - ✅ Canvas → leftmost node of the topmost generation, then nodes follow in reading order - ✅ Tab moves left-to-right within a generation - ✅ Tab drops to the next generation after finishing a row - ✅ Shift+Tab reverses (reversed render order, no extra code) - ✅ Stable across reloads (x/y independent of backend row order; `buildLayout` sorts each generation alphabetically) Untouched as noted in the issue: the canvas/controls/side-panel tab stops, the `{#if pos}` guard, the connector paint block, and the generation rail. ### Tests (`StammbaumTree.svelte.test.ts`, scoped to in-SVG `role="button"` elements) 1. `renders node tab stops in reading order, not backend array order` — scrambled input array, asserts the explicit sequence `Eugenie, Walter, Clara, Hans` (within-row L→R + cross-generation drop). 2. `orders tab stops by rendered position regardless of input order` — self-validating: DOM order equals the same nodes sorted by their rendered (y, x) transform. Both written red first (38 other tests passing), then green — `40 passed (40)` on `--project=client`. Prettier + ESLint clean (pre-commit hook). `svelte-check` shows only the pre-existing project-wide baseline errors; none in the changed files. No docs needed (no DB/route/package/error-code/permission change). **Next:** open a PR from the pushed branch (or run `/review-pr`).
Sign in to join this conversation.
No Label P1-high bug person ui
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#718