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.
This commit is contained in:
Okan YILDIRIM
2026-10-24 07:25:30 +00:00
committed by GitHub
parent e01a38d943
commit b8bb335c56
2 changed files with 138 additions and 23 deletions
@@ -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,
}) => {
+32 -23
View File
@@ -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<typeof useIsolatedBottomSheetVisibility>;
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 <MobileComboboxSheet {...props} />;
}
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 (
<IsolatedBottomSheetModal
ref={props.bottomSheetRef}
ref={bottomSheetRef}
contextBridge={null}
snapPoints={props.snapPoints}
index={0}
enableDynamicSizing={false}
onChange={props.handleSheetChange}
onDismiss={props.handleSheetDismiss}
onChange={handleSheetChange}
onDismiss={handleSheetDismiss}
backdropOpacity={0.45}
enablePanDownToClose
backgroundComponent={ComboboxSheetBackground}
@@ -1168,7 +1180,8 @@ function DesktopComboboxOptionsBody(props: {
);
}
function DesktopComboboxBody(props: DesktopBodyProps): ReactElement {
function DesktopComboboxBody(props: DesktopBodyProps): ReactElement | null {
const active = useRetainedPanelActive();
const handleDesktopKey = props.handleDesktopKey;
const handleWebOverlayKeyDown = useCallback(
(event: KeyboardEvent) => {
@@ -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 = (
<OverlayLayerProvider layer={props.overlayLayer}>
<View
@@ -1309,6 +1325,7 @@ export function Combobox({
const resolvedEmptyText = emptyText ?? t("common.empty.noOptionsMatchSearch");
const resolvedTitle = title ?? t("common.actions.select");
const isMobile = useIsCompactFormFactor();
const active = useRetainedPanelActive();
const floatingLayer = useOverlayLayer("floating");
const safeAreaInsets = useSafeAreaInsets();
const titleColor = theme.colors.foreground;
@@ -1348,6 +1365,10 @@ export function Combobox({
bumpSearchResetKey();
}, [setOpen, setSearchQueryWithCallback]);
useEffect(() => {
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 (
<MobileComboboxBody
bottomSheetRef={bottomSheetRef}
isOpen={isOpen}
snapPoints={snapPoints}
handleSheetChange={handleSheetChange}
handleSheetDismiss={handleSheetDismiss}
handleIndicatorStyle={handleIndicatorStyle}
titleColor={titleColor}
title={resolvedTitle}