From abea7a3a3deadf91f9d33871bc8e46d78806a9ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 7 Aug 2026 10:41:21 +0000 Subject: [PATCH] splitting one panel no longer resets the whole row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit splitInner and insertPanel both ended with `100 / newChildren.length` applied to every sibling, so splitting any panel in a group discarded every proportion in it. A deliberately narrow sidebar became an equal column the first time anyone split the panel next to it — and there was no way to get it back except by dragging the splitter again. The new sibling now takes half of the target's size and nothing else moves. One helper for both call sites, because the drop path (movePanel -> insertPanel) had the identical bug and would otherwise have kept it. Three tests. Two of them were already there asserting the even split, written against the old behaviour on purpose; they now assert the new one. The move test is new and documents the interaction worth knowing: removePanel renormalises the group when the panel leaves, so a move reads as renormalise-then-halve. --- .../components/Workspace/layout-utils.test.ts | 41 +++++++++++++++++-- .../src/components/Workspace/layout-utils.ts | 35 +++++++++------- 2 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts b/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts index 35d0b535..6597da9a 100644 --- a/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts +++ b/src/workspaces/officerdev/src/components/Workspace/layout-utils.test.ts @@ -75,7 +75,8 @@ describe('splitPanel', () => { expect(next.children).toHaveLength(3); // The original node object survives — this is what keeps the panel's React subtree mounted. expect(next.children[0]!.node).toBe(a); - expect(next.children.map((c) => c.size)).toEqual([100 / 3, 100 / 3, 100 / 3]); + // The new panel takes half of `a`; `b` is not touched, and the group still sums to 100. + expect(next.children.map((c) => c.size)).toEqual([25, 25, 50]); }); test('wraps in a new group when the direction differs', () => { @@ -103,8 +104,7 @@ describe('splitPanel', () => { expect(splitPanel(root, 'nope', 'horizontal')).toBe(root); }); - test('sizes are redistributed evenly, discarding tuned proportions', () => { - // Documenting current behaviour, not endorsing it — this is todo §5.5 "preserve sibling sizes". + test('splits the target in half and leaves every other sibling alone', () => { const root: LayoutGroup = { type: 'group', id: 'g', @@ -116,7 +116,20 @@ describe('splitPanel', () => { }; const next = splitPanel(root, 'a', 'horizontal') as LayoutGroup; - expect(next.children.every((c) => c.size === 100 / 3)).toBe(true); + + // A deliberately narrow `b` stays narrow. Splitting one panel is not a reason to reset the row. + expect(next.children.map((c) => c.size)).toEqual([40, 40, 20]); + expect(next.children.reduce((sum, c) => sum + c.size, 0)).toBe(100); + }); + + test('a repeated split keeps halving only the panel being split', () => { + let root: LayoutNode = group('g', 'horizontal', [panel('a'), panel('b')]); + root = splitPanel(root, 'a', 'horizontal'); + root = splitPanel(root, 'a', 'horizontal'); + + const sizes = (root as LayoutGroup).children.map((c) => c.size); + expect(sizes).toEqual([12.5, 12.5, 25, 50]); + expect(sizes.reduce((sum, s) => sum + s, 0)).toBe(100); }); }); @@ -327,6 +340,26 @@ describe('swapPanels', () => { }); describe('movePanel', () => { + test('drops the moved panel into half of its target, leaving the other siblings alone', () => { + const root: LayoutGroup = { + type: 'group', + id: 'g', + direction: 'horizontal', + children: [ + { node: panel('a', 'x'), size: 20 }, + { node: panel('b'), size: 20 }, + { node: panel('c'), size: 60 }, + ], + }; + + const next = movePanel(root, 'a', 'c', 'right') as LayoutGroup; + + // `a` leaves, which `removePanel` renormalises to b=25 / c=75; `c` then gives up half of its width + // to receive `a` back. `b` keeps its share of the row rather than being reset to a third of it. + expect(next.children.map((c) => c.size)).toEqual([25, 37.5, 37.5]); + expect(next.children.reduce((sum, c) => sum + c.size, 0)).toBe(100); + }); + test("carries the panel's config to its new position", () => { // The drag path is dead code today (todo §5.3), but this is the invariant that disarmed it as a // hazard: an agent's name travels with the panel rather than being reset to defaults. diff --git a/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts b/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts index d8419413..b1c74652 100644 --- a/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts +++ b/src/workspaces/officerdev/src/components/Workspace/layout-utils.ts @@ -10,6 +10,24 @@ export const createDefaultLayout = (): LayoutPanel => ({ appType: null, }); +type LayoutChild = LayoutGroup['children'][number]; + +/** + * Insert a new sibling by taking half of the target's space, not by redistributing the whole group. + * Splitting one panel used to reset every proportion around it, so a deliberately narrow sidebar became + * an equal column the first time anyone split anything else in the same row. + */ +function insertSplittingTarget( + children: LayoutChild[], + targetIdx: number, + insertIdx: number, + newPanel: LayoutNode, +): LayoutChild[] { + const half = children[targetIdx]!.size / 2; + const resized = children.map((c, i) => (i === targetIdx ? { ...c, size: half } : c)); + return [...resized.slice(0, insertIdx), { node: newPanel, size: half }, ...resized.slice(insertIdx)]; +} + export function splitPanel(root: LayoutNode, panelId: string, direction: 'horizontal' | 'vertical'): LayoutNode { return splitInner(root, panelId, direction); } @@ -35,13 +53,7 @@ function splitInner(node: LayoutNode, panelId: string, direction: 'horizontal' | const childIdx = node.children.findIndex((c) => c.node.type === 'panel' && c.node.id === panelId); if (childIdx !== -1 && node.direction === direction) { const newPanel: LayoutPanel = { type: 'panel', id: uid(), appType: null }; - const newChildren = [ - ...node.children.slice(0, childIdx + 1), - { node: newPanel, size: 0 }, - ...node.children.slice(childIdx + 1), - ]; - const size = 100 / newChildren.length; - return { ...node, children: newChildren.map((c) => ({ ...c, size })) }; + return { ...node, children: insertSplittingTarget(node.children, childIdx, childIdx + 1, newPanel) }; } // Recurse into children @@ -231,15 +243,8 @@ function insertPanel( const childIdx = node.children.findIndex((c) => c.node.type === 'panel' && c.node.id === targetId); if (childIdx !== -1 && node.direction === direction) { - const newPanel = newPanelFrom(contents); const insertIdx = before ? childIdx : childIdx + 1; - const newChildren = [ - ...node.children.slice(0, insertIdx), - { node: newPanel, size: 0 }, - ...node.children.slice(insertIdx), - ]; - const size = 100 / newChildren.length; - return { ...node, children: newChildren.map((c) => ({ ...c, size })) }; + return { ...node, children: insertSplittingTarget(node.children, childIdx, insertIdx, newPanelFrom(contents)) }; } const newChildren = node.children.map((child) => ({