From d28ced8fd7471302d42e2e272fcbd62f9b1404c8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 9 Mar 2026 22:36:22 -0700 Subject: [PATCH] Move PR checkout flow to dialog and drop /checkout-pr command - Replace the inline composer PR menu with a dedicated `PullRequestThreadDialog` - Remove `/checkout-pr` from composer slash-command parsing and tests - Fix branch selector virtualization sizing for the checkout-PR row --- .../BranchToolbarBranchSelector.tsx | 5 +- apps/web/src/components/ChatView.tsx | 125 +++++-------- ...ndMenu.tsx => PullRequestThreadDialog.tsx} | 176 ++++++++++-------- apps/web/src/composer-logic.test.ts | 8 +- apps/web/src/composer-logic.ts | 6 +- 5 files changed, 154 insertions(+), 166 deletions(-) rename apps/web/src/components/{ComposerPullRequestCommandMenu.tsx => PullRequestThreadDialog.tsx} (62%) diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 1817e29077cdf4bc98a5fd2058c6401deb39d6a1..dbef056ca99dce203fea8358eb06dfc2f33bf7ce 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -291,7 +291,8 @@ export function BranchToolbarBranchSelector({ const branchListScrollElementRef = useRef(null); const branchListVirtualizer = useVirtualizer({ count: filteredBranchPickerItems.length, - estimateSize: () => 28, + estimateSize: (index) => + filteredBranchPickerItems[index] === checkoutPullRequestItemValue ? 44 : 28, getScrollElement: () => branchListScrollElementRef.current, overscan: 12, enabled: isBranchMenuOpen && shouldVirtualizeBranchList, @@ -409,7 +410,7 @@ export function BranchToolbarBranchSelector({ items={branchPickerItems} filteredItems={filteredBranchPickerItems} autoHighlight - virtualized + virtualized={shouldVirtualizeBranchList} onItemHighlighted={(_value, eventDetails) => { if (!isBranchMenuOpen || eventDetails.index < 0) return; branchListVirtualizer.scrollToIndex(eventDetails.index, { align: "auto" }); diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 80d440994458d0d9e1d46dc45b6dd7b2f1ff3bcd..ee3de304eae168c41a0e098fc7e46bb2d72610ac 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -119,7 +119,6 @@ import { type TurnDiffTreeNode, } from "../lib/turnDiffTree"; import BranchToolbar from "./BranchToolbar"; -import { ComposerPullRequestCommandMenu } from "./ComposerPullRequestCommandMenu"; import GitActionsControl from "./GitActionsControl"; import { isOpenFavoriteEditorShortcut, @@ -217,6 +216,7 @@ import { shouldUseCompactComposerFooter } from "./composerFooterLayout"; import { selectThreadTerminalState, useTerminalStateStore } from "../terminalStateStore"; import { clamp } from "effect/Number"; import { ComposerPromptEditor, type ComposerPromptEditorHandle } from "./ComposerPromptEditor"; +import { PullRequestThreadDialog } from "./PullRequestThreadDialog"; import { estimateTimelineMessageHeight } from "./timelineHeight"; function formatMessageMeta(createdAt: string, duration: string | null): string { @@ -408,7 +408,7 @@ type ComposerCommandItem = type SendPhase = "idle" | "preparing-worktree" | "sending-turn"; -interface PullRequestComposerMenuState { +interface PullRequestDialogState { initialReference: string | null; key: number; } @@ -657,8 +657,9 @@ export default function ChatView({ threadId }: ChatViewProps) { const [nowTick, setNowTick] = useState(() => Date.now()); const [terminalFocusRequestId, setTerminalFocusRequestId] = useState(0); const [composerHighlightedItemId, setComposerHighlightedItemId] = useState(null); - const [pullRequestComposerMenuState, setPullRequestComposerMenuState] = - useState(null); + const [pullRequestDialogState, setPullRequestDialogState] = useState( + null, + ); const [attachmentPreviewHandoffByMessageId, setAttachmentPreviewHandoffByMessageId] = useState< Record >({}); @@ -764,19 +765,19 @@ export default function ChatView({ threadId }: ChatViewProps) { const latestTurnSettled = isLatestTurnSettled(activeLatestTurn, activeThread?.session ?? null); const activeProject = projects.find((p) => p.id === activeThread?.projectId); - const openPullRequestComposerMenu = useCallback((reference?: string) => { + const openPullRequestDialog = useCallback((reference?: string) => { if (!canCheckoutPullRequestIntoThread) { return; } - setPullRequestComposerMenuState({ + setPullRequestDialogState({ initialReference: reference ?? null, key: Date.now(), }); setComposerHighlightedItemId(null); }, [canCheckoutPullRequestIntoThread]); - const closePullRequestComposerMenu = useCallback(() => { - setPullRequestComposerMenuState(null); + const closePullRequestDialog = useCallback(() => { + setPullRequestDialogState(null); }, []); const openOrReuseProjectDraftThread = useCallback( @@ -1254,7 +1255,6 @@ export default function ChatView({ threadId }: ChatViewProps) { timelineEntries, ]); const gitCwd = activeThread?.worktreePath ?? activeProject?.cwd ?? null; - const pullRequestComposerMenuOpen = pullRequestComposerMenuState !== null; const composerTriggerKind = composerTrigger?.kind ?? null; const pathTriggerQuery = composerTrigger?.kind === "path" ? composerTrigger.query : ""; const isPathTrigger = composerTriggerKind === "path"; @@ -1311,22 +1311,12 @@ export default function ChatView({ threadId }: ChatViewProps) { label: "/default", description: "Switch this thread back to normal chat mode", }, - { - id: "slash:checkout-pr", - type: "slash-command", - command: "checkout-pr", - label: "/checkout-pr", - description: "Create a new thread from a GitHub pull request", - }, ] satisfies ReadonlyArray>; - const availableSlashCommandItems = canCheckoutPullRequestIntoThread - ? slashCommandItems - : slashCommandItems.filter((item) => item.command !== "checkout-pr"); const query = composerTrigger.query.trim().toLowerCase(); if (!query) { - return [...availableSlashCommandItems]; + return [...slashCommandItems]; } - return availableSlashCommandItems.filter( + return slashCommandItems.filter( (item) => item.command.includes(query) || item.label.slice(1).includes(query), ); } @@ -1347,16 +1337,14 @@ export default function ChatView({ threadId }: ChatViewProps) { label: name, description: `${providerLabel} · ${slug}`, })); - }, [canCheckoutPullRequestIntoThread, composerTrigger, searchableModelOptions, workspaceEntries]); - const composerMenuOpen = Boolean(composerTrigger) || pullRequestComposerMenuOpen; + }, [composerTrigger, searchableModelOptions, workspaceEntries]); + const composerMenuOpen = Boolean(composerTrigger); const activeComposerMenuItem = useMemo( () => - pullRequestComposerMenuOpen - ? null - : (composerMenuItems.find((item) => item.id === composerHighlightedItemId) ?? - composerMenuItems[0] ?? - null), - [composerHighlightedItemId, composerMenuItems, pullRequestComposerMenuOpen], + composerMenuItems.find((item) => item.id === composerHighlightedItemId) ?? + composerMenuItems[0] ?? + null, + [composerHighlightedItemId, composerMenuItems], ); composerMenuOpenRef.current = composerMenuOpen; composerMenuItemsRef.current = composerMenuItems; @@ -2055,7 +2043,7 @@ export default function ChatView({ threadId }: ChatViewProps) { useEffect(() => { setExpandedWorkGroups({}); - setPullRequestComposerMenuState(null); + setPullRequestDialogState(null); if (planSidebarOpenOnNextThreadRef.current) { planSidebarOpenOnNextThreadRef.current = false; setPlanSidebarOpen(true); @@ -2070,16 +2058,12 @@ export default function ChatView({ threadId }: ChatViewProps) { setComposerHighlightedItemId(null); return; } - if (pullRequestComposerMenuOpen) { - setComposerHighlightedItemId(null); - return; - } setComposerHighlightedItemId((existing) => existing && composerMenuItems.some((item) => item.id === existing) ? existing : (composerMenuItems[0]?.id ?? null), ); - }, [composerMenuItems, composerMenuOpen, pullRequestComposerMenuOpen]); + }, [composerMenuItems, composerMenuOpen]); useEffect(() => { setIsRevertingCheckpoint(false); @@ -2589,15 +2573,8 @@ export default function ChatView({ threadId }: ChatViewProps) { } const standaloneSlashCommand = composerImages.length === 0 ? parseStandaloneComposerSlashCommand(trimmed) : null; - const shouldHandleStandaloneSlashCommand = - standaloneSlashCommand !== null && - (standaloneSlashCommand !== "checkout-pr" || canCheckoutPullRequestIntoThread); - if (shouldHandleStandaloneSlashCommand && standaloneSlashCommand) { - if (standaloneSlashCommand === "checkout-pr") { - openPullRequestComposerMenu(); - } else { - await handleInteractionModeChange(standaloneSlashCommand); - } + if (standaloneSlashCommand) { + await handleInteractionModeChange(standaloneSlashCommand); promptRef.current = ""; clearComposerDraftContent(activeThread.id); setComposerHighlightedItemId(null); @@ -3386,11 +3363,7 @@ export default function ChatView({ threadId }: ChatViewProps) { } return; } - if (item.command === "checkout-pr") { - openPullRequestComposerMenu(); - } else { - void handleInteractionModeChange(item.command === "plan" ? "plan" : "default"); - } + void handleInteractionModeChange(item.command === "plan" ? "plan" : "default"); const applied = applyPromptReplacement(trigger.rangeStart, trigger.rangeEnd, "", { expectedText: expectedToken, }); @@ -3410,7 +3383,6 @@ export default function ChatView({ threadId }: ChatViewProps) { [ applyPromptReplacement, handleInteractionModeChange, - openPullRequestComposerMenu, onProviderModelSelect, resolveActiveComposerTrigger, ], @@ -3485,12 +3457,6 @@ export default function ChatView({ threadId }: ChatViewProps) { const { trigger } = resolveActiveComposerTrigger(); const menuIsActive = composerMenuOpenRef.current || trigger !== null; - if (pullRequestComposerMenuOpen) { - if (key === "ArrowDown" || key === "ArrowUp" || key === "Tab" || key === "Enter") { - return true; - } - } - if (menuIsActive) { const currentItems = composerMenuItemsRef.current; if (key === "ArrowDown" && currentItems.length > 0) { @@ -3712,28 +3678,15 @@ export default function ChatView({ threadId }: ChatViewProps) { > {composerMenuOpen && !isComposerApprovalState && (
- {pullRequestComposerMenuState ? ( - { - await handlePreparedPullRequestThread(input); - closePullRequestComposerMenu(); - }} - /> - ) : ( - - )} +
)} @@ -4160,10 +4113,24 @@ export default function ChatView({ threadId }: ChatViewProps) { envLocked={envLocked} onComposerFocusRequest={scheduleComposerFocus} {...(canCheckoutPullRequestIntoThread - ? { onCheckoutPullRequestRequest: openPullRequestComposerMenu } + ? { onCheckoutPullRequestRequest: openPullRequestDialog } : {})} /> )} + {pullRequestDialogState ? ( + { + if (!open) { + closePullRequestDialog(); + } + }} + onPrepared={handlePreparedPullRequestThread} + /> + ) : null} {/* end chat column */} {/* Plan sidebar */} diff --git a/apps/web/src/components/ComposerPullRequestCommandMenu.tsx b/apps/web/src/components/PullRequestThreadDialog.tsx similarity index 62% rename from apps/web/src/components/ComposerPullRequestCommandMenu.tsx rename to apps/web/src/components/PullRequestThreadDialog.tsx index 53dd7ab2ea363c5d8e3d72ce150c1ba7a0252e19..ced24aca7fcc38b73e13213cb0e7cab4e89373ad 100644 --- a/apps/web/src/components/ComposerPullRequestCommandMenu.tsx +++ b/apps/web/src/components/PullRequestThreadDialog.tsx @@ -10,22 +10,33 @@ import { import { cn } from "~/lib/utils"; import { parsePullRequestReference } from "~/pullRequestReference"; import { Button } from "./ui/button"; +import { + Dialog, + DialogDescription, + DialogFooter, + DialogHeader, + DialogPanel, + DialogPopup, + DialogTitle, +} from "./ui/dialog"; import { Input } from "./ui/input"; import { Spinner } from "./ui/spinner"; -interface ComposerPullRequestCommandMenuProps { +interface PullRequestThreadDialogProps { + open: boolean; cwd: string | null; initialReference: string | null; - onCancel: () => void; + onOpenChange: (open: boolean) => void; onPrepared: (input: { branch: string; worktreePath: string | null }) => Promise | void; } -export function ComposerPullRequestCommandMenu({ +export function PullRequestThreadDialog({ + open, cwd, initialReference, - onCancel, + onOpenChange, onPrepared, -}: ComposerPullRequestCommandMenuProps) { +}: PullRequestThreadDialogProps) { const queryClient = useQueryClient(); const referenceInputRef = useRef(null); const [reference, setReference] = useState(initialReference ?? ""); @@ -38,12 +49,14 @@ export function ComposerPullRequestCommandMenu({ ); useEffect(() => { + if (!open) return; setReference(initialReference ?? ""); setReferenceDirty(false); setPreparingMode(null); - }, [initialReference]); + }, [initialReference, open]); useEffect(() => { + if (!open) return; const frame = window.requestAnimationFrame(() => { referenceInputRef.current?.focus(); referenceInputRef.current?.select(); @@ -51,14 +64,14 @@ export function ComposerPullRequestCommandMenu({ return () => { window.cancelAnimationFrame(frame); }; - }, []); + }, [open]); const parsedReference = parsePullRequestReference(reference); const parsedDebouncedReference = parsePullRequestReference(debouncedReference); const resolvePullRequestQuery = useQuery( gitResolvePullRequestQueryOptions({ cwd, - reference: parsedDebouncedReference, + reference: open ? parsedDebouncedReference : null, }), ); const cachedPullRequest = useMemo(() => { @@ -83,6 +96,7 @@ export function ComposerPullRequestCommandMenu({ : null; const resolvedPullRequest = liveResolvedPullRequest ?? cachedPullRequest; const isResolving = + open && parsedReference !== null && resolvedPullRequest === null && (referenceDebouncer.state.isPending || @@ -120,11 +134,13 @@ export function ComposerPullRequestCommandMenu({ branch: result.branch, worktreePath: result.worktreePath, }); + onOpenChange(false); } finally { setPreparingMode(null); } }, [ cwd, + onOpenChange, onPrepared, parsedReference, preparePullRequestThreadMutation, @@ -135,10 +151,10 @@ export function ComposerPullRequestCommandMenu({ !referenceDirty ? null : reference.trim().length === 0 - ? "Paste a GitHub pull request URL or enter 123 / #123." - : parsedReference === null - ? "Use a GitHub pull request URL, 123, or #123." - : null; + ? "Paste a GitHub pull request URL or enter 123 / #123." + : parsedReference === null + ? "Use a GitHub pull request URL, 123, or #123." + : null; const errorMessage = validationMessage ?? (resolvedPullRequest === null && resolvePullRequestQuery.isError @@ -152,73 +168,79 @@ export function ComposerPullRequestCommandMenu({ : null)); return ( -
-
-
-
-
-

Checkout Pull Request

-

- Resolve a PR, then choose Local or Worktree for the new draft thread. -

-
-
- { - setReferenceDirty(true); - setReference(event.target.value); - }} - onKeyDown={(event) => { - if (event.key !== "Enter") { - return; - } - event.preventDefault(); - if (!isResolving && !preparePullRequestThreadMutation.isPending) { - void handleConfirm("local"); - } - }} - /> -
+ { + if (!preparePullRequestThreadMutation.isPending) { + onOpenChange(nextOpen); + } + }} + > + + + Checkout Pull Request + + Resolve a GitHub pull request, then create the draft thread in the main repo or in a + dedicated worktree. + + + + - {resolvedPullRequest ? ( -
-
-
-

{resolvedPullRequest.title}

-

- #{resolvedPullRequest.number} · {resolvedPullRequest.headBranch} to{" "} - {resolvedPullRequest.baseBranch} -

+ {resolvedPullRequest ? ( +
+
+
+

{resolvedPullRequest.title}

+

+ #{resolvedPullRequest.number} · {resolvedPullRequest.headBranch} to{" "} + {resolvedPullRequest.baseBranch} +

+
+ + {resolvedPullRequest.state} +
- - {resolvedPullRequest.state} -
-
- ) : null} + ) : null} - {isResolving ? ( -
- - Resolving pull request... -
- ) : null} - - {errorMessage ?

{errorMessage}

: null} -
+ {isResolving ? ( +
+ + Resolving pull request... +
+ ) : null} -
-

- {resolvedPullRequest - ? "Create the draft thread in the main repo or in a dedicated worktree." - : "Resolve the pull request first."} -

-
- -
-
-
+ + + ); } diff --git a/apps/web/src/composer-logic.test.ts b/apps/web/src/composer-logic.test.ts index ef490f6e954109956def53812378420695e0d4a0..4e4bd145d7bb3177ecfc4c4e89f4d7eef23caf04 100644 --- a/apps/web/src/composer-logic.test.ts +++ b/apps/web/src/composer-logic.test.ts @@ -134,11 +134,11 @@ describe("parseStandaloneComposerSlashCommand", () => { expect(parseStandaloneComposerSlashCommand("/default")).toBe("default"); }); - it("parses standalone /checkout-pr command", () => { - expect(parseStandaloneComposerSlashCommand(" /checkout-pr ")).toBe("checkout-pr"); - }); - it("ignores slash commands with extra message text", () => { expect(parseStandaloneComposerSlashCommand("/plan explain this")).toBeNull(); }); + + it("ignores removed /checkout-pr command", () => { + expect(parseStandaloneComposerSlashCommand(" /checkout-pr ")).toBeNull(); + }); }); diff --git a/apps/web/src/composer-logic.ts b/apps/web/src/composer-logic.ts index cde009708b33e8c996308e6a0e54f3d97080dff4..9bb30132a76444604bc3530209f058f12944a933 100644 --- a/apps/web/src/composer-logic.ts +++ b/apps/web/src/composer-logic.ts @@ -1,7 +1,7 @@ import { splitPromptIntoComposerSegments } from "./composer-editor-mentions"; export type ComposerTriggerKind = "path" | "slash-command" | "slash-model"; -export type ComposerSlashCommand = "model" | "plan" | "default" | "checkout-pr"; +export type ComposerSlashCommand = "model" | "plan" | "default"; export interface ComposerTrigger { kind: ComposerTriggerKind; @@ -14,7 +14,6 @@ const SLASH_COMMANDS: readonly ComposerSlashCommand[] = [ "model", "plan", "default", - "checkout-pr", ]; function clampCursor(text: string, cursor: number): number { @@ -169,13 +168,12 @@ export function parseStandaloneComposerSlashCommand(text: string): Exclude< ComposerSlashCommand, "model" > | null { - const match = /^\/(plan|default|checkout-pr)\s*$/i.exec(text.trim()); + const match = /^\/(plan|default)\s*$/i.exec(text.trim()); if (!match) { return null; } const command = match[1]?.toLowerCase(); if (command === "plan") return "plan"; - if (command === "checkout-pr") return "checkout-pr"; return "default"; } -- 2.54.0