From cecbd1ad9301769acdac67fccfe5e10131d0ba0d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 4 May 2026 13:27:34 -0700 Subject: [PATCH] Fix new workspace composer focus restore --- .../components/NewWorkspaceComposerModal.tsx | 322 +++++++----------- .../new-workspace/AnimatedTabPanels.tsx | 162 +++++++++ .../new-workspace/CreateFromTab.tsx | 61 ++-- 3 files changed, 316 insertions(+), 229 deletions(-) create mode 100644 src/renderer/src/components/new-workspace/AnimatedTabPanels.tsx diff --git a/src/renderer/src/components/NewWorkspaceComposerModal.tsx b/src/renderer/src/components/NewWorkspaceComposerModal.tsx index 8a00a32370b..ee031a0ff22 100644 --- a/src/renderer/src/components/NewWorkspaceComposerModal.tsx +++ b/src/renderer/src/components/NewWorkspaceComposerModal.tsx @@ -1,21 +1,15 @@ -import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useAppStore } from '@/store' -import { - Dialog, - DialogContent, - DialogDescription, - DialogHeader, - DialogTitle -} from '@/components/ui/dialog' +import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog' import { Tabs, TabsList, TabsTrigger } from '@/components/ui/tabs' import NewWorkspaceComposerCard from '@/components/NewWorkspaceComposerCard' import AgentSettingsDialog from '@/components/agent/AgentSettingsDialog' import CreateFromTab from '@/components/new-workspace/CreateFromTab' +import AnimatedTabPanels from '@/components/new-workspace/AnimatedTabPanels' import { useComposerState } from '@/hooks/useComposerState' import { AGENT_CATALOG } from '@/lib/agent-catalog' import type { LinkedWorkItemSummary } from '@/lib/new-workspace' import { shouldSuppressEnterSubmit } from '@/lib/new-workspace-enter-guard' -import { cn } from '@/lib/utils' import type { TuiAgent } from '../../../shared/types' type ComposerModalData = { @@ -44,6 +38,36 @@ function ShortcutHint({ children }: { children: React.ReactNode }): React.JSX.El ) } +function isRestorablePanelFocusTarget(panel: HTMLElement, target: HTMLElement | null): boolean { + return Boolean( + target && + target.isConnected && + panel.contains(target) && + !target.closest('[aria-hidden="true"]') && + !target.hasAttribute('disabled') && + target.getAttribute('aria-disabled') !== 'true' + ) +} + +function getComposerPanelFocusTarget( + panel: HTMLElement, + remembered: HTMLElement | null +): HTMLElement | null { + if (isRestorablePanelFocusTarget(panel, remembered)) { + return remembered + } + for (const selector of [ + '[data-repo-combobox-root="true"][role="combobox"]', + '[data-create-from-search-input="true"]' + ]) { + const target = panel.querySelector(selector) + if (isRestorablePanelFocusTarget(panel, target)) { + return target + } + } + return null +} + export default function NewWorkspaceComposerModal(): React.JSX.Element | null { const visible = useAppStore((s) => s.activeModal === 'new-workspace-composer') const modalData = useAppStore((s) => s.modalData as ComposerModalData | undefined) @@ -115,6 +139,82 @@ function ComposerModalBody({ onClose() }, [onClose]) + // Why: the composer preserves both tab panels across swaps, so users expect + // their focus context to survive the round-trip too — switching Quick → Create-from + // → Quick should land them back on the Quick field they were last on, not blow + // focus back to the top of the form. Track the most recent focused element + // inside each panel and restore it on tab activation, falling back to the + // panel's RepoCombobox trigger when nothing is remembered yet. + const lastFocusedByTabRef = useRef>({ + quick: null, + 'create-from': null + }) + + useEffect(() => { + const handler = (event: FocusEvent): void => { + const target = event.target + if (!(target instanceof HTMLElement)) { + return + } + // Why: Radix popovers (RepoCombobox list, search results) portal outside + // the panels, so their focus events resolve to `closest()` → null and + // we don't pollute the per-panel memory with transient popover focus. + const panel = target.closest('[data-composer-panel]') + if (!panel) { + return + } + const tab = panel.dataset.composerPanel + if (tab === 'quick' || tab === 'create-from') { + lastFocusedByTabRef.current[tab] = target + } + } + document.addEventListener('focusin', handler, true) + return () => document.removeEventListener('focusin', handler, true) + }, []) + + const prevActiveTabRef = useRef(null) + useEffect(() => { + const prev = prevActiveTabRef.current + prevActiveTabRef.current = activeTab + // Why: skip the initial mount — the dialog's onOpenAutoFocus below handles + // that case. This effect only runs for user-driven tab swaps. + if (prev === null || prev === activeTab) { + return + } + // Why: when the panel class flips from visible → invisible, Chrome blurs + // whatever element was focused inside the leaving panel. Radix Dialog's + // FocusScope notices the blur and synchronously focuses the first tabbable + // inside the dialog (a tab trigger) on a microtask. A plain synchronous + // focus in this effect lands first, then FocusScope clobbers it. + // setTimeout(..., 0) schedules our focus as a macrotask, which runs after + // FocusScope's microtask restoration settles. + let frame = 0 + const restoreFocus = (): void => { + const panel = document.querySelector(`[data-composer-panel="${activeTab}"]`) + if (!panel) { + return + } + // Why: the Quick panel is remounted via `quickKey` after a Create-from + // fallback, and Create-from can hide parts of itself while preserving + // DOM. Validate the remembered node before restoring it. + const target = getComposerPanelFocusTarget(panel, lastFocusedByTabRef.current[activeTab]) + target?.focus({ preventScroll: true }) + } + const timer = window.setTimeout(() => { + restoreFocus() + // Why: Radix popover close-auto-focus and tab panel visibility changes + // can run in the same turn as the tab swap. A frame-late restore makes + // the active panel win without remounting either tab. + frame = window.requestAnimationFrame(restoreFocus) + }, 0) + return () => { + window.clearTimeout(timer) + if (frame !== 0) { + window.cancelAnimationFrame(frame) + } + } + }, [activeTab]) + return ( { - // Why: Radix's FocusScope fires this once the dialog has mounted and - // the DOM is ready. preventDefault stops it from focusing the first - // tabbable in the Quick tab (the repo combobox trigger) when the - // Create-from tab is active — that tab wants the search input to - // own initial focus. The QuickTabBody handles its own focus below - // when the Quick tab is active. - if (activeTab === 'create-from') { - return - } + // Why: Radix's FocusScope fires this once the dialog has mounted. + // preventDefault stops it from focusing whatever first-tabbable it + // picks (tab trigger, close button), and we instead focus the + // RepoCombobox inside the active panel so both Quick and Create-from + // open with the same top-of-form anchor. event.preventDefault() const content = event.currentTarget as HTMLElement - const trigger = content.querySelector( - '[data-repo-combobox-root="true"][role="combobox"]' - ) - trigger?.focus({ preventScroll: true }) + const panel = content.querySelector(`[data-composer-panel="${activeTab}"]`) + getComposerPanelFocusTarget( + panel ?? content, + lastFocusedByTabRef.current[activeTab] + )?.focus({ preventScroll: true }) }} > - Form + Create {tabShortcut.quick} @@ -176,11 +273,6 @@ function ComposerModalBody({ Create Workspace - - {activeTab === 'quick' - ? 'Pick a repository and agent to spin up a new workspace.' - : 'Start from an existing PR, issue, branch, or Linear ticket.'} - @@ -321,24 +413,6 @@ function QuickTabBody({ return () => window.removeEventListener('keydown', onKeyDown, { capture: true }) }, [active, composerRef, createDisabled, handleCreate, onClose]) - // Why: when the Quick tab becomes active (initial mount, or switched to - // from Create-from), focus the repo combobox trigger so the confirmed - // selection sits ready and the keyboard flow starts at the top of the - // form — matching Dialog's onOpenAutoFocus behavior in the pre-tabs modal. - useEffect(() => { - if (!active) { - return - } - const root = composerRef.current - if (!root) { - return - } - const trigger = root.querySelector( - '[data-repo-combobox-root="true"][role="combobox"]' - ) - trigger?.focus({ preventScroll: true }) - }, [active, composerRef]) - return ( <> -}): React.JSX.Element { - const wrapperRef = useRef(null) - const quickRef = useRef(null) - const createFromRef = useRef(null) - const previousActiveRef = useRef(active) - const [wrapperHeight, setWrapperHeight] = useState(null) - const [isAnimating, setIsAnimating] = useState(false) - - // Why: track the active panel's intrinsic height via ResizeObserver so - // the wrapper follows content (Advanced drawer open/close, async search - // results landing). When active changes, the FLIP effect below runs - // first (capturing the outgoing height), then this observer swings the - // wrapper height toward the new active panel's height via the CSS - // transition on `height`. - // - // Why offsetHeight and not getBoundingClientRect().height: Radix's - // dialog open animation (`data-[state=open]:zoom-in-95`) scales the - // dialog from 0.95 → 1.0 over 200ms. getBoundingClientRect reports the - // *visual* (transformed) height, so the very first measurement lands at - // ~95 % of the real layout size and pins that into the inline height. - // The ResizeObserver never refires because actual layout size didn't - // change — only the transform did — so the wrapper stays permanently - // ~16 px short and clips the Create Workspace button at the bottom. - // offsetHeight returns the un-transformed layout box, which is what the - // wrapper should match. - useLayoutEffect(() => { - const target = active === 'quick' ? quickRef.current : createFromRef.current - if (!target) { - return - } - const update = (): void => { - const next = target.offsetHeight - if (next > 0) { - setWrapperHeight(next) - } - } - update() - const observer = new ResizeObserver(update) - observer.observe(target) - return () => observer.disconnect() - }, [active]) - - // Why: on tab swap, override the observer-driven height for one frame - // so the transition starts from the outgoing panel's size. Without this - // the wrapper would snap to the new panel's height before the ResizeObserver - // could read it. - useLayoutEffect(() => { - const prev = previousActiveRef.current - previousActiveRef.current = active - if (prev === active) { - return - } - const wrapper = wrapperRef.current - if (!wrapper) { - return - } - // Why: offsetHeight, not getBoundingClientRect — see comment on the - // observer effect above. Same transform-scale trap applies to this - // FLIP capture if a tab swap happens while any ancestor transform is - // still animating. - const from = wrapper.offsetHeight - if (from > 0) { - setWrapperHeight(from) - setIsAnimating(true) - } - }, [active]) - - useEffect(() => { - const wrapper = wrapperRef.current - if (!wrapper || !isAnimating) { - return - } - const onEnd = (event: TransitionEvent): void => { - if (event.propertyName !== 'height') { - return - } - setIsAnimating(false) - } - wrapper.addEventListener('transitionend', onEnd) - return () => wrapper.removeEventListener('transitionend', onEnd) - }, [isAnimating]) - - return ( -
-
- {children.quick} -
-
- {children['create-from']} -
-
- ) -} diff --git a/src/renderer/src/components/new-workspace/AnimatedTabPanels.tsx b/src/renderer/src/components/new-workspace/AnimatedTabPanels.tsx new file mode 100644 index 00000000000..298156e8a54 --- /dev/null +++ b/src/renderer/src/components/new-workspace/AnimatedTabPanels.tsx @@ -0,0 +1,162 @@ +import React, { useEffect, useLayoutEffect, useRef, useState } from 'react' +import { cn } from '@/lib/utils' + +type TabKey = 'quick' | 'create-from' + +/** + * Keeps both tab panels mounted so their local state (typed query on + * Create-from, repo pick / workspace name on Quick) survives a tab swap. + * Only the active panel is in normal flow; the inactive panel is + * absolutely positioned + `visibility: hidden` + `pointer-events-none` + * so it + * (a) doesn't contribute to DialogContent's scrollHeight (otherwise the + * host dialog grows a scrollbar whenever the inactive panel is + * taller than the active one), and + * (b) keeps its React subtree mounted (no remount, so input focus, + * typed query, and selection survive). + * + * The wrapper's height is JS-driven: a ResizeObserver on the active + * panel's inner node keeps wrapper height in sync with content so the + * wrapper never clips. On tab change we capture the pre-swap wrapper + * height and transition to the new active panel's measured height — a + * FLIP-style animation so the modal resizes smoothly rather than + * snapping. + */ +export default function AnimatedTabPanels({ + active, + children +}: { + active: TabKey + children: Record +}): React.JSX.Element { + const wrapperRef = useRef(null) + const quickRef = useRef(null) + const createFromRef = useRef(null) + const previousActiveRef = useRef(active) + const [wrapperHeight, setWrapperHeight] = useState(null) + const [isAnimating, setIsAnimating] = useState(false) + + // Why: track the active panel's intrinsic height via ResizeObserver so + // the wrapper follows content (Advanced drawer open/close, async search + // results landing). When active changes, the FLIP effect below runs + // first (capturing the outgoing height), then this observer swings the + // wrapper height toward the new active panel's height via the CSS + // transition on `height`. + // + // Why offsetHeight and not getBoundingClientRect().height: Radix's + // dialog open animation (`data-[state=open]:zoom-in-95`) scales the + // dialog from 0.95 → 1.0 over 200ms. getBoundingClientRect reports the + // *visual* (transformed) height, so the very first measurement lands at + // ~95 % of the real layout size and pins that into the inline height. + // The ResizeObserver never refires because actual layout size didn't + // change — only the transform did — so the wrapper stays permanently + // ~16 px short and clips the Create Workspace button at the bottom. + // offsetHeight returns the un-transformed layout box, which is what the + // wrapper should match. + useLayoutEffect(() => { + const target = active === 'quick' ? quickRef.current : createFromRef.current + if (!target) { + return + } + const update = (): void => { + const next = target.offsetHeight + if (next > 0) { + setWrapperHeight(next) + } + } + update() + const observer = new ResizeObserver(update) + observer.observe(target) + return () => observer.disconnect() + }, [active]) + + // Why: on tab swap, override the observer-driven height for one frame + // so the transition starts from the outgoing panel's size. Without this + // the wrapper would snap to the new panel's height before the ResizeObserver + // could read it. + useLayoutEffect(() => { + const prev = previousActiveRef.current + previousActiveRef.current = active + if (prev === active) { + return + } + const wrapper = wrapperRef.current + if (!wrapper) { + return + } + // Why: offsetHeight, not getBoundingClientRect — see comment on the + // observer effect above. Same transform-scale trap applies to this + // FLIP capture if a tab swap happens while any ancestor transform is + // still animating. + const from = wrapper.offsetHeight + if (from > 0) { + setWrapperHeight(from) + setIsAnimating(true) + } + }, [active]) + + useEffect(() => { + const wrapper = wrapperRef.current + if (!wrapper || !isAnimating) { + return + } + const onEnd = (event: TransitionEvent): void => { + if (event.propertyName !== 'height') { + return + } + setIsAnimating(false) + } + wrapper.addEventListener('transitionend', onEnd) + return () => wrapper.removeEventListener('transitionend', onEnd) + }, [isAnimating]) + + return ( +
+
+ {children.quick} +
+
+ {children['create-from']} +
+
+ ) +} diff --git a/src/renderer/src/components/new-workspace/CreateFromTab.tsx b/src/renderer/src/components/new-workspace/CreateFromTab.tsx index a0f8d07fc61..49f6dd8455f 100644 --- a/src/renderer/src/components/new-workspace/CreateFromTab.tsx +++ b/src/renderer/src/components/new-workspace/CreateFromTab.tsx @@ -2,7 +2,7 @@ and Linear sub-tabs + their fetch/resolve/launch plumbing so the "Create from…" entry point lives in one file. Splitting would scatter debounce/caching logic that only these sub-tabs use. */ -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' import { useShallow } from 'zustand/react/shallow' import { CircleDot, @@ -188,30 +188,6 @@ export default function CreateFromTab({ return () => window.clearTimeout(timer) }, [query]) - // Why: auto-focus the search input when this tab becomes visible — but - // only when it's actually visible AND the parent tab-panel height - // transition has settled. Without the `active` guard, the - // AnimatedTabPanels wrapper (which keeps both panels mounted for - // state-preservation) would let the search field steal focus from the - // Quick tab's repo combobox on modal open. The transition-settle delay - // matters because focusing the input fires onFocus → setResultsOpen(true) - // → Popover positions against the anchor. If the anchor is still moving - // (AnimatedTabPanels animates the wrapper height over ~200ms), Radix - // anchors the popover mid-flight and it visibly drifts down as the - // layout settles. Waiting past the transition lets Radix place it - // against the final input position from the start. - useEffect(() => { - if (!active) { - return - } - const el = searchInputRef.current - if (!el) { - return - } - const timer = window.setTimeout(() => el.focus({ preventScroll: true }), 220) - return () => window.clearTimeout(timer) - }, [active]) - // Why: the results popover is portaled outside this panel, so switching // to the Quick tab (via click or the ⌘N hotkey) doesn't unmount or hide // it — users would see the CreateFrom dropdown floating over the Quick @@ -222,6 +198,19 @@ export default function CreateFromTab({ } }, [active]) + // Why: PopoverContent's default `onCloseAutoFocus` restores focus to the + // anchor (the search input) when the popover closes. When the parent + // switches tabs it closes the popover as a side effect, which would steal + // focus away from whatever target the parent's tab-switch focus logic just + // focused on the newly-active panel. Track active via a ref so the handler + // below can preventDefault the restore only in the tab-swap case without + // affecting normal close flows (Escape, row select) where returning focus + // to the input is correct. + const activeRef = useRef(active) + useLayoutEffect(() => { + activeRef.current = active + }, [active]) + const selectedRepo = useMemo( () => eligibleRepos.find((r) => r.id === selectedRepoId) ?? null, [eligibleRepos, selectedRepoId] @@ -987,7 +976,7 @@ export default function CreateFromTab({ )} aria-hidden={!showGhRepoPicker} > -
+
@@ -1049,6 +1043,7 @@ export default function CreateFromTab({ value={selectedLabel ?? query} readOnly={launching} aria-busy={launching} + data-create-from-search-input="true" onChange={(e) => { if (launching) { return @@ -1170,6 +1165,18 @@ export default function CreateFromTab({ // would pull focus to its first child on open. event.preventDefault() }} + onCloseAutoFocus={(event) => { + // Why: the parent closes this popover as a side effect of + // tab-switching (⌘N / ⌘⇧N / clicking the other tab), and the + // default focus-restore to the search input would then + // clobber the focus the parent just placed on the newly- + // active panel. Suppress the restore whenever this tab is + // no longer active; normal closes (Escape, row select) still + // get the default return-to-input behavior. + if (!activeRef.current) { + event.preventDefault() + } + }} onPointerDownOutside={(event) => { // Why: the search input is the popover's anchor (not its // trigger), so Radix treats clicks on it as "outside" and