From cc742ca31e562aa413a79f53cb2eabd135c7ffdf Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Fri, 24 Jul 2026 20:36:34 +0800 Subject: [PATCH] fix(web-shell): keep git mode popover open when picking branch/worktree (#7668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking the "New branch" or "Worktree" option dismissed the popover instead of revealing the branch-name input / confirm button. The option click bubbles as a React synthetic event through the portal up to the composer surface onClick, which calls core.focus() and moves focus outside the popover — tripping the Radix focus-outside dismissal (the popover only guarded the pointer path via onInteractOutside). Stop click propagation on the popover content, matching the composer ToolbarPopover pattern in ChatEditor. Also fix the e2e selectors (getByText('New branch') matched both the name and description spans) and add a delayed still-open assertion so the flash-then-dismiss regression cannot false-pass. --- .../client/components/GitModePopover.tsx | 7 +++++ .../client/e2e/web-shell.git-mode.spec.ts | 28 +++++++++++++------ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/packages/web-shell/client/components/GitModePopover.tsx b/packages/web-shell/client/components/GitModePopover.tsx index 5cb5b660a9..1d7cc37572 100644 --- a/packages/web-shell/client/components/GitModePopover.tsx +++ b/packages/web-shell/client/components/GitModePopover.tsx @@ -160,6 +160,13 @@ export function GitModePopover({ align="end" sideOffset={8} className={styles.popover} + // The content is portaled out of the composer, but React synthetic + // clicks still bubble through the React tree to the composer + // surface's onClick, which calls core.focus() and steals focus out of + // the popover — Radix then dismisses it via focus-outside. Stop the + // bubble so option clicks keep focus inside (mirrors the composer + // ToolbarPopover pattern in ChatEditor). + onClick={(e) => e.stopPropagation()} onOpenAutoFocus={(e) => e.preventDefault()} onInteractOutside={(e) => { // The portal container fools Radix's dismissable-layer into diff --git a/packages/web-shell/client/e2e/web-shell.git-mode.spec.ts b/packages/web-shell/client/e2e/web-shell.git-mode.spec.ts index d7e89ba583..992967006a 100644 --- a/packages/web-shell/client/e2e/web-shell.git-mode.spec.ts +++ b/packages/web-shell/client/e2e/web-shell.git-mode.spec.ts @@ -83,13 +83,20 @@ test('git mode chip shows popover with three modes and captures screenshots', as animations: 'disabled', }); - // Click "New branch" option - const branchOption = popover.getByText('New branch', { exact: false }); - await branchOption.click(); + // Click "New branch" option (match by role: the option's text is split + // across a name + description span, so getByText('New branch') is ambiguous) + await popover.getByRole('radio', { name: /New branch/ }).click(); // Wait for the branch input to appear const branchInput = page.locator('[data-testid="git-mode-branch-input"]'); await expect(branchInput).toBeVisible({ timeout: 5_000 }); + // Regression guard: clicking an option used to steal focus to the composer + // (via the surface onClick bubbling through the portal), dismissing the + // popover ~100ms after the input flashed visible. Assert it stays open. + await expect(branchInput).toBeVisible(); + await page.waitForTimeout(300); + await expect(popover).toBeVisible(); + await expect(branchInput).toBeVisible(); // Type a branch name await branchInput.fill('feat/git-mode-selector'); @@ -145,13 +152,18 @@ test('git mode chip worktree mode sends worktree intent', async ({ const popover = page.locator('[data-slot="popover-content"]'); await expect(popover).toBeVisible({ timeout: 5_000 }); - // Click "Worktree" option - const worktreeOption = popover.getByText('Worktree', { exact: false }); - await worktreeOption.click(); + // Click "Worktree" option (match by role; see the branch test above) + await popover.getByRole('radio', { name: /Worktree/ }).click(); // Confirm worktree selection const confirmBtn = page.locator('[data-testid="git-mode-confirm-worktree"]'); await expect(confirmBtn).toBeVisible(); + // Regression guard: same focus-steal dismissal as the branch test — the + // confirm button flashed visible, then the popover closed before it could + // be clicked. Assert the popover survives the click. + await page.waitForTimeout(300); + await expect(popover).toBeVisible(); + await expect(confirmBtn).toBeVisible(); await confirmBtn.click(); await expect(popover).not.toBeVisible(); @@ -209,8 +221,8 @@ test('git mode chip clear button resets to current branch', async ({ const popover = page.locator('[data-slot="popover-content"]'); await expect(popover).toBeVisible({ timeout: 5_000 }); - // Select branch mode - await popover.getByText('New branch', { exact: false }).click(); + // Select branch mode (match by role; see the first branch test) + await popover.getByRole('radio', { name: /New branch/ }).click(); const branchInput = page.locator('[data-testid="git-mode-branch-input"]'); await branchInput.fill('feat/temp'); await page.locator('[data-testid="git-mode-confirm-branch"]').click();