From c79b5a287b3074a95b6fc4131444b84b321b2064 Mon Sep 17 00:00:00 2001 From: Andre Padez Date: Sat, 8 Aug 2026 22:08:08 +0100 Subject: [PATCH] retire the green and amber lights, and the shallow depth they drove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maximize could only ever stop below the nav header — the content region is an `absolute z-2` stacking context and the header a `fixed z-10` sibling, so that was the deepest a panel could get on its own. Full screen reaches the rest by asking the shell to stand its header down, which leaves the shallow version as a state nobody picks on purpose. So it goes, and the two lights with it. `MaximizeMode` and the `{ id, mode }` session value collapse back to a bare `fullscreenPanelId` — renamed because "maximized" would now be a lie about what it does — under a new `FULLSCREEN_PANEL:` key, so a tab open across this reads nothing rather than an object where a string belongs. `MaximizeButton` is gone; locked screens keep only the fullscreen toggle, which writes no layout and so was always the one control the lock could permit. Red stays. The amber used to REPLACE it while maximized so the way out could never be a way to delete; with amber gone that guard would have cost the close button entirely, and red is already absent exactly where it should be — locked screens render no traffic lights at all. Co-Authored-By: Claude Opus 5 --- src/workspaces/hooks/src/useSessionState.ts | 4 +- .../components/Workspace/PanelSlot.test.tsx | 104 +++++---- .../src/components/Workspace/PanelSlot.tsx | 203 +++++------------- .../components/Workspace/WorkspaceContext.ts | 24 ++- .../Workspace/WorkspaceView.test.tsx | 2 +- .../components/Workspace/WorkspaceView.tsx | 56 +++-- .../src/components/Workspace/index.ts | 1 - .../src/components/Workspace/types.ts | 12 -- 8 files changed, 146 insertions(+), 260 deletions(-) diff --git a/src/workspaces/hooks/src/useSessionState.ts b/src/workspaces/hooks/src/useSessionState.ts index 1b561faa..df96ac13 100644 --- a/src/workspaces/hooks/src/useSessionState.ts +++ b/src/workspaces/hooks/src/useSessionState.ts @@ -57,10 +57,10 @@ export function clearSessionValue(key: string): void { * * Same shape as `useState`: a value, a setter that accepts an updater, and a `reset` that forgets the * stored copy and returns to `initialValue`. `key` is the sessionStorage key verbatim — scope it - * yourself (`` `MAXIMIZED_PANEL:${dashboardId}` ``) when one screen needs one value per thing. + * yourself (`` `FULLSCREEN_PANEL:${dashboardId}` ``) when one screen needs one value per thing. * * `null` is a real stored value, not an absence: setting it persists "explicitly nothing", which is the - * difference between a panel you un-maximised and one you never maximised. + * difference between a panel you took back out of full screen and one you never put there. */ export function useSessionState(key: string, initialValue: T) { // Read the stored value once per mount rather than on every render — `useGlobal` re-evaluates its diff --git a/src/workspaces/officerdev/src/components/Workspace/PanelSlot.test.tsx b/src/workspaces/officerdev/src/components/Workspace/PanelSlot.test.tsx index 006cc8f2..3ed0d1cc 100644 --- a/src/workspaces/officerdev/src/components/Workspace/PanelSlot.test.tsx +++ b/src/workspaces/officerdev/src/components/Workspace/PanelSlot.test.tsx @@ -13,7 +13,7 @@ import { PanelSlot } from './PanelSlot'; * "in this mode, which controls exist, and does each one call the handler it is named after" — cheap to * answer and nobody had. * - * The mode matrix is the substance. `interactive`, `locked`, `isMobile`, `isLastPanel`, `maximizedPanelId` + * The mode matrix is the substance. `interactive`, `locked`, `isMobile`, `isLastPanel`, `fullscreenPanelId` * and an app's own `zoomable`/`transparent` flags combine into chrome that ranges from full traffic * lights down to a bare outline, and the combinations are chosen in six separate ternaries spread over * the file. A control appearing in a mode that should not have it is a way to edit a locked screen; a @@ -21,7 +21,7 @@ import { PanelSlot } from './PanelSlot'; * * `WorkspaceView.test.tsx` covers the same chrome from above, through the real renderer. This drives * `PanelSlot` directly so it can put the workspace into states a whole view cannot easily be pushed - * into — maximized, mid-swap, mobile. + * into — fullscreen, mid-swap, mobile. */ const Icon = ((props: Record) => ) as AppRegistryMap[string]['icon']; @@ -52,7 +52,7 @@ type Calls = { remove: string[]; zoom: [string, number][]; swap: [string, string][]; - maximize: [string | null, string | undefined][]; + fullscreen: (string | null)[]; swapSource: (string | null)[]; mobileBack: number; }; @@ -60,7 +60,7 @@ type Calls = { let calls: Calls; beforeEach(() => { - calls = { setApp: [], split: [], remove: [], zoom: [], swap: [], maximize: [], swapSource: [], mobileBack: 0 }; + calls = { setApp: [], split: [], remove: [], zoom: [], swap: [], fullscreen: [], swapSource: [], mobileBack: 0 }; }); type MountOptions = { @@ -83,7 +83,7 @@ function mount(options: MountOptions = {}) { onMobileBack: null, onSwap: (source, target) => calls.swap.push([source, target]), setSwapSourceId: (id) => calls.swapSource.push(id), - setMaximizedPanelId: (id, mode) => calls.maximize.push([id, mode]), + setFullscreenPanelId: (id) => calls.fullscreen.push(id), onSetZoom: (id, z) => calls.zoom.push([id, z]), ...options.context, }; @@ -130,26 +130,26 @@ describe('the red button does what its own label says', () => { }); describe('which chrome each mode gets', () => { - test('interactive and unlocked: close and maximize', () => { + test('interactive and unlocked: close and full screen', () => { mount(); expect(screen.getByTitle('Close panel')).toBeTruthy(); - expect(screen.getByTitle('Maximize')).toBeTruthy(); + expect(screen.getByTitle('Full screen')).toBeTruthy(); }); - test('locked: maximize only — a locked screen is not the user’s to edit', () => { + test('locked: full screen only — a locked screen is not the user’s to edit', () => { mount({ locked: true }); expect(screen.queryByTitle('Close panel')).toBeNull(); expect(screen.queryByTitle('Clear app')).toBeNull(); - expect(screen.getByTitle('Maximize')).toBeTruthy(); + expect(screen.getByTitle('Full screen')).toBeTruthy(); }); test('not interactive: no chrome buttons at all', () => { mount({ interactive: false }); expect(screen.queryByTitle('Close panel')).toBeNull(); - expect(screen.queryByTitle('Maximize')).toBeNull(); + expect(screen.queryByTitle('Full screen')).toBeNull(); expect(screen.queryByTitle('Zoom in')).toBeNull(); expect(screen.getByTestId('app')).toBeTruthy(); }); @@ -164,28 +164,29 @@ describe('which chrome each mode gets', () => { expect(calls.mobileBack).toBe(1); }); - test('maximized: Restore replaces close, so the way out is never a way to delete', () => { - mount({ context: { maximizedPanelId: 'p1' } }); - - expect(screen.queryByTitle('Close panel')).toBeNull(); - fireEvent.click(screen.getByTitle('Restore')); - - expect(calls.maximize).toEqual([[null, undefined]]); - expect(calls.remove).toEqual([]); - }); - - test('maximize sets this panel, and a locked maximize toggles', () => { + test('the green and amber lights are gone, at both depths of the old feature', () => { + // They drove a shallower "maximize" that stopped below the nav header — all a panel can reach from + // inside the content region's stacking context. Full screen replaced it outright, so the pair went + // rather than becoming a second way to do a subset of one thing. const view = mount(); - fireEvent.click(screen.getByTitle('Maximize')); - expect(calls.maximize).toEqual([['p1', undefined]]); + expect(screen.queryByTitle('Maximize')).toBeNull(); + expect(screen.queryByTitle('Restore')).toBeNull(); view.unmount(); - mount({ locked: true, context: { maximizedPanelId: 'p1' } }); - fireEvent.click(screen.getByTitle('Restore')); - expect(calls.maximize).toEqual([ - ['p1', undefined], - [null, undefined], - ]); + mount({ context: { fullscreenPanelId: 'p1' } }); + expect(screen.queryByTitle('Maximize')).toBeNull(); + expect(screen.queryByTitle('Restore')).toBeNull(); + }); + + test('red survives full screen, because closing and shrinking were never the same act', () => { + // The amber light used to REPLACE red while maximized, so the way out could never be a way to delete. + // With amber gone that guard would have cost the close button entirely; the fullscreen toggle sits + // beside red instead, and the two read differently enough to carry it. + mount({ context: { fullscreenPanelId: 'p1' } }); + + expect(screen.getByTitle('Close panel')).toBeTruthy(); + expect(screen.getByTitle('Exit full screen')).toBeTruthy(); + expect(calls.remove).toEqual([]); }); test('noHeader drops the whole header bar but keeps the app', () => { @@ -197,40 +198,31 @@ describe('which chrome each mode gets', () => { }); }); -describe('full screen is the second depth of maximize', () => { +describe('full screen is the only depth, and the only control', () => { const wrapper = () => document.querySelector('[data-panel-id] > div'); - const fullscreen = { maximizedPanelId: 'p1', maximizeMode: 'screen' as const }; - - test('offered from a tiled panel, so taking the window is one click and not two', () => { - mount(); + const fullscreen = { fullscreenPanelId: 'p1' }; + test('one toggle in, one toggle out', () => { + const view = mount(); fireEvent.click(screen.getByTitle('Full screen')); + expect(calls.fullscreen).toEqual(['p1']); + view.unmount(); - expect(calls.maximize).toEqual([['p1', 'screen']]); - }); - - test('the toggle steps back to a maximized panel, and the amber light is still the way out', () => { - // Two different exits on purpose, both one click: the toggle undoes the depth it added, the way a mac - // window does, and Restore undoes maximize entirely. Neither is a trap — a fullscreen panel that - // could only be escaped through a state you never asked for would be. mount({ context: fullscreen }); - fireEvent.click(screen.getByTitle('Exit full screen')); - fireEvent.click(screen.getByTitle('Restore')); - - expect(calls.maximize).toEqual([ - ['p1', 'panel'], - [null, undefined], - ]); + expect(calls.fullscreen).toEqual(['p1', null]); }); - test('locked screens get it too — it edits nothing', () => { + test('locked screens get it, and it is the only chrome they get', () => { + // Their whole restriction is "not yours to edit". Full screen edits nothing — no layout write, not + // even a persisted-per-dashboard one — so it is the one control that survives the lock. mount({ locked: true }); fireEvent.click(screen.getByTitle('Full screen')); - expect(calls.maximize).toEqual([['p1', 'screen']]); + expect(calls.fullscreen).toEqual(['p1']); expect(screen.queryByTitle('Close panel')).toBeNull(); + expect(screen.queryByTitle('Clear app')).toBeNull(); }); test('no app opts out: unlike zoom, this is a property of the frame', () => { @@ -249,19 +241,19 @@ describe('full screen is the second depth of maximize', () => { expect(screen.queryByTitle('Full screen')).toBeNull(); }); - test('it drops the gutter and the rounding that say "floating above the app"', () => { - // The geometry is the feature — `inset-0` is only the whole window because the shell hides its own - // header for it (see panel-fullscreen.ts). Maximize keeps the gutter and clears the nav at - // `top-[56px]`, which is what this is pinned against. + test('it takes the window, dropping the gutter and rounding that say "floating above the app"', () => { + // `inset-0` is only the whole window because the shell hides its own header for it (see + // panel-fullscreen.ts). `top-[56px]` was the old shallower depth clearing the nav it could not paint + // over; it should appear nowhere now. const view = mount({ context: fullscreen }); expect(wrapper()?.className).toContain('inset-0'); expect(wrapper()?.className).not.toContain('rounded-lg'); expect(wrapper()?.className).not.toContain('top-[56px]'); view.unmount(); - mount({ context: { maximizedPanelId: 'p1' } }); - expect(wrapper()?.className).toContain('top-[56px]'); + mount(); expect(wrapper()?.className).toContain('rounded-lg'); + expect(wrapper()?.className).not.toContain('fixed'); }); }); diff --git a/src/workspaces/officerdev/src/components/Workspace/PanelSlot.tsx b/src/workspaces/officerdev/src/components/Workspace/PanelSlot.tsx index 44e4ae6b..c45cd433 100644 --- a/src/workspaces/officerdev/src/components/Workspace/PanelSlot.tsx +++ b/src/workspaces/officerdev/src/components/Workspace/PanelSlot.tsx @@ -1,6 +1,6 @@ import type { ComponentType } from 'react'; import { useCallback } from 'react'; -import { ArrowLeftRight, ChevronLeft, X, Maximize2, Minimize2, Minus, ZoomIn, ZoomOut } from 'lucide-react'; +import { ArrowLeftRight, ChevronLeft, X, Maximize2, Minimize2, ZoomIn, ZoomOut } from 'lucide-react'; import type { LayoutPanel, AppRegistryMap, PanelComponents, PanelComponentEntry } from './types'; import { useWorkspace } from './WorkspaceContext'; import { ZOOM_MIN, ZOOM_MAX, ZOOM_STEP } from './layout-utils'; @@ -60,7 +60,7 @@ const PanelContextMenu = ({ {children} - + onSplit(panelId, 'horizontal')}>Split horizontal onSplit(panelId, 'vertical')}>Split vertical @@ -132,9 +132,6 @@ const TrafficLights = ({ onRemove: (panelId: string) => void; onClearApp: () => void; }) => { - const { maximizedPanelId, setMaximizedPanelId } = useWorkspace(); - const isMaximized = maximizedPanelId === panelId; - // The button says "Close panel" unless this is the only panel left, and it used to call `onClearApp` // either way — `isLastPanel` reached here only to choose the tooltip. So closing a panel from its own // chrome was impossible: the panel stayed, emptied, and the only working path was the context menu. @@ -145,32 +142,14 @@ const TrafficLights = ({ else onRemove(panelId); }, [isLastPanel, onClearApp, onRemove, panelId]); - const handleRestore = useCallback(() => { - setMaximizedPanelId(null); - }, [setMaximizedPanelId]); - - const handleMaximize = useCallback(() => { - setMaximizedPanelId(panelId); - }, [panelId, setMaximizedPanelId]); - - if (isMaximized) { - return ( -
- -
- ); - } - + // One light, and it is not a traffic light any more. The green "maximize" and amber "restore" that used + // to sit here drove a second, shallower depth that stopped below the nav header — the most a panel can + // reach on its own, from inside the content region's stacking context. Once `FullscreenToggle` could + // take the whole window, that half-measure was a state nobody chose on purpose, so both are gone and + // full screen is the only depth. + // + // Red survives because it does something neither of them did, and it is already absent exactly where it + // should be: locked screens render no traffic lights at all. return (
-
); }; -// Locked screens get maximize only — no close, no clear. Green means "there is room to grow", amber -// means "this one is already filling the screen", the same way the two colours read on a mac window; -// `TrafficLights` below switches on the same pair. -const MaximizeButton = ({ panelId }: { panelId: string }) => { - const { maximizedPanelId, setMaximizedPanelId } = useWorkspace(); - const isMaximized = maximizedPanelId === panelId; - - return ( -
- -
- ); -}; - -// Full screen is the second DEPTH of maximize, not a rival to it. `panel` fills the dashboard's content -// region and leaves the nav header reachable; `screen` takes the whole browser window, header included. -// -// Offered from every state rather than only from a maximized panel, so it is one click from a tiled -// panel — and it steps back to a maximized panel rather than all the way out, the way a mac window does. -// The amber light is the way out, and it is present at both depths, so neither state is a trap. +// The only way to blow a panel up, and the only way back. It takes the whole browser window, nav header +// included — the shell hides its own chrome for it (`panel-fullscreen.ts`). // // Hidden until the header is hovered, like the zoom buttons — except while it is active, where it stays -// visible because the panel's own header is then the only chrome on screen. +// visible: the panel's own header is then the only chrome on screen, so a control that had to be +// discovered by hovering would be the way out of a state with no other exit. const FullscreenToggle = ({ panelId }: { panelId: string }) => { - const { maximizedPanelId, maximizeMode, setMaximizedPanelId } = useWorkspace(); - const isFullscreen = maximizedPanelId === panelId && maximizeMode === 'screen'; + const { fullscreenPanelId, setFullscreenPanelId } = useWorkspace(); + const isFullscreen = fullscreenPanelId === panelId; const Icon = isFullscreen ? Minimize2 : Maximize2; return (