From b8bb335c563279be45544b1b20a1fa5b3e56d422 Mon Sep 17 00:00:00 2001 From: Okan YILDIRIM <63511519+ragokan@users.noreply.github.com> Date: Sat, 24 Oct 2026 10:25:30 +0300 Subject: [PATCH] fix(app): close comboboxes when their retained panel becomes inactive (#7783) Close open comboboxes through their existing handler and remove their presentation before a retained chat can freeze its closing update. The compact regression came from unmounting the sheet while its visibility tracker survived in Combobox. Its non-closed phase blocked the next present(). Keep the tracker inside the sheet component so they unmount together, and preserve the desktop closed-state guard in Combobox. Cover draft and existing-chat model selectors and the compact tab switcher through Settings, Back, and reopening. The desktop regressions also verify Escape dismissal. --- .../e2e/browser/settings-navigation.spec.ts | 106 ++++++++++++++++++ packages/app/src/components/ui/combobox.tsx | 55 +++++---- 2 files changed, 138 insertions(+), 23 deletions(-) diff --git a/packages/app/e2e/browser/settings-navigation.spec.ts b/packages/app/e2e/browser/settings-navigation.spec.ts index bc8e1eedec..165bb168cd 100644 --- a/packages/app/e2e/browser/settings-navigation.spec.ts +++ b/packages/app/e2e/browser/settings-navigation.spec.ts @@ -43,6 +43,8 @@ import { } from "../support/helpers/settings"; import { getServerId } from "../support/helpers/server-id"; import { expectAppRoute } from "../support/helpers/route-assertions"; +import { clickNewChat, gotoWorkspace } from "../support/helpers/launcher"; +import { openAgentRoute, seedMockAgentWorkspace } from "../support/helpers/mock-agent"; async function openWorkspace( page: import("@playwright/test").Page, @@ -53,6 +55,74 @@ async function openWorkspace( } test.describe("Settings sidebar navigation", () => { + test("opening Settings closes the composer model selector until it is opened again", async ({ + page, + withWorkspace, + }, testInfo) => { + const workspace = await withWorkspace({ prefix: "settings-model-selector-" }); + await gotoWorkspace(page, workspace.workspaceId); + await clickNewChat(page); + + const selector = page.getByTestId("combined-model-selector").filter({ visible: true }); + const popover = page.getByTestId("combobox-desktop-container"); + await selector.click(); + await expect(popover).toBeVisible(); + + const modifier = process.platform === "darwin" ? "Meta" : "Control"; + await page.keyboard.press(`${modifier}+,`); + await expect(page).toHaveURL(/\/settings\/general$/); + await page.screenshot({ path: testInfo.outputPath("settings-after-model-selector.png") }); + await expect(popover).toHaveCount(0); + + await clickSettingsBackToWorkspace(page); + await expect(selector).toBeVisible(); + await expect(popover).toHaveCount(0); + + await selector.click(); + await expect(popover).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(popover).toHaveCount(0); + }); + + test("opening Settings closes an existing chat's model selector until it is opened again", async ({ + page, + }, testInfo) => { + const workspace = await seedMockAgentWorkspace({ + repoPrefix: "settings-existing-model-", + title: "Settings model selector", + }); + try { + await openAgentRoute(page, { + workspaceId: workspace.workspaceId, + agentId: workspace.agentId, + }); + + const selector = page.getByTestId("combined-model-selector").filter({ visible: true }); + const popover = page.getByTestId("combobox-desktop-container"); + await selector.click(); + await expect(popover).toBeVisible(); + + const modifier = process.platform === "darwin" ? "Meta" : "Control"; + await page.keyboard.press(`${modifier}+,`); + await expect(page).toHaveURL(/\/settings\/general$/); + await page.screenshot({ + path: testInfo.outputPath("settings-after-existing-model-selector.png"), + }); + await expect(popover).toHaveCount(0); + + await clickSettingsBackToWorkspace(page); + await expect(selector).toBeVisible(); + await expect(popover).toHaveCount(0); + + await selector.click(); + await expect(popover).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(popover).toHaveCount(0); + } finally { + await workspace.cleanup(); + } + }); + test("clicking a sidebar section updates the URL and renders the section", async ({ page }) => { await gotoAppShell(page); await openSettings(page); @@ -147,6 +217,42 @@ test.describe("Settings sidebar navigation", () => { test.describe("Settings — compact master-detail", () => { test.use({ viewport: { width: 390, height: 844 } }); + test("the compact tab switcher reopens after Settings hides its workspace", async ({ page }) => { + const workspace = await seedMockAgentWorkspace({ + repoPrefix: "settings-tab-switcher-", + title: "Compact tab switcher", + }); + try { + await openAgentRoute(page, { + workspaceId: workspace.workspaceId, + agentId: workspace.agentId, + }); + + const switcher = page.getByTestId("workspace-tab-switcher-trigger").filter({ visible: true }); + const newAgent = page.getByTestId("workspace-tab-switcher-new-agent"); + await switcher.click(); + await expect(newAgent).toBeInViewport(); + + // Navigate without dismissing the sheet through its backdrop first. + await page + .getByTestId("menu-button") + .filter({ visible: true }) + .first() + .dispatchEvent("click"); + await page.getByTestId("sidebar-settings").first().dispatchEvent("click"); + await expectAppRoute(page, buildSettingsRoute()); + await expect(newAgent).toHaveCount(0); + + await closeCompactSettings(page); + await expect(switcher).toBeVisible(); + await expect(newAgent).toHaveCount(0); + await switcher.click(); + await expect(newAgent).toBeInViewport(); + } finally { + await workspace.cleanup(); + } + }); + test("opens compact app and host details and returns through the settings list", async ({ page, }) => { diff --git a/packages/app/src/components/ui/combobox.tsx b/packages/app/src/components/ui/combobox.tsx index 7401f7aac3..84d76c1083 100644 --- a/packages/app/src/components/ui/combobox.tsx +++ b/packages/app/src/components/ui/combobox.tsx @@ -28,6 +28,7 @@ import { useSafeAreaInsets } from "react-native-safe-area-context"; import type { EditingTextInputHandle } from "@/components/ui/text-input"; import { StyleSheet, useUnistyles } from "react-native-unistyles"; import { useIsCompactFormFactor } from "@/constants/layout"; +import { useRetainedPanelActive } from "@/components/retained-panel"; import { BottomSheetScrollView, type BottomSheetBackgroundProps } from "@gorhom/bottom-sheet"; import Animated, { FadeIn, FadeOut } from "react-native-reanimated"; import { GestureHandlerRootView } from "react-native-gesture-handler"; @@ -910,13 +911,9 @@ function resolveInitialActiveIndex( return selectedIndex >= 0 ? selectedIndex : fallbackIndex; } -type BottomSheetVisibility = ReturnType; - interface MobileBodyProps { - bottomSheetRef: BottomSheetVisibility["sheetRef"]; + isOpen: boolean; snapPoints: string[]; - handleSheetChange: BottomSheetVisibility["handleSheetChange"]; - handleSheetDismiss: BottomSheetVisibility["handleSheetDismiss"]; handleIndicatorStyle: { backgroundColor: string }; titleColor: string; title: string; @@ -944,7 +941,22 @@ interface MobileBodyProps { safeAreaBottom: number; } -function MobileComboboxBody(props: MobileBodyProps): ReactElement { +function MobileComboboxBody(props: MobileBodyProps): ReactElement | null { + const active = useRetainedPanelActive(); + if (!active) return null; + // The visibility tracker must leave the tree with its sheet. + return ; +} + +function MobileComboboxSheet(props: MobileBodyProps): ReactElement { + const { + sheetRef: bottomSheetRef, + handleSheetChange, + handleSheetDismiss, + } = useIsolatedBottomSheetVisibility({ + visible: props.isOpen, + onClose: props.onClose, + }); const comboboxTitleStyle = useMemo( () => [styles.comboboxTitle, { color: props.titleColor }], [props.titleColor], @@ -979,13 +991,13 @@ function MobileComboboxBody(props: MobileBodyProps): ReactElement { return ( { @@ -1190,6 +1203,9 @@ function DesktopComboboxBody(props: DesktopBodyProps): ReactElement { [props.refs, setWebOverlayScope], ); + // Remove the portal before a retained chat can freeze its closing update. + if (!active) return null; + const overlay = ( { + if (!active && isOpen) handleClose(); + }, [active, isOpen, handleClose]); + useResetSearchOnOpen(isOpen, setSearchQueryWithCallback, bumpSearchResetKey); const collisionPadding = useMemo(computeCollisionPadding, []); @@ -1413,16 +1434,6 @@ export function Combobox({ referenceWidth, }); - const { - sheetRef: bottomSheetRef, - handleSheetChange, - handleSheetDismiss, - } = useIsolatedBottomSheetVisibility({ - visible: isOpen, - isEnabled: isMobile, - onClose: handleClose, - }); - const normalizedSearch = searchable ? searchQuery.trim().toLowerCase() : ""; const sanitizedSearchValue = searchQuery.trim(); const showCustomOption = useMemo( @@ -1572,10 +1583,8 @@ export function Combobox({ if (isMobile) { return (