diff --git a/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.test.tsx b/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.test.tsx new file mode 100644 index 00000000..5d7b8320 --- /dev/null +++ b/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.test.tsx @@ -0,0 +1,171 @@ +import { describe, expect, test } from 'bun:test'; +import { useEffect } from 'react'; +import { render, cleanup } from '@testing-library/react'; +import type { AppRegistryMap, LayoutGroup, LayoutNode, LayoutPanel } from './types'; +import { movePanel, removePanel, splitPanel, updateSizes } from './layout-utils'; +import { WorkspaceRenderer } from './WorkspaceRenderer'; +import { WorkspaceProvider, inertInteraction } from './WorkspaceContext'; + +/** + * `docs/workspace-panel-todo.md` §5.2 carries a table of which layout operations remount a panel, and + * names one cause: the React key on the panel's nearest ancestor group slot. These tests exist because + * that table was reasoned from the code and never observed. A remount is not cosmetic — it is scroll + * position, media playback, a transcode, and for a chat panel a re-read of the durable log. + * + * The instrument is a probe app that counts its own mounts. Everything is measured through the real + * `WorkspaceRenderer`, with the real `layout-utils` mutators producing the "after" tree, so a change to + * either half moves these numbers rather than quietly agreeing with a hand-written fixture. + */ + +const mounts = new Map(); +const mountCount = (id: string) => mounts.get(id) ?? 0; + +const Probe = ({ panelId }: { panelId: string }) => { + useEffect(() => { + mounts.set(panelId, (mounts.get(panelId) ?? 0) + 1); + }, [panelId]); + return
; +}; + +const ProbeIcon = ((props: Record) => ) as AppRegistryMap[string]['icon']; + +const registry: AppRegistryMap = { probe: { name: 'Probe', icon: ProbeIcon, component: Probe } }; + +const panel = (id: string): LayoutPanel => ({ type: 'panel', id, appType: 'probe' }); + +const group = (id: string, direction: 'horizontal' | 'vertical', children: LayoutNode[]): LayoutGroup => ({ + type: 'group', + id, + direction, + children: children.map((node) => ({ node, size: 100 / children.length })), +}); + +const noop = () => {}; + +// The interaction half switched off: these tests are about reconciliation, not about rearranging. +const context = { + workspace: null, + cwd: '~', + panelConfigs: {}, + ...inertInteraction, + isMobile: false, + onMobileBack: null, +}; + +const tree = (layout: LayoutNode) => ( + + + +); + +const mount = (layout: LayoutNode) => { + mounts.clear(); + const view = render(tree(layout)); + return { rerender: (next: LayoutNode) => view.rerender(tree(next)) }; +}; + +describe('what actually remounts a panel', () => { + test('resizing a group remounts nothing', () => { + const root = group('g', 'horizontal', [panel('a'), panel('b')]); + const view = mount(root); + expect(mountCount('a')).toBe(1); + + view.rerender(updateSizes(root, 'g', [70, 30])); + + expect(mountCount('a')).toBe(1); + expect(mountCount('b')).toBe(1); + cleanup(); + }); + + test('splitting in the parent direction keeps every existing panel mounted', () => { + const root = group('g', 'horizontal', [panel('a'), panel('b')]); + const view = mount(root); + + view.rerender(splitPanel(root, 'a', 'horizontal')); + + expect(mountCount('a')).toBe(1); + expect(mountCount('b')).toBe(1); + cleanup(); + }); + + test('splitting against the parent direction remounts the panel being split — and only it', () => { + const root = group('g', 'horizontal', [panel('a'), panel('b')]); + const view = mount(root); + + view.rerender(splitPanel(root, 'a', 'vertical')); + + // `a` is wrapped in a new group, so the element type at that position changes from a PanelSlot to a + // ResizablePanelGroup. No key can prevent this: React reconciles by type before it looks at keys. + expect(mountCount('a')).toBe(2); + expect(mountCount('b')).toBe(1); + cleanup(); + }); + + test('splitting a root that is a single panel remounts it', () => { + const root = panel('a'); + const view = mount(root); + + view.rerender(splitPanel(root, 'a', 'horizontal')); + + expect(mountCount('a')).toBe(2); + cleanup(); + }); + + test('removing from a three-child group leaves the survivors mounted', () => { + const root = group('g', 'horizontal', [panel('a'), panel('b'), panel('c')]); + const view = mount(root); + + view.rerender(removePanel(root, 'b')); + + expect(mountCount('a')).toBe(1); + expect(mountCount('c')).toBe(1); + cleanup(); + }); + + test('the panel that becomes first is not remounted for it', () => { + const root = group('g', 'horizontal', [panel('a'), panel('b'), panel('c')]); + const view = mount(root); + + view.rerender(removePanel(root, 'a')); + + // `b` moves from index 1 to index 0, so it loses the splitter handle that used to precede it. That + // handle sat in the same fragment slot the panel now occupies, and React remounted the panel over + // it. Nothing about `b` changed; it only stopped being second. + expect(mountCount('b')).toBe(1); + expect(mountCount('c')).toBe(1); + cleanup(); + }); + + test('removing from a two-child group remounts the survivor, and only it', () => { + const root = group('outer', 'horizontal', [group('inner', 'vertical', [panel('a'), panel('b')]), panel('c')]); + const view = mount(root); + + view.rerender(removePanel(root, 'b')); + + // The inner group collapses to `a`, so `a` takes the group's slot: both its key and its element type + // change. `c`, in an untouched slot, does not pay for it. + expect(mountCount('a')).toBe(2); + expect(mountCount('c')).toBe(1); + cleanup(); + }); + + test('a move leaves the panels it was dropped beside mounted', () => { + const root = group('g', 'horizontal', [panel('a'), panel('b'), panel('c')]); + const view = mount(root); + + view.rerender(movePanel(root, 'a', 'c', 'right')); + + // `movePanel` mints a fresh id for the moved panel, so its own remount is unavoidable and is counted + // under an id this test cannot predict. What is worth pinning is that its neighbours survive. + expect(mountCount('b')).toBe(1); + expect(mountCount('c')).toBe(1); + cleanup(); + }); +}); diff --git a/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx b/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx index 038d89e0..cca924e7 100644 --- a/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx +++ b/src/workspaces/officerdev/src/components/Workspace/WorkspaceRenderer.tsx @@ -78,7 +78,10 @@ const getFixedHeight = (node: LayoutNode, registry: AppRegistryMap): number | un }; /** Find a panel node by id anywhere in the layout tree */ -const findChildById = (children: { node: LayoutNode; size: number }[], id: string): { node: LayoutNode; size: number } | undefined => { +const findChildById = ( + children: { node: LayoutNode; size: number }[], + id: string, +): { node: LayoutNode; size: number } | undefined => { for (const child of children) { if (child.node.id === id) return child; if (child.node.type === 'panel' && child.node.id === id) return child; @@ -188,7 +191,9 @@ const LayoutNodeRenderer = ({ const hasFixedChild = node.direction === 'vertical' && - node.children.some((c) => getFixedHeight(c.node, registry) !== undefined || (c.node.type === 'panel' && c.node.fitContent)); + node.children.some( + (c) => getFixedHeight(c.node, registry) !== undefined || (c.node.type === 'panel' && c.node.fitContent), + ); if (hasFixedChild) { return ( @@ -197,7 +202,11 @@ const LayoutNodeRenderer = ({ const fixed = getFixedHeight(child.node, registry); const fit = child.node.type === 'panel' && child.node.fitContent; return ( -
+
{node.children.map((child, i) => ( - +
{ - if (index === 0) return <>{children}; - return ( - <> - - {children} - - ); -}; +/** + * Every child but the first is preceded by a splitter handle. The `null` matters: returning + * `<>{children}` for the first child put the panel in slot 0 of the fragment and the handle in slot 0 + * for everyone else, so a panel that became first — remove the leftmost of three, drag one away — found a + * `ResizableHandle` where it used to be and React remounted it. Keeping two slots keeps the panel at + * index 1 whatever happens around it. Measured in `WorkspaceRenderer.test.tsx`. + */ +const ChildEntry = ({ index, children }: ChildEntryProps) => ( + <> + {index > 0 ? : null} + {children} + +);