make the appType allow-list a prop instead of fourteen copies
Every locked screen shipped the same recursive normaliser: an ALLOWED_APP_TYPES set, a
normalizeLayout, a useMemo to apply it before the wrong panel could render, and a useEffect to
persist the fix. Fourteen copies, character-for-character identical except the two names — so a
fifteenth screen was a copy-paste, and a bug in the shape was a bug in fourteen places.
It is now `<WorkspaceView appTypes={{ allowed, fallback }} />`. WorkspaceView normalises before it
renders and persists the diff itself, which is the same two effects the screens were writing by hand.
One deliberate behaviour change: the framework normaliser drops `config` when it replaces an app.
The fourteen copies did `{ ...node, appType: fallback }`, keeping the old app's config on the panel
the new app now owns. That is the opposite of what `setApp` does, and a config belongs to whoever
wrote it.
Headscale keeps a local useMemo. Its check is not "is this appType allowed" but "is the server
picker present at all" — a layout saved before that panel existed is discarded for the default
wholesale. That is about a panel being missing, which the allow-list cannot see.
QrTransfer gains a persist-back it never had: it normalised on read and threw the result away every
time.
Tests: normalizeLayout is pinned on reference-identity for a no-op, null always allowed, config
dropped on replacement, rebuilding only changed branches, and idempotence — because a normaliser
that does not normalise to itself makes the persist-back an infinite write loop.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,7 +5,7 @@ import { useIsMobile } from 'hooks/useIsMobile';
|
||||
import { useSessionState } from 'hooks/useSessionState';
|
||||
import type { LayoutNode, DashboardState, EphemeralPanels, PanelComponents, PanelConfig } from './types';
|
||||
import type { DropPosition } from './layout-utils';
|
||||
import { splitPanel, removePanel, setApp, updateSizes, swapPanels, movePanel, countPanels, setZoom, setPanelConfig, collectPanelConfigs, findPanelApp } from './layout-utils';
|
||||
import { splitPanel, removePanel, setApp, updateSizes, swapPanels, movePanel, countPanels, setZoom, setPanelConfig, collectPanelConfigs, findPanelApp, normalizeLayout } from './layout-utils';
|
||||
import { WorkspaceProvider } from './WorkspaceContext';
|
||||
import { parseWorkspaceKey } from './workspace-identity';
|
||||
import { WorkspaceRenderer } from './WorkspaceRenderer';
|
||||
@@ -14,6 +14,17 @@ import { useAppRegistry } from '../../AppRegistry/useAppRegistry';
|
||||
type WorkspaceViewProps = {
|
||||
workspace: DashboardState;
|
||||
locked?: boolean;
|
||||
/**
|
||||
* Pin every panel to one of these app types, replacing anything else with `fallback` and writing the
|
||||
* repair back. A locked screen should always pass this: its layout is persisted user state that
|
||||
* outlives the code, and an appType the screen no longer renders becomes an empty box the user cannot
|
||||
* get out of, because a locked screen has no app picker.
|
||||
*
|
||||
* This used to be fourteen character-identical `normalizeLayout` functions in fourteen screens, each
|
||||
* with its own `useMemo` and persist-back `useEffect` — which is how nine screens ended up with no
|
||||
* guard at all and one with a guard that re-ran forever because it never wrote the correction back.
|
||||
*/
|
||||
appTypes?: { allowed: readonly string[]; fallback: string };
|
||||
cwd?: string;
|
||||
root?: string;
|
||||
components?: PanelComponents;
|
||||
@@ -24,12 +35,24 @@ type WorkspaceViewProps = {
|
||||
|
||||
const noop = () => {};
|
||||
|
||||
export const WorkspaceView = ({ workspace, locked, cwd = '~', root, components, ephemeral, mobilePanelId, onMobilePanelChange }: WorkspaceViewProps) => {
|
||||
export const WorkspaceView = ({ workspace, locked, appTypes, cwd = '~', root, components, ephemeral, mobilePanelId, onMobilePanelChange }: WorkspaceViewProps) => {
|
||||
const { registry } = useAppRegistry();
|
||||
const isMobile = useIsMobile();
|
||||
|
||||
const layout = workspace.value;
|
||||
// Deliberately not memoised. `normalizeLayout` returns its input by reference when nothing needed
|
||||
// fixing, so the identity everything downstream depends on is already stable; a `useMemo` here would
|
||||
// only add a dependency array to get wrong, and the callers pass a fresh object literal for `appTypes`
|
||||
// anyway, which would defeat it. The walk is a handful of nodes.
|
||||
const layout = appTypes ? normalizeLayout(workspace.value, appTypes.allowed, appTypes.fallback) : workspace.value;
|
||||
const onLayoutChange = workspace.setValue;
|
||||
|
||||
// Write the repair back, so a layout is fixed once rather than re-fixed on every mount for the rest of
|
||||
// its life. Terminates because the normalised tree normalises to itself.
|
||||
const rawLayout = workspace.value;
|
||||
useEffect(() => {
|
||||
if (!workspace.isLoaded || layout === rawLayout) return;
|
||||
onLayoutChange(layout);
|
||||
}, [workspace.isLoaded, layout, rawLayout, onLayoutChange]);
|
||||
const [swapSourceId, setSwapSourceId] = useState<string | null>(null);
|
||||
const [dragSourceId, setDragSourceId] = useState<string | null>(null);
|
||||
// Per tab and per dashboard: maximising the chat panel and refreshing should come back maximised, while
|
||||
|
||||
@@ -27,6 +27,7 @@ export {
|
||||
setPanelConfig,
|
||||
collectPanelConfigs,
|
||||
findPanelApp,
|
||||
normalizeLayout,
|
||||
} from './layout-utils';
|
||||
export type { WorkspaceIdentity } from './workspace-identity';
|
||||
export { parseWorkspaceKey } from './workspace-identity';
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
findPanelApp,
|
||||
hasAnyApp,
|
||||
movePanel,
|
||||
normalizeLayout,
|
||||
pruneEmptyPanels,
|
||||
removePanel,
|
||||
setApp,
|
||||
@@ -548,3 +549,67 @@ describe('findPanelApp', () => {
|
||||
expect(findPanelApp(panel('only', 'chat'), 'only')).toBe('chat');
|
||||
});
|
||||
});
|
||||
|
||||
// The guard that keeps a locked screen renderable. It runs against persisted state that outlives the code
|
||||
// that wrote it, so the reference-equality contract is the load-bearing part: it is what tells the caller
|
||||
// whether there is a repair worth writing back, and a normaliser that always returned a new tree would
|
||||
// PATCH the server on every mount of every screen.
|
||||
describe('normalizeLayout', () => {
|
||||
const ALLOWED = ['nav', 'view'];
|
||||
|
||||
test('returns the same tree by reference when nothing needs fixing', () => {
|
||||
const root = group('g', 'horizontal', [panel('p1', 'nav'), panel('p2', 'view')]);
|
||||
expect(normalizeLayout(root, ALLOWED, 'view')).toBe(root);
|
||||
});
|
||||
|
||||
test('leaves empty panels alone — null is always allowed', () => {
|
||||
const root = group('g', 'horizontal', [panel('p1'), panel('p2', 'view')]);
|
||||
expect(normalizeLayout(root, ALLOWED, 'view')).toBe(root);
|
||||
});
|
||||
|
||||
test('replaces an app the screen does not know with the fallback', () => {
|
||||
const root = group('g', 'horizontal', [panel('p1', 'renamed-away'), panel('p2', 'view')]);
|
||||
|
||||
const next = normalizeLayout(root, ALLOWED, 'view');
|
||||
|
||||
expect(next).not.toBe(root);
|
||||
expect(find(next, 'p1')!.appType).toBe('view');
|
||||
// The panel keeps its id and its position; only the app it names changes.
|
||||
expect(ids(next)).toEqual(['p1', 'p2']);
|
||||
});
|
||||
|
||||
test('drops the config with the app that owned it', () => {
|
||||
// The same rule `setApp` follows: a config belongs to whichever app wrote it, and the fallback would
|
||||
// read the previous occupant's settings as its own.
|
||||
const root = group('g', 'horizontal', [panel('p1', 'gone', { config: { agentName: 'frontend' } })]);
|
||||
|
||||
const fixed = find(normalizeLayout(root, ALLOWED, 'view'), 'p1')!;
|
||||
|
||||
expect(fixed.appType).toBe('view');
|
||||
expect('config' in fixed).toBe(false);
|
||||
});
|
||||
|
||||
test('fixes nested panels and rebuilds only the branches that changed', () => {
|
||||
const clean = group('h', 'vertical', [panel('p1', 'nav'), panel('p2', 'view')]);
|
||||
const root = group('g', 'horizontal', [clean, group('k', 'vertical', [panel('p3', 'stale')])]);
|
||||
|
||||
const next = normalizeLayout(root, ALLOWED, 'view') as LayoutGroup;
|
||||
|
||||
expect(find(next, 'p3')!.appType).toBe('view');
|
||||
// The untouched branch is carried through by reference, which is what keeps its panels mounted.
|
||||
expect(next.children[0]!.node).toBe(clean);
|
||||
});
|
||||
|
||||
test('a normalised tree normalises to itself — the persist-back cannot loop', () => {
|
||||
const root = group('g', 'horizontal', [panel('p1', 'stale'), panel('p2', 'view')]);
|
||||
|
||||
const once = normalizeLayout(root, ALLOWED, 'view');
|
||||
expect(normalizeLayout(once, ALLOWED, 'view')).toBe(once);
|
||||
});
|
||||
|
||||
test('handles a bare panel as the whole tree', () => {
|
||||
expect((normalizeLayout(panel('only', 'stale'), ALLOWED, 'view') as LayoutPanel).appType).toBe('view');
|
||||
const ok = panel('only', 'nav');
|
||||
expect(normalizeLayout(ok, ALLOWED, 'view')).toBe(ok);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -132,6 +132,36 @@ export function updateSizes(root: LayoutNode, groupId: string, sizes: number[]):
|
||||
return { ...root, children: newChildren };
|
||||
}
|
||||
|
||||
/**
|
||||
* Pin every panel in a tree to an app the screen actually knows how to render.
|
||||
*
|
||||
* A locked screen's layout is persisted user state, so it outlives the code that wrote it: rename an
|
||||
* appType, drop a panel from a screen, restore an old row, and the stored tree still names something the
|
||||
* screen has no answer for. `PanelSlot` renders that as an empty bordered box with no picker — and on a
|
||||
* locked screen there is no way for the user to get out of it.
|
||||
*
|
||||
* Returns the input by reference when nothing needed fixing, which is what lets the caller tell "this
|
||||
* layout was already fine" from "this layout was repaired and should be written back".
|
||||
*
|
||||
* `null` — an empty panel — is always allowed. It is a state of the framework rather than an app, every
|
||||
* hand-rolled copy of this allowed it, and a screen that forgot to would rewrite each of its empty panels
|
||||
* to the fallback on first load.
|
||||
*/
|
||||
export function normalizeLayout(node: LayoutNode, allowed: readonly string[], fallback: string): LayoutNode {
|
||||
if (node.type === 'panel') {
|
||||
if (node.appType === null || allowed.includes(node.appType)) return node;
|
||||
// The config goes with the app that owned it. Whatever is in there was written by the app this panel
|
||||
// is no longer running, and handing it to the fallback is the same mistake `setApp` avoids.
|
||||
const { config: _drop, ...rest } = node;
|
||||
return { ...rest, appType: fallback };
|
||||
}
|
||||
const children = node.children.map((child) => {
|
||||
const fixed = normalizeLayout(child.node, allowed, fallback);
|
||||
return fixed === child.node ? child : { ...child, node: fixed };
|
||||
});
|
||||
return children.some((c, i) => c !== node.children[i]) ? { ...node, children } : node;
|
||||
}
|
||||
|
||||
export function pruneEmptyPanels(root: LayoutNode): LayoutNode | null {
|
||||
if (root.type === 'panel') {
|
||||
return root.appType ? root : null;
|
||||
|
||||
Reference in New Issue
Block a user