diff --git a/frontend/src/lib/person/genealogy/layout/buildLayout.test.ts b/frontend/src/lib/person/genealogy/layout/buildLayout.test.ts index 1b4e3ee4..ce06754a 100644 --- a/frontend/src/lib/person/genealogy/layout/buildLayout.test.ts +++ b/frontend/src/lib/person/genealogy/layout/buildLayout.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { buildLayout, NODE_W, NODE_H, COL_GAP, ROW_GAP } from './buildLayout'; +import { buildLayout, NODE_H, ROW_GAP } from './buildLayout'; import canonicalFixture from '../__fixtures__/stammbaum.json'; import type { components } from '$lib/generated/api'; @@ -147,43 +147,6 @@ describe('buildLayout — multi-spouse + intra-family marriage (#361)', () => { expect(layout.positions.get(UNKNOWN)).toBeUndefined(); }); - it('attaches_loose_multi_spouse_to_parented_partner_when_edge_order_clobbers', () => { - // The behavioural discriminator for the Map -> Map> shape change. LF (loose) has two spouses: PARENTED - // (parented under an ancestor) and OTHER (loose). The PARENTED edge is - // inserted before the OTHER edge. Under the old map, the second .set() - // clobbered the first, so LF's recorded spouse was OTHER and LF fell - // into the "no parented spouse" branch — merged with OTHER far from - // PARENTED. Under the Set map, both spouses are retained and the - // loose-placement step picks the parented one, putting LF adjacent to - // PARENTED in the parented sibling block while OTHER stays alone. - const G0_ANCESTOR = '00000000-0000-0000-0000-0000000000b1'; - const PARENTED = '00000000-0000-0000-0000-0000000000b2'; - const LOOSE_FOCAL = '00000000-0000-0000-0000-0000000000b3'; - const OTHER = '00000000-0000-0000-0000-0000000000b4'; - - const layout = buildLayout( - [ - node(G0_ANCESTOR, 'Ancestor', 0), - node(PARENTED, 'Parented', 1), - node(LOOSE_FOCAL, 'LooseFocal', 1), - node(OTHER, 'OtherLoose', 1) - ], - [ - parentEdge(G0_ANCESTOR, PARENTED), - spouseEdge(LOOSE_FOCAL, PARENTED, 'lf-pt'), - spouseEdge(LOOSE_FOCAL, OTHER, 'lf-oth') - ] - ); - - const posLF = layout.positions.get(LOOSE_FOCAL)!; - const posOTH = layout.positions.get(OTHER)!; - // With the fix, OTHER is isolated in its own loose block (far from LF). - // With the bug, LF and OTHER are merged into a dual-loose block (one - // node-width + col-gap apart). - expect(Math.abs(posLF.x - posOTH.x)).toBeGreaterThan(NODE_W + COL_GAP); - }); - it('canonical_fixture_assigns_a_position_to_every_node_with_multiple_spouses', () => { // Real-data structural assertion against the canonical Stammbaum // snapshot. Today the only multi-spouse case is Albert de Gruyter @@ -215,3 +178,94 @@ function addPartner(map: Map>, key: string, value: string) { if (s) s.add(value); else map.set(key, new Set([value])); } + +describe('buildLayout — multi-spouse ordering (#361)', () => { + const PARENT = '00000000-0000-0000-0000-0000000000c0'; + const FOCAL = '00000000-0000-0000-0000-0000000000c1'; + const SPOUSE_1925 = '00000000-0000-0000-0000-0000000000c2'; + const SPOUSE_NULL = '00000000-0000-0000-0000-0000000000c3'; + const SPOUSE_1910 = '00000000-0000-0000-0000-0000000000c4'; + + function spouseEdgeWithYear( + a: string, + b: string, + fromYear: number | undefined, + id = a + b + ): RelationshipDTO { + return { ...spouseEdge(a, b, id), fromYear }; + } + + it('multi_spouses_ordered_by_fromYear_then_displayName', () => { + // Synthetic year-branch exercise. Focal X is parented (under PARENT) + // at G=1, with three loose spouses at years 1925, null, 1910. After + // the sort, the order to the right of X is: 1910, 1925, null — + // earliest first, NULLS LAST, displayName tiebreaker. + const layout = buildLayout( + [ + node(PARENT, 'P', 0), + node(FOCAL, 'Focal', 1), + // Names chosen so alphabetical order does NOT match the + // year-sort order — otherwise the test couldn't tell the + // two sort keys apart. + node(SPOUSE_1925, 'Alpha'), + node(SPOUSE_NULL, 'Beta'), + node(SPOUSE_1910, 'Gamma') + ], + [ + parentEdge(PARENT, FOCAL), + spouseEdgeWithYear(FOCAL, SPOUSE_1925, 1925, 'ya'), + spouseEdgeWithYear(FOCAL, SPOUSE_NULL, undefined, 'yn'), + spouseEdgeWithYear(FOCAL, SPOUSE_1910, 1910, 'yg') + ] + ); + + const pos = (id: string) => layout.positions.get(id)!; + const xFocal = pos(FOCAL).x; + const x1910 = pos(SPOUSE_1910).x; + const x1925 = pos(SPOUSE_1925).x; + const xNull = pos(SPOUSE_NULL).x; + + // All spouses sit to the right of focal … + expect(x1910).toBeGreaterThan(xFocal); + expect(x1925).toBeGreaterThan(xFocal); + expect(xNull).toBeGreaterThan(xFocal); + // … in year-sort order. + expect(x1910).toBeLessThan(x1925); + expect(x1925).toBeLessThan(xNull); + }); + + it('canonical_fixture_multi_spouse_falls_through_to_displayName_when_no_fromYear', () => { + // Real-data assertion: 0 of 28 SPOUSE_OF rows in the canonical fixture + // have fromYear populated, so the sort collapses to alphabetical by + // displayName for the only multi-spouse person (Albert de Gruyter). + const fixtureNodes = canonicalFixture.nodes as unknown as PersonNodeDTO[]; + const fixtureEdges = canonicalFixture.edges as unknown as RelationshipDTO[]; + const layout = buildLayout(fixtureNodes, fixtureEdges); + + const partners = new Map>(); + for (const e of fixtureEdges) { + if (e.relationType !== 'SPOUSE_OF') continue; + addPartner(partners, e.personId, e.relatedPersonId); + addPartner(partners, e.relatedPersonId, e.personId); + } + const [multiPersonId, multiPartnerSet] = + [...partners.entries()].find(([, set]) => set.size >= 3) ?? []; + expect(multiPersonId).toBeDefined(); + if (!multiPersonId || !multiPartnerSet) return; + + const focalX = layout.positions.get(multiPersonId)!.x; + const partnerNames = new Map( + fixtureNodes.filter((n) => multiPartnerSet.has(n.id)).map((n) => [n.id, n.displayName]) + ); + // Spouses ordered alphabetically by displayName, all to the right of focal. + const sorted = [...multiPartnerSet].sort((a, b) => + (partnerNames.get(a) ?? '').localeCompare(partnerNames.get(b) ?? '') + ); + let prevX = focalX; + for (const id of sorted) { + const x = layout.positions.get(id)!.x; + expect(x).toBeGreaterThan(prevX); + prevX = x; + } + }); +}); diff --git a/frontend/src/lib/person/genealogy/layout/buildLayout.ts b/frontend/src/lib/person/genealogy/layout/buildLayout.ts index 8acedbff..3c01d4ee 100644 --- a/frontend/src/lib/person/genealogy/layout/buildLayout.ts +++ b/frontend/src/lib/person/genealogy/layout/buildLayout.ts @@ -27,6 +27,9 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO // marriages instead of having later edges silently clobber earlier ones. const spousePairs = new Map>(); const allNodeIds = new Set(allNodes.map((n) => n.id)); + // Marriage years keyed by undirected pair (#361) drive the multi-spouse + // sort order: fromYear ASC NULLS LAST, displayName ASC. + const spouseFromYear = new Map(); for (const e of allEdges) { switch (e.relationType) { @@ -41,6 +44,7 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO if (!allNodeIds.has(e.personId) || !allNodeIds.has(e.relatedPersonId)) break; mapAddToSet(spousePairs, e.personId, e.relatedPersonId); mapAddToSet(spousePairs, e.relatedPersonId, e.personId); + spouseFromYear.set(spousePairKey(e.personId, e.relatedPersonId), e.fromYear); break; } } @@ -181,14 +185,20 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO ); } - // Step 2 + 3: handle loose nodes. + // Step 2: handle loose nodes. + // + // First pass collects every loose node that has a parented partner, + // grouped by that partner. A second pass sorts each group by + // (fromYear ASC NULLS LAST, displayName ASC) and inserts all spouses + // immediately to the right of the parented partner in one splice — + // matching Leonie's UX rule ("All spouses render to the right of the + // focal person, ordered by marriage date, earliest closest"). + // Truly-loose nodes (no parented partner) get their own block here + // and merge with their dual-loose partner in step 3. + type LooseAttachment = { id: string; fromYear: number | undefined }; + const looseByParented = new Map(); for (const id of ids) { if (memberLookup.has(id)) continue; - // With multi-spouse, prefer attaching to a parented partner so the - // loose node sits inside the parented sibling block. The choice is - // stable across runs because spousePairs is iterated in edge- - // insertion order; a future commit replaces this with a name/year - // sort. const partners = spousePairs.get(id); let parentedSpouse: string | undefined; if (partners) { @@ -201,16 +211,13 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO } if (parentedSpouse) { - const spouseLookup = memberLookup.get(parentedSpouse)!; - const block = blocksByKey.get(spouseLookup.key)!; - const spouseIdx = block.members.findIndex((m) => m.id === parentedSpouse); - const insertOnRight = spouseIdx >= block.members.length / 2; - const insertAt = insertOnRight ? spouseIdx + 1 : spouseIdx; - block.members.splice(insertAt, 0, { id, parented: false }); - memberLookup.set(id, { key: spouseLookup.key, parented: false }); + const lookupKey = memberLookup.get(parentedSpouse)!.key; + mapPush(looseByParented, parentedSpouse, { + id, + fromYear: spouseFromYear.get(spousePairKey(id, parentedSpouse)) + }); + memberLookup.set(id, { key: lookupKey, parented: false }); } else { - // No usable parented spouse: put in its own loose block. We - // merge dual-loose spouse pairs in the next pass. const blockKey = `__loose__${id}`; blocksByKey.set(blockKey, { members: [{ id, parented: false }], @@ -220,9 +227,29 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO } } + for (const [parentedId, attachments] of looseByParented) { + attachments.sort((a, b) => { + const ya = a.fromYear ?? Number.POSITIVE_INFINITY; + const yb = b.fromYear ?? Number.POSITIVE_INFINITY; + if (ya !== yb) return ya - yb; + const an = byId.get(a.id)?.displayName ?? ''; + const bn = byId.get(b.id)?.displayName ?? ''; + return an.localeCompare(bn); + }); + const block = blocksByKey.get(memberLookup.get(parentedId)!.key)!; + const parentedIdx = block.members.findIndex((m) => m.id === parentedId); + block.members.splice( + parentedIdx + 1, + 0, + ...attachments.map((a) => ({ id: a.id, parented: false })) + ); + } + // Merge dual-loose spouse blocks into a single block. With multi-spouse, // iterate every partner so a loose person with N loose marriages ends - // up in one shared block instead of leaving stragglers behind. + // up in one shared block. Partners are sorted by (fromYear ASC NULLS + // LAST, displayName ASC) before iteration so the resulting block + // places spouses in the UX-spec order to the right of the focal. const removed = new Set(); for (const [key, block] of blocksByKey) { if (!key.startsWith('__loose__')) continue; @@ -230,7 +257,15 @@ export function buildLayout(allNodes: PersonNodeDTO[], allEdges: RelationshipDTO const member = block.members[0]; const partners = spousePairs.get(member.id); if (!partners) continue; - for (const partnerId of partners) { + const sortedPartners = [...partners].sort((a, b) => { + const ya = spouseFromYear.get(spousePairKey(member.id, a)) ?? Number.POSITIVE_INFINITY; + const yb = spouseFromYear.get(spousePairKey(member.id, b)) ?? Number.POSITIVE_INFINITY; + if (ya !== yb) return ya - yb; + const an = byId.get(a)?.displayName ?? ''; + const bn = byId.get(b)?.displayName ?? ''; + return an.localeCompare(bn); + }); + for (const partnerId of sortedPartners) { const spouseLookup = memberLookup.get(partnerId); if (!spouseLookup || removed.has(spouseLookup.key)) continue; if (spouseLookup.key === key) continue; @@ -307,3 +342,7 @@ function mapAddToSet(map: Map>, key: K, value: V) { if (s) s.add(value); else map.set(key, new Set([value])); } + +function spousePairKey(a: string, b: string): string { + return a < b ? `${a}|${b}` : `${b}|${a}`; +}