officerdev/platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

zoom is the user's, so an authored layout has to graft it back

Browse Source 
`useAuthoredLayout` keeps the author's structure and re-applies only what the
user owns. It knew about sizes and nothing else, so `zoom` — which lives on the
panel node — was rebuilt from source on every render and lost.

The failure was invisible rather than merely unsaved. `setValue` re-derived a
tree identical to the one it already held, the no-write guard dropped it as a
no-op, and the header went on reading 100%. Every plugin screen, `/bots` and
`/plugins` had a zoom control that took the click and did nothing.

Collect and graft zoom alongside sizes. An absent panel id means the user has no
opinion, because `setZoom` deletes the field on reset rather than writing 1 —
that is what keeps Reset zoom reachable on a screen whose author ships a zoom of
their own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
pastilhas
2026-08-29 10:45:30 +01:00
co-authored by Claude Opus 5
parent 82d24a8feb
commit 2738f799f2
2 changed files with 153 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/workspaces/state/src/useAuthoredLayout.test.ts
+92 -2
View File
@@ -1,5 +1,5 @@
import { describe, expect, test } from 'bun:test';
import type { LayoutNode } from 'officerdev';
import type { LayoutNode, LayoutPanel } from 'officerdev';
import { __testing } from './useAuthoredLayout';
const { collectSizes, applySizes } = __testing;
@@ -12,7 +12,7 @@ const { collectSizes, applySizes } = __testing;
* way it can be wrong is silent, because a layout with the wrong numbers in it still renders.
*/
const panel = (id: string, appType: string): LayoutNode => ({ type: 'panel', id, appType });
const panel = (id: string, appType: string): LayoutPanel => ({ type: 'panel', id, appType });
const group = (id: string, children: { node: LayoutNode; size: number }[]): LayoutNode => ({
type: 'group',
@@ -127,3 +127,93 @@ describe('a repair that cannot stick must not be written', () => {
expect(JSON.stringify(afterRepair)).toBe(JSON.stringify(current));
});
});
/**
* Zoom, which was silently deleted for as long as only sizes were grafted.
*
* The failure was invisible rather than merely unsaved: the round trip produced a tree identical to the
* one already held, so `setValue`'s no-write guard dropped it and the header went on reading 100%. These
* tests are the round trip, at the level where it went wrong.
*/
describe('zoom survives the graft', () => {
const { collectZooms, applyZooms, graft } = __testing;
/** The user has zoomed the chat panel, as `setZoom` would leave it. */
const zoomed = group('root', [
{
node: group('lists', [
{ node: panel('hosts', 'proxy-manager-hosts'), size: 62 },
{ node: panel('certs', 'proxy-manager-certificates'), size: 38 },
]),
size: 66,
},
{ node: { ...panel('chat', 'proxy-manager-chat'), zoom: 1.2 }, size: 34 },
]);
test('collects only the panels that carry one', () => {
expect(collectZooms(zoomed)).toEqual({ chat: 1.2 });
expect(collectZooms(source)).toEqual({});
});
test('grafts onto the author structure, which never carries zoom itself', () => {
const merged = graft(source, zoomed);
expect(collectZooms(merged)).toEqual({ chat: 1.2 });
});
test('a zoom change is a real difference, so the no-write guard lets it through', () => {
const current = graft(source, source);
const next = graft(source, zoomed);
expect(JSON.stringify(next)).not.toBe(JSON.stringify(current));
});
test('sizes and zoom graft together, neither eating the other', () => {
const resized = group('root', [
{
node: group('lists', [
{ node: panel('hosts', 'proxy-manager-hosts'), size: 20 },
{ node: panel('certs', 'proxy-manager-certificates'), size: 80 },
]),
size: 66,
},
{ node: { ...panel('chat', 'proxy-manager-chat'), zoom: 0.8 }, size: 34 },
]);
const merged = graft(source, resized);
expect(collectSizes(merged)).toEqual({ root: [66, 34], lists: [20, 80] });
expect(collectZooms(merged)).toEqual({ chat: 0.8 });
});
/**
* `setZoom` deletes the field on reset rather than writing 1, so an absent id has to mean "no opinion"
* — otherwise Reset zoom would be unreachable on a screen whose author shipped a zoom of their own.
*/
test('a reset drops back to whatever the author said', () => {
const authored = group('root', [
{ node: { ...panel('a', 'x'), zoom: 1.4 }, size: 50 },
{ node: panel('b', 'y'), size: 50 },
]);
// Nothing stored: the author's 1.4 stands.
expect(collectZooms(graft(authored, authored))).toEqual({ a: 1.4 });
// The user overrides it.
const overridden = group('root', [
{ node: { ...panel('a', 'x'), zoom: 0.9 }, size: 50 },
{ node: panel('b', 'y'), size: 50 },
]);
expect(collectZooms(graft(authored, overridden))).toEqual({ a: 0.9 });
// The user resets — `setZoom` removed the field — and the author's value returns.
const reset = group('root', [
{ node: panel('a', 'x'), size: 50 },
{ node: panel('b', 'y'), size: 50 },
]);
expect(collectZooms(graft(authored, reset))).toEqual({ a: 1.4 });
});
test('a panel id that no longer exists is simply not applied', () => {
const stale = group('root', [{ node: { ...panel('gone', 'x'), zoom: 2 }, size: 100 }]);
expect(collectZooms(graft(source, stale))).toEqual({});
});
test('applyZooms returns the node by reference when nothing changes', () => {
const node = panel('a', 'x');
expect(applyZooms(node, {})).toBe(node);
});
});
src/workspaces/state/src/useAuthoredLayout.ts
+61 -9
View File
@@ -3,7 +3,9 @@ import { useCallback, useMemo } from 'react';
import { useDashboardState } from './useDashboardState';
/**
* A layout whose STRUCTURE belongs to whoever wrote the screen, and whose SIZES belong to whoever uses it.
* A layout whose STRUCTURE belongs to whoever wrote the screen, and whose VIEW belongs to whoever uses it.
*
* The view is sizes and zoom: the two things a `locked` screen still lets a person change.
*
* ── The bug this ends ──
*
@@ -20,18 +22,30 @@ import { useDashboardState } from './useDashboardState';
* first panel and SAVED, so the corruption survived every reload, every fix, and every restart. The
* row had to be deleted from Postgres, which is not a thing a user can do.
*
* ── The bug this ended a second time, and why zoom is here ──
*
* Sizes were the only thing grafted back, so `zoom` — which lives on the panel node, because the node is
* the thing already persisted — was collected by nothing and rebuilt from `source` every render. The
* result was a control that rendered, took the click, and discarded it: `setValue` re-derived a tree
* identical to the one it already had, the no-write guard below dropped it as a no-op, and the header
* went on reading 100%. Silent on every plugin screen, `/bots` and `/plugins`, and reported as "the zoom
* buttons do nothing".
*
* The lesson is the general one: anything the user owns has to be named here, because the default for a
* field this hook does not know about is not "passed through" — it is "deleted, quietly".
*
* ── The fix, and why it is a shape rather than a rule ──
*
* The structure comes from `source` on every render and is never read back from storage. Only sizes are
* The structure comes from `source` on every render and is never read back from storage. Only the view is
* persisted. So a stale structure cannot exist, a repair has nothing to write itself into, and a plugin
* that changes its layout reaches everybody on the next load — with no version to bump, no migration, and
* nothing for an author to remember.
*
* The user keeps the one thing that was theirs.
* The user keeps the things that were theirs.
*
* ── What is stored ──
*
* `source` with the user's sizes applied — a valid layout, because the storage validates one on both the
* `source` with the user's view applied — a valid layout, because the storage validates one on both the
* route and a CHECK constraint, and because a value that is always freshly derived from source cannot rot.
* Its structure is written and never read. That asymmetry is the whole design and is why it is safe.
*
@@ -43,6 +57,7 @@ import { useDashboardState } from './useDashboardState';
*/
type Sizes = Record<string, number[]>;
type Zooms = Record<string, number>;
const isGroup = (node: LayoutNode): node is LayoutGroup => node.type === 'group';
@@ -54,6 +69,22 @@ function collectSizes(node: LayoutNode, into: Sizes = {}): Sizes {
return into;
}
/**
* Every panel's zoom, by panel id — and only where one is actually set.
*
* `setZoom` DELETES the field on a reset to 100% rather than writing 1, so an absent id means "the user
* has no opinion" and the author's value stands. That is what makes Reset zoom reachable on a screen
* whose author shipped a zoom of their own.
*/
function collectZooms(node: LayoutNode, into: Zooms = {}): Zooms {
if (!isGroup(node)) {
if (node.zoom !== undefined) into[node.id] = node.zoom;
return into;
}
for (const child of node.children) collectZooms(child.node, into);
return into;
}
/**
* Put saved sizes back onto the author's structure.
*
@@ -78,22 +109,39 @@ function applySizes(node: LayoutNode, sizes: Sizes): LayoutNode {
};
}
/** Put saved zoom back onto the author's structure. A panel id nobody saved keeps whatever `source` says. */
function applyZooms(node: LayoutNode, zooms: Zooms): LayoutNode {
if (!isGroup(node)) {
const saved = zooms[node.id];
if (saved === undefined || saved === node.zoom) return node;
return { ...node, zoom: saved };
}
return {
...node,
children: node.children.map((child) => ({ ...child, node: applyZooms(child.node, zooms) })),
};
}
/** The author's structure wearing the user's view. The only way either half of this hook builds a tree. */
const graft = (source: LayoutNode, stored: LayoutNode): LayoutNode =>
applyZooms(applySizes(source, collectSizes(stored)), collectZooms(stored));
/**
* Same shape as `useDashboardState`, so a screen swaps one call and changes nothing else.
*
* `setValue` accepts a whole layout because that is what `WorkspaceView` reports — a resize hands back the
* entire tree. Everything but its sizes is discarded here, which is precisely the point: the view is free
* to report a normalised, repaired, or otherwise rewritten structure and none of it can be stored.
* entire tree. Everything but the user's view is discarded here, which is precisely the point: the view is
* free to report a normalised, repaired, or otherwise rewritten structure and none of it can be stored.
*/
export function useAuthoredLayout(key: string, source: LayoutNode) {
const stored = useDashboardState<LayoutNode>(key, source);
const value = useMemo(() => applySizes(source, collectSizes(stored.value)), [source, stored.value]);
const value = useMemo(() => graft(source, stored.value), [source, stored.value]);
const setValue = useCallback(
(update: LayoutNode | ((prev: LayoutNode) => LayoutNode)) => {
const next = typeof update === 'function' ? update(value) : update;
const merged = applySizes(source, collectSizes(next));
const merged = graft(source, next);
/*
A write that changes nothing is dropped, and this is not an optimisation — it is what stops an
@@ -109,6 +157,10 @@ export function useAuthoredLayout(key: string, source: LayoutNode) {
panel keys, so every render of `/browser` produced a repair. Shipping the sizes-only hook turned a
silent, harmless fallback into a hard crash on that screen.
It is also why a field this hook forgets to graft is INVISIBLE rather than merely unsaved: the
change round-trips to the same tree, compares equal, and is dropped here. That is exactly how zoom
was lost.
Comparing the serialised value is enough — these trees are small, this runs only on a real change,
and it is the same check `setValue` would otherwise force React Query to make against a new object.
*/
@@ -123,4 +175,4 @@ export function useAuthoredLayout(key: string, source: LayoutNode) {
}
/** The tree walking, exposed for its test. Not part of the hook's API — import the hook. */
export const __testing = { collectSizes, applySizes };
export const __testing = { collectSizes, applySizes, collectZooms, applyZooms, graft };