From 36ace3f2258c183e7cdf49834d66381cfc2fc8eb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 29 May 2026 09:04:20 -0700 Subject: [PATCH] Resolve lazy modal mounts before commit --- src/renderer/src/App.tsx | 43 ++++++------------- .../src/lazy-modal-mount-state.test.ts | 38 ++++++++++++++++ src/renderer/src/lazy-modal-mount-state.ts | 26 +++++++++++ 3 files changed, 78 insertions(+), 29 deletions(-) create mode 100644 src/renderer/src/lazy-modal-mount-state.test.ts create mode 100644 src/renderer/src/lazy-modal-mount-state.ts diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 2d27852b98e..2f36e1ca298 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -128,6 +128,7 @@ import { } from '../../shared/keybindings' import { isGitRepoKind } from '../../shared/repo-kind' import { showTerminalShortcutCaptureNotification } from '@/lib/terminal-shortcut-capture-notification' +import { resolveMountedLazyModalIds, type LazyModalId } from './lazy-modal-mount-state' const isMac = navigator.userAgent.includes('Mac') const isWindows = !isMac && navigator.userAgent.includes('Windows') @@ -473,7 +474,7 @@ function App(): React.JSX.Element { const canGoForwardWorktree = useAppStore(canGoForwardWorktreeHistory) const titlebarLeftControlsRef = useRef(null) const [collapsedSidebarHeaderWidth, setCollapsedSidebarHeaderWidth] = useState(0) - const [mountedLazyModalIds, setMountedLazyModalIds] = useState(() => new Set()) + const [mountedLazyModalIds, setMountedLazyModalIds] = useState>(() => new Set()) const [onboarding, setOnboarding] = useState(null) const featureTipsPromptedThisSessionRef = useRef(false) const featureTipsSuppressedByOnboardingThisSessionRef = useRef(false) @@ -1407,28 +1408,12 @@ function App(): React.JSX.Element { return () => observer.disconnect() }, [isFullScreen, settings?.showTitlebarAppName, showSidebar, workspaceActive, sidebarOpen]) - useEffect(() => { - if ( - activeModal !== 'quick-open' && - activeModal !== 'worktree-palette' && - activeModal !== 'new-workspace-composer' && - activeModal !== 'workspace-cleanup' && - activeModal !== 'feature-wall' && - activeModal !== 'feature-tips' - ) { - return - } - setMountedLazyModalIds((currentIds) => { - if (currentIds.has(activeModal)) { - return currentIds - } - const nextIds = new Set(currentIds) - // Why: lazy-load these modals only after first use, then keep them mounted - // so repeat opens preserve their local state and avoid re-fetch flashes. - nextIds.add(activeModal) - return nextIds - }) - }, [activeModal]) + const resolvedMountedLazyModalIds = resolveMountedLazyModalIds(activeModal, mountedLazyModalIds) + if (resolvedMountedLazyModalIds !== mountedLazyModalIds) { + // Why: lazy-load these modals only after first use, then keep them mounted + // so repeat opens preserve their local state and avoid re-fetch flashes. + setMountedLazyModalIds(new Set(resolvedMountedLazyModalIds)) + } // Why: extracted so both the full-width titlebar (settings/landing) and // the sidebar-width left header (workspace view) can share the same @@ -1850,7 +1835,7 @@ function App(): React.JSX.Element { {/* Why: root overlays can render Radix s; keep them inside the shared provider so lazy surfaces mount safely from any entry point. */} - {mountedLazyModalIds.has('new-workspace-composer') ? ( + {resolvedMountedLazyModalIds.has('new-workspace-composer') ? ( ) : null} - {mountedLazyModalIds.has('workspace-cleanup') ? ( + {resolvedMountedLazyModalIds.has('workspace-cleanup') ? ( - {mountedLazyModalIds.has('quick-open') ? ( + {resolvedMountedLazyModalIds.has('quick-open') ? ( ) : null} - {mountedLazyModalIds.has('worktree-palette') ? ( + {resolvedMountedLazyModalIds.has('worktree-palette') ? ( ) : null} - {mountedLazyModalIds.has('feature-wall') ? ( + {resolvedMountedLazyModalIds.has('feature-wall') ? ( ) : null} - {mountedLazyModalIds.has('feature-tips') ? ( + {resolvedMountedLazyModalIds.has('feature-tips') ? ( { + it('recognizes only lazily retained root modal ids', () => { + expect(isLazyModalId('quick-open')).toBe(true) + expect(isLazyModalId('feature-tips')).toBe(true) + expect(isLazyModalId('delete-worktree')).toBe(false) + expect(isLazyModalId('none')).toBe(false) + }) +}) + +describe('resolveMountedLazyModalIds', () => { + it('preserves set identity when the active modal is not lazy-mounted', () => { + const mounted = new Set(['quick-open']) + + expect(resolveMountedLazyModalIds('none', mounted)).toBe(mounted) + }) + + it('preserves set identity when the lazy modal is already mounted', () => { + const mounted = new Set(['workspace-cleanup']) + + expect(resolveMountedLazyModalIds('workspace-cleanup', mounted)).toBe(mounted) + }) + + it('adds newly opened lazy modal ids without mutating the existing set', () => { + const mounted = new Set(['quick-open']) + const resolved = resolveMountedLazyModalIds('feature-wall', mounted) + + expect(resolved).toEqual(new Set(['quick-open', 'feature-wall'])) + expect(resolved).not.toBe(mounted) + expect(mounted).toEqual(new Set(['quick-open'])) + }) +}) diff --git a/src/renderer/src/lazy-modal-mount-state.ts b/src/renderer/src/lazy-modal-mount-state.ts new file mode 100644 index 00000000000..8f106d8d243 --- /dev/null +++ b/src/renderer/src/lazy-modal-mount-state.ts @@ -0,0 +1,26 @@ +const LAZY_MODAL_IDS = [ + 'quick-open', + 'worktree-palette', + 'new-workspace-composer', + 'workspace-cleanup', + 'feature-wall', + 'feature-tips' +] as const + +export type LazyModalId = (typeof LAZY_MODAL_IDS)[number] + +const LAZY_MODAL_ID_SET = new Set(LAZY_MODAL_IDS) + +export function isLazyModalId(value: string): value is LazyModalId { + return LAZY_MODAL_ID_SET.has(value) +} + +export function resolveMountedLazyModalIds( + activeModal: string, + mountedIds: ReadonlySet +): ReadonlySet { + if (!isLazyModalId(activeModal) || mountedIds.has(activeModal)) { + return mountedIds + } + return new Set([...mountedIds, activeModal]) +}