[bug] Stammbaum keyboard tab order is unpredictable — follows DB order, not visual layout #718
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
On the Stammbaum (family tree,
/stammbaum), pressingTabto 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
/stammbaum(a tree with several people across 2+ generations).Tabinto the canvas — the first Tab focuses the SVG canvas itself (it is a tab stop, see below).Tabagain 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+Tabreverses it).Environment
/stammbaum, keyboard navigation between nodesRoot cause (confirmed against source)
Tab order is driven entirely by DOM source order, which is the order of the
nodesarray — and that array order has nothing to do with on-screen position.StammbaumNode.svelte:32-43renders each person as<g role="button" tabindex="0">. Because all focusable elements usetabindex="0"(no positive tabindex exists anywhere in the genealogy components), the browser walks them in DOM source order.{#each nodes as node (node.id)}inStammbaumTree.svelte:294. The{#if pos}guard at:296only skips unpositioned nodes; it does not reorder.nodesarray comes straight fromGET /api/networkviastammbaum/+page.server.ts:28-29(network.nodes ?? []) with no sorting — effectively database insertion / UUID order.buildLayout()(rank to row, sibling-block packing tox/y, written into thepositionsmap atbuildLayout.ts:271) and is consumed only for each node's SVGtransform(StammbaumNode.svelte:37) and connector geometry. This spatial ordering is never fed back into the{#each}iteration order.Net effect:
Tabwalks 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
tabindexsources, both"0", and mapped the real page tab order so tests aren't written off-by-one: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.StammbaumControls, rendered after the tree in+page.svelte:149) come after the nodes.StammbaumSidePanel) adds further focusable controls when a node is selected.<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$derivedinStammbaumTree.svelteis 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 nopositionsentry; the sort comparator must do the same (treat a missing entry as last / +Infinity rather than dereferencing.yonundefined). Add a final tie-break onnode.idto make the comparator total and deterministic — the packing invariant already guarantees distinct x within a generation (groupLeft >= cursorRight + COL_GAPatbuildLayout.ts:269), so this is bulletproofing rather than fixing a known collision. Determinism across reloads holds becausebuildLayoutalphabetically 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 enforcesgroupLeft >= cursorRight + COL_GAPatbuildLayout.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+Tabneeds no extra code — reversing the render order automatically reverses backward tabbing.Acceptance criteria
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
+/-zoom, arrow-key pan inhandleCanvasKey) 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).Implemented ✅
Tab order now follows the visual reading order instead of backend/DB row order.
Branch:
feat/issue-718-stammbaum-tab-order(offmain)Commit:
eab4153b— fix(stammbaum): order keyboard tab stops by visual layout, not DB order (#718)What changed
StammbaumTree.svelte: added anodesInReadingOrder$derived.bythat sorts a copy ofnodesby 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) andnode.idas the final tie-break for a total, deterministic comparator. The node{#each}now iterates this derived list.y(y = g * (NODE_H + ROW_GAP)), so a future vertical-stagger change won't silently regress tab order.Acceptance criteria
buildLayoutsorts 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-SVGrole="button"elements)renders node tab stops in reading order, not backend array order— scrambled input array, asserts the explicit sequenceEugenie, Walter, Clara, Hans(within-row L→R + cross-generation drop).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-checkshows 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).