Fix stacked provider settings sheets

This commit is contained in:
Mohamed Boudra
2026-06-05 21:15:44 +07:00
parent abf129f56e
commit dd329a4f52
13 changed files with 242 additions and 62 deletions
+5
View File
@@ -169,6 +169,11 @@ on that attach races Gorhom's dismiss path and leaves the modal unable to reopen
Track an explicit phase (`closed` / `presenting` / `presented` / `dismissing`) and
ignore ref churn while dismissing.
Do not treat `onChange(-1)` as a close by itself. In a stacked
`BottomSheetModal`, `-1` can also mean the sheet is temporarily hidden under
another pushed sheet. Close React state from `onDismiss`; use `onChange` only to
track phase.
## Recipe for a new anchored panel
Before you write a new one, ask:
@@ -0,0 +1,109 @@
import { expect, test, type Page } from "./fixtures";
import { expectComposerVisible } from "./helpers/composer";
import { openAgentRoute, seedMockAgentWorkspace } from "./helpers/mock-agent";
const MOBILE_VIEWPORT = { width: 390, height: 844 };
async function openMockAgentAtMobileBreakpoint(page: Page) {
await page.setViewportSize(MOBILE_VIEWPORT);
const session = await seedMockAgentWorkspace({
repoPrefix: "provider-sheet-stack-",
title: "Provider sheet stack e2e",
initialPrompt: "Prepare provider sheet stack test agent.",
});
await openAgentRoute(page, session);
await expectComposerVisible(page);
await expect(page.getByRole("button", { name: /Select model/ })).toBeVisible({
timeout: 30_000,
});
return session;
}
async function openProviderSettingsFromModelSelector(page: Page) {
await page.getByRole("button", { name: /Select model/ }).click();
await expect(page.getByLabel("Bottom Sheet", { exact: true })).toBeVisible({ timeout: 10_000 });
const openCodeRow = page.getByText("OpenCode", { exact: true }).first();
if (await openCodeRow.isVisible().catch(() => false)) {
await openCodeRow.click();
}
await page.getByRole("button", { name: /Open .* settings/ }).click();
await expect(page.getByTestId("provider-settings-sheet")).toBeVisible({ timeout: 10_000 });
}
async function expectModelSelectorVisible(page: Page) {
await expect(page.getByRole("button", { name: /Open .* settings/ })).toBeVisible({
timeout: 10_000,
});
}
async function closeTopSheet(page: Page) {
const closeTarget = page.getByLabel("Close", { exact: true }).last();
if (await closeTarget.isVisible().catch(() => false)) {
await closeTarget.click({ force: true });
return;
}
const handle = page.getByRole("slider", { name: "Bottom sheet handle" }).last();
const handleBox = await handle.boundingBox();
if (!handleBox) {
throw new Error("Bottom sheet handle was not measurable");
}
const startX = handleBox.x + handleBox.width / 2;
const startY = handleBox.y + handleBox.height / 2;
await page.mouse.move(startX, startY);
await page.mouse.down();
await page.mouse.move(startX, startY + 400, { steps: 8 });
await page.mouse.up();
}
async function expectProviderSettingsVisible(page: Page) {
await expect(page.getByTestId("provider-settings-sheet")).toBeVisible({ timeout: 10_000 });
await expect(page.getByRole("button", { name: "Add model" })).toBeVisible();
await expect(page.getByRole("button", { name: "Diagnostic", exact: true })).toBeVisible();
}
async function exerciseProviderSettingsStack(page: Page) {
await expectProviderSettingsVisible(page);
await page.getByRole("button", { name: "Add model" }).click();
await expect(page.getByTestId("add-custom-model-sheet")).toBeVisible({ timeout: 10_000 });
await closeTopSheet(page);
await expectProviderSettingsVisible(page);
await page.getByRole("button", { name: "Diagnostic", exact: true }).click();
await expect(page.getByTestId("provider-diagnostic-sheet")).toBeVisible({ timeout: 10_000 });
await page.getByRole("button", { name: /Refresh diagnostic/ }).click();
await expect(page.getByTestId("provider-diagnostic-sheet")).toBeVisible({ timeout: 10_000 });
await closeTopSheet(page);
await expectProviderSettingsVisible(page);
await page.getByRole("button", { name: "Refresh", exact: true }).click();
await expectProviderSettingsVisible(page);
}
test.describe("provider settings bottom-sheet stack", () => {
test("provider settings and children close back through the model selector stack", async ({
page,
}) => {
const session = await openMockAgentAtMobileBreakpoint(page);
try {
await openProviderSettingsFromModelSelector(page);
await exerciseProviderSettingsStack(page);
await closeTopSheet(page);
await expectModelSelectorVisible(page);
await page.getByRole("button", { name: /Open .* settings/ }).click();
await expect(page.getByTestId("provider-settings-sheet")).toBeVisible({ timeout: 10_000 });
await exerciseProviderSettingsStack(page);
await closeTopSheet(page);
await expectModelSelectorVisible(page);
await closeTopSheet(page);
} finally {
await session.cleanup();
}
});
});
+7 -3
View File
@@ -1,4 +1,5 @@
import "@/styles/unistyles";
import { BottomSheetModalProvider } from "@gorhom/bottom-sheet";
import { PortalProvider } from "@gorhom/portal";
import { QueryClientProvider } from "@tanstack/react-query";
import * as Linking from "expo-linking";
@@ -931,17 +932,20 @@ function RuntimeProviders({ children }: { children: ReactNode }) {
);
}
// PortalProvider must remain the innermost global provider here.
// PortalProvider must stay inside normal app-wide context providers here.
// `@gorhom/portal` renders portaled children at the host's location in the
// tree, so any context a portaled sheet might consume (QueryClient, theme,
// auth, settings, …) must wrap PortalProvider — not be wrapped by it.
// Adding a new global provider? Put it above PortalProvider.
// BottomSheetModalProvider is the exception: Gorhom modals consume portal
// context and need one shared provider for sibling sheets to stack.
function RootProviders({ children }: { children: ReactNode }) {
return (
<QueryProvider>
<SafeAreaProvider>
<KeyboardProvider>
<PortalProvider>{children}</PortalProvider>
<PortalProvider>
<BottomSheetModalProvider>{children}</BottomSheetModalProvider>
</PortalProvider>
</KeyboardProvider>
</SafeAreaProvider>
</QueryProvider>
@@ -12,6 +12,7 @@ import {
BottomSheetTextInput,
type BottomSheetBackgroundProps,
} from "@gorhom/bottom-sheet";
import Animated from "react-native-reanimated";
import { ArrowLeft, Search, X } from "lucide-react-native";
import { FileDropZone } from "@/components/file-drop-zone";
import type { ImageAttachment } from "@/composer/types";
@@ -232,14 +233,14 @@ function SheetBackground({ style }: BottomSheetBackgroundProps) {
() => [
style,
{
backgroundColor: theme.colors.surface1,
borderTopLeftRadius: theme.borderRadius.xl,
borderTopRightRadius: theme.borderRadius.xl,
backgroundColor: theme.colors.surface0,
borderTopLeftRadius: theme.borderRadius["2xl"],
borderTopRightRadius: theme.borderRadius["2xl"],
},
],
[style, theme.colors.surface1, theme.borderRadius.xl],
[style, theme.colors.surface0, theme.borderRadius],
);
return <View style={combinedStyle} />;
return <Animated.View pointerEvents="none" style={combinedStyle} />;
}
export type AdaptiveTextInputProps = TextInputProps & {
@@ -444,6 +445,7 @@ export interface AdaptiveModalSheetProps {
/** When provided, wraps the card content in a FileDropZone. */
onFilesDropped?: (files: ImageAttachment[]) => void;
scrollable?: boolean;
presentation?: "push" | "replace";
}
export function AdaptiveModalSheet({
@@ -457,6 +459,7 @@ export function AdaptiveModalSheet({
desktopMaxWidth,
onFilesDropped,
scrollable = true,
presentation,
}: AdaptiveModalSheetProps) {
const { theme } = useUnistyles();
const isMobile = useIsCompactFormFactor();
@@ -501,8 +504,8 @@ export function AdaptiveModalSheet({
[compactSafeAreaPadding.footerPaddingBottom],
);
const handleIndicatorStyle = useMemo(
() => ({ backgroundColor: theme.colors.surface2 }),
[theme.colors.surface2],
() => ({ backgroundColor: theme.colors.palette.zinc[600] }),
[theme.colors.palette.zinc],
);
const { sheetRef, handleSheetChange, handleSheetDismiss } = useIsolatedBottomSheetVisibility({
visible,
@@ -543,6 +546,7 @@ export function AdaptiveModalSheet({
keyboardBehavior="extend"
keyboardBlurBehavior="restore"
accessible={false}
presentation={presentation}
>
<SheetHeaderView header={header} onClose={onClose} testID={testID} />
{scrollable ? (
@@ -685,10 +685,6 @@ export function CombinedModelSelector({
handleOpenChange(!isOpen);
}, [handleOpenChange, isOpen]);
const handleClose = useCallback(() => {
handleOpenChange(false);
}, [handleOpenChange]);
const triggerStyle = useCallback(
({ pressed, hovered }: PressableStateCallbackType & { hovered?: boolean }) => [
styles.trigger,
@@ -714,11 +710,10 @@ export function CombinedModelSelector({
setSearchQuery(value);
}, []);
const openHeaderProviderSettings = useCallback(() => {
const openProviderSettings = useCallback(() => {
if (!serverId || view.kind !== "provider") return;
useProviderSettingsStore.getState().open({ serverId, provider: view.providerId });
handleClose();
}, [serverId, view, handleClose]);
}, [serverId, view]);
const sheetHeader = useMemo<SheetHeader>(() => {
if (view.kind === "all") {
@@ -727,7 +722,7 @@ export function CombinedModelSelector({
const ProviderIconForView = getProviderIcon(view.providerId);
const headerActions = (
<Pressable
onPress={openHeaderProviderSettings}
onPress={openProviderSettings}
disabled={!serverId}
hitSlop={8}
style={iconButtonStyle}
@@ -760,7 +755,7 @@ export function CombinedModelSelector({
view,
singleProviderView,
serverId,
openHeaderProviderSettings,
openProviderSettings,
theme.colors.border,
theme.colors.foregroundMuted,
handleBackToAll,
@@ -196,6 +196,7 @@ function AddCustomModelSubSheet({
onClose={onClose}
desktopMaxWidth={420}
snapPoints={ADD_SNAP_POINTS}
testID="add-custom-model-sheet"
>
<View style={sheetStyles.formGroup}>
<Text style={sheetStyles.formLabel}>Model ID</Text>
@@ -339,6 +340,7 @@ function DiagnosticSubSheet({
onClose={onClose}
snapPoints={DIAGNOSTIC_SNAP_POINTS}
scrollable={false}
testID="provider-diagnostic-sheet"
>
<View style={DIAGNOSTIC_CARD_STYLE}>{body}</View>
</AdaptiveModalSheet>
@@ -616,6 +618,7 @@ export function ProviderDiagnosticSheet({
header={sheetHeader}
visible={visible}
onClose={onClose}
testID="provider-settings-sheet"
footer={renderProviderSheetFooter({
fetchedAtLabel,
isCompact,
@@ -5,6 +5,7 @@ import { useProviderSettingsStore } from "@/stores/provider-settings-store";
export function ProviderSettingsHost() {
const serverId = useProviderSettingsStore((state) => state.serverId);
const provider = useProviderSettingsStore((state) => state.provider);
const visible = useProviderSettingsStore((state) => state.visible);
const close = useProviderSettingsStore((state) => state.close);
const handleClose = useCallback(() => {
@@ -19,7 +20,7 @@ export function ProviderSettingsHost() {
<ProviderDiagnosticSheet
provider={provider}
serverId={serverId}
visible
visible={visible}
onClose={handleClose}
/>
);
@@ -95,6 +95,7 @@ export interface ComboboxProps {
*/
header?: SheetHeader;
mobileChildrenScrollEnabled?: boolean;
presentation?: "push" | "replace";
open?: boolean;
onOpenChange?: (open: boolean) => void;
desktopPlacement?: "top-start" | "bottom-start";
@@ -965,6 +966,7 @@ interface MobileBodyProps {
searchable: boolean;
hasChildren: boolean;
mobileChildrenScrollEnabled: boolean;
presentation?: "push" | "replace";
searchResetKey: number;
searchPlaceholder: string;
searchQuery: string;
@@ -1024,6 +1026,7 @@ function MobileComboboxBody(props: MobileBodyProps): ReactElement {
handleIndicatorStyle={props.handleIndicatorStyle}
keyboardBehavior="extend"
keyboardBlurBehavior="none"
presentation={props.presentation}
>
{props.header ? (
<SheetHeaderView header={props.header} onClose={props.onClose} />
@@ -1248,6 +1251,7 @@ export function Combobox({
title = "Select",
header,
mobileChildrenScrollEnabled = true,
presentation,
open,
onOpenChange,
desktopPlacement = "top-start",
@@ -1534,6 +1538,7 @@ export function Combobox({
searchable={searchable}
hasChildren={hasChildren}
mobileChildrenScrollEnabled={mobileChildrenScrollEnabled}
presentation={presentation}
searchResetKey={searchResetKey}
searchPlaceholder={effectiveSearchPlaceholder}
searchQuery={searchQuery}
@@ -0,0 +1,76 @@
/**
* @vitest-environment jsdom
*/
import React, { act } from "react";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
vi.mock("@gorhom/portal", () => ({
Portal: ({ children, hostName }: { children?: React.ReactNode; hostName?: string }) =>
React.createElement("div", { "data-portal-host": hostName }, children),
PortalHost: ({ name }: { name?: string }) => React.createElement("div", { "data-host": name }),
}));
vi.mock("@gorhom/bottom-sheet", () => ({
BottomSheetModalProvider: ({ children }: { children?: React.ReactNode }) =>
React.createElement("div", { "data-bottom-sheet-provider": true }, children),
BottomSheetModal: React.forwardRef(
(
{
children,
stackBehavior,
}: {
children?: React.ReactNode;
stackBehavior?: string;
},
_ref,
) => React.createElement("div", { "data-stack-behavior": stackBehavior }, children),
),
}));
import { IsolatedBottomSheetModal } from ".";
describe("IsolatedBottomSheetModal presentation", () => {
let container: HTMLDivElement;
let root: Root;
beforeEach(() => {
container = document.createElement("div");
document.body.appendChild(container);
root = createRoot(container);
});
afterEach(() => {
act(() => root.unmount());
container.remove();
});
it("defaults sibling top-level sheets to push instead of replacing by React ancestry", () => {
act(() => {
root.render(
<>
<IsolatedBottomSheetModal>Settings</IsolatedBottomSheetModal>
<IsolatedBottomSheetModal>Diagnostic</IsolatedBottomSheetModal>
</>,
);
});
expect(
Array.from(container.querySelectorAll("[data-stack-behavior]")).map((node) =>
node.getAttribute("data-stack-behavior"),
),
).toEqual(["push", "push"]);
});
it("only replaces when the callsite asks for replacement", () => {
act(() => {
root.render(
<IsolatedBottomSheetModal presentation="replace">Selector</IsolatedBottomSheetModal>,
);
});
expect(
container.querySelector("[data-stack-behavior]")?.getAttribute("data-stack-behavior"),
).toBe("replace");
});
});
@@ -1,10 +1,8 @@
import {
BottomSheetModal as GorhomBottomSheetModal,
BottomSheetModalProvider,
type BottomSheetModalProps,
} from "@gorhom/bottom-sheet";
import { Portal } from "@gorhom/portal";
import React, { createContext, useContext } from "react";
import React from "react";
import { forwardRef, useCallback, useEffect, useMemo, useRef } from "react";
import type { ElementRef } from "react";
import {
@@ -17,50 +15,29 @@ type GorhomBottomSheetModalMethods = ElementRef<typeof GorhomBottomSheetModal>;
type IsolatedBottomSheetModalProps = Omit<
BottomSheetModalProps,
"enableDismissOnClose" | "stackBehavior"
>;
> & {
presentation?: "push" | "replace";
};
export type IsolatedBottomSheetModalRef = GorhomBottomSheetModalMethods;
const IsolatedBottomSheetScopeContext = createContext(false);
export const IsolatedBottomSheetModal = forwardRef<
IsolatedBottomSheetModalRef,
IsolatedBottomSheetModalProps
>(function IsolatedBottomSheetModal(props, ref) {
const isNestedSheet = useContext(IsolatedBottomSheetScopeContext);
const { children, ...bottomSheetProps } = props;
const scopedChildren =
typeof children === "function" ? (
(input: { data?: unknown }) => (
<IsolatedBottomSheetScopeContext.Provider value={true}>
{children(input) as React.ReactNode}
</IsolatedBottomSheetScopeContext.Provider>
)
) : (
<IsolatedBottomSheetScopeContext.Provider value={true}>
{children}
</IsolatedBottomSheetScopeContext.Provider>
);
const { children, presentation = "push", ...bottomSheetProps } = props;
const modal = (
<GorhomBottomSheetModal
{...bottomSheetProps}
ref={ref}
enableDismissOnClose
stackBehavior={isNestedSheet ? "push" : "replace"}
stackBehavior={presentation}
>
{scopedChildren}
{children}
</GorhomBottomSheetModal>
);
if (isNestedSheet) {
return modal;
}
return (
<Portal hostName="root">
<BottomSheetModalProvider>{modal}</BottomSheetModalProvider>
</Portal>
);
return modal;
});
export function useIsolatedBottomSheetVisibility({
@@ -70,17 +70,17 @@ describe("bottom sheet visibility tracker", () => {
expect(sheet.events).toEqual([]);
});
it("only reports a user close when the sheet was visible", () => {
it("does not treat index -1 as a close because stacked sheets can be hidden without dismissing", () => {
const { sheet, tracker, closeCount } = setup();
tracker.attachController(sheet);
tracker.syncDesired({ visible: true });
tracker.handleSheetIndexChange(-1);
expect(closeCount()).toBe(1);
expect(closeCount()).toBe(0);
tracker.syncDesired({ visible: false });
tracker.handleSheetIndexChange(-1);
expect(closeCount()).toBe(1);
expect(closeCount()).toBe(0);
});
it("reports a dismiss while visible as a close request", () => {
@@ -93,7 +93,7 @@ describe("bottom sheet visibility tracker", () => {
expect(closeCount()).toBe(1);
});
it("deduplicates close notifications from change and dismiss callbacks", () => {
it("reports close once when a hidden sheet is actually dismissed", () => {
const { sheet, tracker, closeCount } = setup();
tracker.attachController(sheet);
tracker.syncDesired({ visible: true });
@@ -110,12 +110,14 @@ describe("bottom sheet visibility tracker", () => {
tracker.syncDesired({ visible: true });
tracker.handleSheetIndexChange(-1);
tracker.handleSheetDismiss();
expect(closeCount()).toBe(1);
tracker.syncDesired({ visible: false });
tracker.syncDesired({ visible: true });
tracker.handleSheetIndexChange(-1);
tracker.handleSheetDismiss();
expect(closeCount()).toBe(2);
});
@@ -128,7 +130,7 @@ describe("bottom sheet visibility tracker", () => {
tracker.attachController(null);
tracker.attachController(sheet);
expect(closeCount()).toBe(1);
expect(closeCount()).toBe(0);
expect(sheet.events).toEqual([{ type: "present" }]);
});
@@ -69,7 +69,7 @@ export function createBottomSheetVisibilityTracker(opts: {
},
handleSheetIndexChange(index) {
if (index !== -1) {
if (phase === "presenting") {
if (phase === "presenting" || phase === "dismissing") {
phase = "presented";
}
return;
@@ -77,9 +77,6 @@ export function createBottomSheetVisibilityTracker(opts: {
if (phase === "presenting" || phase === "presented") {
phase = "dismissing";
}
if (visible) {
notifyClose();
}
},
handleSheetDismiss() {
if (visible) {
@@ -8,6 +8,7 @@ interface ProviderSettingsTarget {
interface ProviderSettingsStoreState {
serverId: string | null;
provider: string | null;
visible: boolean;
open: (target: ProviderSettingsTarget) => void;
close: () => void;
}
@@ -15,10 +16,11 @@ interface ProviderSettingsStoreState {
export const useProviderSettingsStore = create<ProviderSettingsStoreState>()((set) => ({
serverId: null,
provider: null,
visible: false,
open: ({ serverId, provider }) => {
set({ serverId, provider });
set({ serverId, provider, visible: true });
},
close: () => {
set({ serverId: null, provider: null });
set({ visible: false });
},
}));