diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index b8e629e2d..2c2532563 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -69,6 +69,10 @@ import { onOnboardingReopened } from './components/onboarding/show-onboarding-ev import { shouldShowOnboarding } from './components/onboarding/should-show-onboarding' import { MarkdownTemplatePicker } from './components/editor/MarkdownTemplatePicker' import { FloatingTerminalToggleButton } from './components/floating-terminal/FloatingTerminalToggleButton' +import { + persistFloatingTerminalPanelOpen, + readPersistedFloatingTerminalPanelViewState +} from './components/floating-terminal/floating-terminal-panel-view-state' import { TOGGLE_FLOATING_TERMINAL_EVENT, requestFloatingTerminalOpenMaximized @@ -440,7 +444,11 @@ function App(): React.JSX.Element { const clearUnreadDockBadge = useUnreadDockBadge() useRadixBodyPointerEventsRecovery() useWebSessionTabsSync() - const [floatingTerminalOpen, setFloatingTerminalOpen] = useState(false) + // Why restored: leaving the panel closed forces the user to reopen and re-maximize it, + // and that size jump reflows a live TUI's buffer (see floating-terminal-panel-view-state). + const [floatingTerminalOpen, setFloatingTerminalOpen] = useState( + () => readPersistedFloatingTerminalPanelViewState()?.open === true + ) const floatingWorkspaceTourInteractionSnapshotRef = useRef<{ wasPreviouslyInteracted?: boolean persisted?: Promise @@ -535,6 +543,10 @@ function App(): React.JSX.Element { const historyBackShortcutLabel = useShortcutLabel('worktree.history.back') const historyForwardShortcutLabel = useShortcutLabel('worktree.history.forward') const floatingTerminalEnabled = useAppStore((s) => s.settings?.floatingTerminalEnabled === true) + // Why tracked separately: the flag reads false while settings are still loading, and a + // false read at boot must not be treated as the user disabling the feature. The store + // initializes `settings` to null (not undefined) - fetchSettings replaces it atomically. + const floatingTerminalSettingsHydrated = useAppStore((s) => s.settings != null) const floatingTerminalTriggerLocation = useAppStore( (s) => s.settings?.floatingTerminalTriggerLocation ?? 'floating-button' ) @@ -632,8 +644,19 @@ function App(): React.JSX.Element { restoreFloatingTerminalReturnFocus() } setFloatingTerminalOpen(resolvedOpen) + // Why gated on the flag: `settings` is undefined until it hydrates, so the + // feature-off effect force-closes the panel on every boot. Persisting there would + // overwrite the user's restored `open` with a value they never chose. + if (floatingTerminalEnabled) { + persistFloatingTerminalPanelOpen(resolvedOpen) + } }, - [floatingTerminalOpen, rememberFloatingTerminalReturnFocus, restoreFloatingTerminalReturnFocus] + [ + floatingTerminalEnabled, + floatingTerminalOpen, + rememberFloatingTerminalReturnFocus, + restoreFloatingTerminalReturnFocus + ] ) useEffect(() => { @@ -647,10 +670,13 @@ function App(): React.JSX.Element { }, [floatingTerminalEnabled, setFloatingTerminalOpenWithFocus]) useEffect(() => { - if (!floatingTerminalEnabled) { + // Why the hydration gate: this effect fires on every boot while settings are still + // undefined, and closing there discards the restored open state before the real flag + // value arrives. Only a hydrated flag-off is an actual disable. + if (floatingTerminalSettingsHydrated && !floatingTerminalEnabled) { setFloatingTerminalOpenWithFocus(false) } - }, [floatingTerminalEnabled, setFloatingTerminalOpenWithFocus]) + }, [floatingTerminalSettingsHydrated, floatingTerminalEnabled, setFloatingTerminalOpenWithFocus]) const sidebarWidth = useAppStore((s) => s.sidebarWidth) const sidebarOpen = useAppStore((s) => s.sidebarOpen) diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx index d9f846fbb..9fc9e1268 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx @@ -18,6 +18,7 @@ import { consumeFloatingTerminalOpenMaximizedIntent, requestFloatingTerminalOpenMaximized } from '@/lib/floating-terminal' +import { FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY } from './floating-terminal-panel-view-state' import { clearFloatingPanelReclaimIntent, consumeFloatingPanelReclaimIntent @@ -1197,14 +1198,45 @@ describe('FloatingTerminalPanel close behavior', () => { element = await renderPanel(true) expect(getPanelStyleBounds(element)).toEqual(getMaximizedFloatingTerminalBounds()) - expect(getMockedLocalStorage().setItem).not.toHaveBeenCalled() + // Why key-scoped rather than "no writes": maximize now persists panel view state under + // its own key. The invariant here is that the saved NORMAL bounds are never clobbered. + expect(getMockedLocalStorage().setItem).not.toHaveBeenCalledWith( + FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY, + expect.anything() + ) const restoredControls = findByTypeName(element, 'FloatingTerminalWindowControls') ;(restoredControls.props.onToggleMaximized as () => void)() element = await renderPanel(true) expect(getPanelStyleBounds(element)).toEqual(savedBounds) - expect(getMockedLocalStorage().setItem).not.toHaveBeenCalled() + // Why key-scoped rather than "no writes": maximize now persists panel view state under + // its own key. The invariant here is that the saved NORMAL bounds are never clobbered. + expect(getMockedLocalStorage().setItem).not.toHaveBeenCalledWith( + FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY, + expect.anything() + ) + }) + + it('restores saved normal bounds after starting maximized', async () => { + const savedBounds = { left: 120, top: 96, width: 760, height: 420 } + getMockedLocalStorage().getItem.mockImplementation((key: string) => { + if (key === FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY) { + return JSON.stringify(savedBounds) + } + return key === FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY + ? JSON.stringify({ open: true, maximized: true }) + : null + }) + + let element = await renderPanel(true) + expect(getPanelStyleBounds(element)).toEqual(getMaximizedFloatingTerminalBounds()) + + const controls = findByTypeName(element, 'FloatingTerminalWindowControls') + ;(controls.props.onToggleMaximized as () => void)() + element = await renderPanel(true) + + expect(getPanelStyleBounds(element)).toEqual(savedBounds) }) it('restores committed normal bounds after maximizing from a skinny clamp', async () => { @@ -1239,7 +1271,12 @@ describe('FloatingTerminalPanel close behavior', () => { width: 920, height: 560 }) - expect(getMockedLocalStorage().setItem).not.toHaveBeenCalled() + // Why key-scoped rather than "no writes": maximize now persists panel view state under + // its own key. The invariant here is that the saved NORMAL bounds are never clobbered. + expect(getMockedLocalStorage().setItem).not.toHaveBeenCalledWith( + FLOATING_TERMINAL_PANEL_BOUNDS_STORAGE_KEY, + expect.anything() + ) }) it('does not bootstrap a terminal tab when the panel opens empty', async () => { diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index 66c83ac06..94c0a2316 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -95,18 +95,24 @@ export { FloatingTerminalToggleButton } from './FloatingTerminalToggleButton' import { anchorFloatingTerminalPanelBounds, clampFloatingTerminalBounds, - getDefaultFloatingTerminalCommittedBounds, getDefaultFloatingTerminalBounds, + getDefaultFloatingTerminalCommittedBounds, getMaximizedFloatingTerminalBounds, persistFloatingTerminalPanelBounds, readPersistedFloatingTerminalPanelBounds, - resolveFloatingTerminalPanelCommittedBounds, resolveFloatingTerminalPanelBounds, + resolveFloatingTerminalPanelCommittedBounds, shouldReconcileFloatingTerminalPanelBounds, type FloatingTerminalPanelBounds, - type FloatingTerminalPanelCommittedBounds, - type FloatingTerminalPanelBoundsSource + type FloatingTerminalPanelBoundsSource, + type FloatingTerminalPanelCommittedBounds } from './floating-terminal-panel-bounds' +import { + persistFloatingTerminalPanelMaximized, + readPersistedFloatingTerminalPanelViewState +} from './floating-terminal-panel-view-state' +import { useSettledPanelViewport } from './use-settled-panel-viewport' +import { shouldRestoreMaximizedPanelBounds } from './floating-terminal-panel-restore-geometry' import { translate } from '@/i18n/i18n' import { consumeFloatingTerminalOpenMaximizedIntent } from '@/lib/floating-terminal' import { selectFloatingTerminalPanelInputs } from './floating-terminal-panel-inputs' @@ -162,6 +168,17 @@ function readInitialPanelBounds(): FloatingTerminalPanelBoundsState { const defaultCommittedBounds = getDefaultFloatingTerminalCommittedBounds() const defaultRenderedBounds = getDefaultFloatingTerminalBounds() const persistedBounds = readPersistedFloatingTerminalPanelBounds() + if (shouldRestoreMaximizedPanelBounds(readPersistedFloatingTerminalPanelViewState())) { + // Why maximized wins the RENDERED rect while the committed rect stays the restore + // target: the first paint must already be final geometry, or the panes fit at the + // smaller size and the later maximize reflows them. Un-maximizing still returns to + // the user's own bounds because those remain committed. + return { + committedBounds: persistedBounds ?? defaultCommittedBounds, + renderedBounds: getMaximizedFloatingTerminalBounds(), + source: persistedBounds ? 'user' : 'default' + } + } return persistedBounds ? { committedBounds: persistedBounds, @@ -268,7 +285,10 @@ export function FloatingTerminalPanel({ initialBoundsStateRef.current.committedBounds ) const [bounds, setBounds] = useState(initialBoundsStateRef.current.renderedBounds) - const [maximized, setMaximized] = useState(false) + const [maximized, setMaximized] = useState( + () => readPersistedFloatingTerminalPanelViewState()?.maximized === true + ) + const panelViewportSettled = useSettledPanelViewport() const [orchestrationDialogOpen, setOrchestrationDialogOpen] = useState(false) const [showOrchestrationSetup, setShowOrchestrationSetup] = useState( () => !hasOrchestrationSetupMarker() && !isOrchestrationSetupDismissed() @@ -1115,9 +1135,9 @@ export function FloatingTerminalPanel({ const toggleMaximized = useCallback(() => { if (maximized) { const restoredState = restoreBoundsRef.current ?? { - committedBounds: getDefaultFloatingTerminalCommittedBounds(), - renderedBounds: getDefaultFloatingTerminalBounds(), - source: 'default' as const + committedBounds: committedBoundsRef.current, + renderedBounds: resolveFloatingTerminalPanelCommittedBounds(committedBoundsRef.current), + source: boundsSourceRef.current } restoreBoundsRef.current = null boundsSourceRef.current = restoredState.source @@ -1128,6 +1148,7 @@ export function FloatingTerminalPanel({ stagedBoundsRef.current = null setBounds(restoredBounds) setMaximized(false) + persistFloatingTerminalPanelMaximized(false) return } restoreBoundsRef.current = { @@ -1138,6 +1159,7 @@ export function FloatingTerminalPanel({ stagedBoundsRef.current = null setBounds(getMaximizedFloatingTerminalBounds()) setMaximized(true) + persistFloatingTerminalPanelMaximized(true) }, [bounds, maximized]) const maximizePanel = useCallback(() => { @@ -1155,6 +1177,7 @@ export function FloatingTerminalPanel({ stagedBoundsRef.current = null setBounds(getMaximizedFloatingTerminalBounds()) setMaximized(true) + persistFloatingTerminalPanelMaximized(true) }, [bounds, maximized]) useEffect(() => { @@ -1840,7 +1863,11 @@ export function FloatingTerminalPanel({ hasVisibleFloatingTabs ? 'floating-workspace-surface' : undefined } > - {cwd + {/* Why also gated on a settled viewport: a restored-maximized panel derives its + rect from the live viewport, so mounting terminals before the window finishes + maximizing fits them to a grid it is about to leave, and the correcting fit + reflows the buffer under a live TUI. */} + {cwd && panelViewportSettled ? tabs .filter((tab) => !parkedTerminalTabIds.has(tab.id)) .map((tab) => { diff --git a/src/renderer/src/components/floating-terminal/floating-terminal-panel-restore-geometry.ts b/src/renderer/src/components/floating-terminal/floating-terminal-panel-restore-geometry.ts new file mode 100644 index 000000000..b9114cf58 --- /dev/null +++ b/src/renderer/src/components/floating-terminal/floating-terminal-panel-restore-geometry.ts @@ -0,0 +1,18 @@ +import { hasUsableFloatingTerminalPanelViewport } from './floating-terminal-panel-bounds' +import type { FloatingTerminalPanelViewState } from './floating-terminal-panel-view-state' + +/** + * Whether the panel's first rendered rect should be the maximized one. + * + * Why this is a decision and not just a flag read: maximized geometry is derived from the + * live viewport, so restoring it against a viewport too small to hold the panel would pin + * the terminals to a grid the window is about to leave — the size jump this restore exists + * to remove. When the viewport cannot answer yet, fall back to the committed bounds and let + * the ordinary reconcile path maximize once layout is real. + */ +export function shouldRestoreMaximizedPanelBounds( + viewState: FloatingTerminalPanelViewState | null, + hasUsableViewport: () => boolean = hasUsableFloatingTerminalPanelViewport +): boolean { + return viewState?.maximized === true && hasUsableViewport() +} diff --git a/src/renderer/src/components/floating-terminal/floating-terminal-panel-view-state.test.ts b/src/renderer/src/components/floating-terminal/floating-terminal-panel-view-state.test.ts new file mode 100644 index 000000000..32fff3881 --- /dev/null +++ b/src/renderer/src/components/floating-terminal/floating-terminal-panel-view-state.test.ts @@ -0,0 +1,93 @@ +/** @vitest-environment happy-dom */ +import { afterEach, describe, expect, it } from 'vitest' +import { + FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY, + persistFloatingTerminalPanelMaximized, + persistFloatingTerminalPanelOpen, + readPersistedFloatingTerminalPanelViewState +} from './floating-terminal-panel-view-state' +import { shouldRestoreMaximizedPanelBounds } from './floating-terminal-panel-restore-geometry' + +afterEach(() => { + window.localStorage.clear() +}) + +describe('floating terminal panel view state', () => { + it('returns null when nothing was ever persisted', () => { + expect(readPersistedFloatingTerminalPanelViewState()).toBeNull() + }) + + it('round-trips both flags', () => { + persistFloatingTerminalPanelOpen(true) + persistFloatingTerminalPanelMaximized(true) + expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: true, maximized: true }) + }) + + it('does not clobber the other owner_s flag', () => { + // Why: `open` is written by the app shell and `maximized` by the panel, so a + // whole-record write from either would drop the other's value. + persistFloatingTerminalPanelMaximized(true) + persistFloatingTerminalPanelOpen(false) + expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: false, maximized: true }) + + persistFloatingTerminalPanelOpen(true) + persistFloatingTerminalPanelMaximized(false) + expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: true, maximized: false }) + }) + + it('lets a later write destroy a restored open preference', () => { + // Why pinned: this is the hazard the App-side guard exists for. `settings` hydrates + // asynchronously, so the feature flag reads false on every boot before it resolves and + // the feature-off effect force-closes the panel. If that path persists, it overwrites a + // preference the user never changed - which is exactly what shipped and had to be fixed. + persistFloatingTerminalPanelOpen(true) + expect(readPersistedFloatingTerminalPanelViewState()?.open).toBe(true) + + persistFloatingTerminalPanelOpen(false) + expect(readPersistedFloatingTerminalPanelViewState()?.open).toBe(false) + }) + + it('restores the half a older record carries', () => { + // Why: a record written before the second flag existed must still restore. + window.localStorage.setItem( + FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY, + JSON.stringify({ open: true }) + ) + expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: true, maximized: false }) + }) + + it('treats malformed storage as absent instead of throwing', () => { + for (const value of ['not json', '[]', 'null', '"open"', '42']) { + window.localStorage.setItem(FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY, value) + expect(readPersistedFloatingTerminalPanelViewState()).toBeNull() + } + }) + + it('ignores non-boolean flag values rather than coercing them', () => { + window.localStorage.setItem( + FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY, + JSON.stringify({ open: 'yes', maximized: 1 }) + ) + expect(readPersistedFloatingTerminalPanelViewState()).toEqual({ open: false, maximized: false }) + }) +}) + +describe('shouldRestoreMaximizedPanelBounds', () => { + it('restores maximized geometry only when the viewport can hold it', () => { + expect(shouldRestoreMaximizedPanelBounds({ open: true, maximized: true }, () => true)).toBe( + true + ) + // Why: maximized bounds come from the live viewport, so restoring against one too small + // pins terminals to a grid the window is about to leave - the jump this restore removes. + expect(shouldRestoreMaximizedPanelBounds({ open: true, maximized: true }, () => false)).toBe( + false + ) + }) + + it('does not restore maximized geometry for a non-maximized or absent record', () => { + expect(shouldRestoreMaximizedPanelBounds({ open: true, maximized: false }, () => true)).toBe( + false + ) + expect(shouldRestoreMaximizedPanelBounds(null, () => true)).toBe(false) + }) +}) diff --git a/src/renderer/src/components/floating-terminal/floating-terminal-panel-view-state.ts b/src/renderer/src/components/floating-terminal/floating-terminal-panel-view-state.ts new file mode 100644 index 000000000..dd9f743b4 --- /dev/null +++ b/src/renderer/src/components/floating-terminal/floating-terminal-panel-view-state.ts @@ -0,0 +1,66 @@ +export const FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY = + 'orca-floating-terminal-panel-view-state-v1' + +export type FloatingTerminalPanelViewState = { + open: boolean + maximized: boolean +} + +function getWindowStorage(): Storage | null { + return typeof window === 'undefined' ? null : window.localStorage +} + +/** + * Why persisted separately from the bounds record: maximized geometry is derived + * from the live viewport rather than stored, so this is view state, not a rect. + * Keeping it out of the bounds union also keeps that type a pure rectangle. + * + * Why it matters: an unpersisted maximize means every restart drops the user into + * a default-sized panel that they re-maximize by hand. That size jump reflows the + * xterm buffer under a live relative-cursor TUI, which unwraps its rows and leaves + * permanently mangled scrollback. Restoring the panel as it was removes the jump. + */ +export function readPersistedFloatingTerminalPanelViewState(): FloatingTerminalPanelViewState | null { + try { + const serialized = getWindowStorage()?.getItem(FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY) + if (!serialized) { + return null + } + const parsed: unknown = JSON.parse(serialized) + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + return null + } + const record = parsed as Record + // Why each flag is read independently: a record written before the other flag + // existed must still restore the half it does carry. + return { + open: record.open === true, + maximized: record.maximized === true + } + } catch { + return null + } +} + +export function persistFloatingTerminalPanelViewState(state: FloatingTerminalPanelViewState): void { + try { + getWindowStorage()?.setItem( + FLOATING_TERMINAL_PANEL_VIEW_STATE_STORAGE_KEY, + JSON.stringify(state) + ) + } catch { + // Why: storage can be unavailable or full; losing the restore is not worth a crash. + } +} + +// Why field-scoped writers: `open` is owned by the app shell and `maximized` by the panel, +// so a whole-record write from either side would clobber the other's flag. +export function persistFloatingTerminalPanelOpen(open: boolean): void { + const current = readPersistedFloatingTerminalPanelViewState() + persistFloatingTerminalPanelViewState({ maximized: current?.maximized === true, open }) +} + +export function persistFloatingTerminalPanelMaximized(maximized: boolean): void { + const current = readPersistedFloatingTerminalPanelViewState() + persistFloatingTerminalPanelViewState({ open: current?.open === true, maximized }) +} diff --git a/src/renderer/src/components/floating-terminal/use-settled-panel-viewport.ts b/src/renderer/src/components/floating-terminal/use-settled-panel-viewport.ts new file mode 100644 index 000000000..f9fec2c3e --- /dev/null +++ b/src/renderer/src/components/floating-terminal/use-settled-panel-viewport.ts @@ -0,0 +1,69 @@ +import { useEffect, useState } from 'react' + +// Why two frames: the main window restores its saved bounds and only then maximizes, so +// the renderer's first layout can be a size the window is about to leave. One unchanged +// frame can land inside that gap; two straddle it. +const REQUIRED_STABLE_FRAMES = 2 +// Why a cap: a window that never stops resizing (a drag, a display change) must not hold +// the panel's terminals forever. Mounting at a stale size is recoverable; never mounting is not. +const MAX_SETTLE_MS = 300 + +function readViewport(): { width: number; height: number } { + return { width: window.innerWidth, height: window.innerHeight } +} + +/** + * Reports true once the viewport has held one size across consecutive frames. + * + * Why the floating panel waits for this: its maximized rect is derived from the live + * viewport, so mounting terminals against a pre-maximize viewport fits them to a grid the + * window is about to leave. The correcting fit then reflows the xterm buffer under a live + * TUI, which is the damage this whole path exists to avoid. Latching true is deliberate — + * later resizes are ordinary user resizes and the normal fit path owns them. + */ +export function useSettledPanelViewport(): boolean { + const [settled, setSettled] = useState(false) + + useEffect(() => { + if (settled || typeof window === 'undefined') { + return + } + if (typeof requestAnimationFrame !== 'function') { + setSettled(true) + return + } + let frameId: number | null = null + let previous = readViewport() + let stableFrames = 0 + const settle = (): void => { + if (frameId !== null) { + cancelAnimationFrame(frameId) + frameId = null + } + setSettled(true) + } + const capId = setTimeout(settle, MAX_SETTLE_MS) + const step = (): void => { + const current = readViewport() + stableFrames = + current.width === previous.width && current.height === previous.height + ? stableFrames + 1 + : 0 + previous = current + if (stableFrames >= REQUIRED_STABLE_FRAMES) { + settle() + return + } + frameId = requestAnimationFrame(step) + } + frameId = requestAnimationFrame(step) + return () => { + clearTimeout(capId) + if (frameId !== null) { + cancelAnimationFrame(frameId) + } + } + }, [settled]) + + return settled +}