diff --git a/packages/app/src/components/adaptive-modal-sheet.tsx b/packages/app/src/components/adaptive-modal-sheet.tsx index 0618a1fd7..380441173 100644 --- a/packages/app/src/components/adaptive-modal-sheet.tsx +++ b/packages/app/src/components/adaptive-modal-sheet.tsx @@ -196,7 +196,7 @@ export function AdaptiveModalSheet({ () => ({ backgroundColor: theme.colors.surface2 }), [theme.colors.surface2], ); - const { sheetRef, handleSheetChange } = useIsolatedBottomSheetVisibility({ + const { sheetRef, handleSheetChange, handleSheetDismiss } = useIsolatedBottomSheetVisibility({ visible, isEnabled: isMobile, onClose, @@ -228,6 +228,7 @@ export function AdaptiveModalSheet({ index={0} enableDynamicSizing={false} onChange={handleSheetChange} + onDismiss={handleSheetDismiss} backdropComponent={renderBackdrop} enablePanDownToClose backgroundComponent={SheetBackground} diff --git a/packages/app/src/components/tool-call-sheet.tsx b/packages/app/src/components/tool-call-sheet.tsx index 131c7ffa6..4c616e8d9 100644 --- a/packages/app/src/components/tool-call-sheet.tsx +++ b/packages/app/src/components/tool-call-sheet.tsx @@ -1,4 +1,5 @@ -import React, { createContext, useContext, useCallback, useMemo, useRef, ReactNode } from "react"; +import React, { createContext, useContext, useCallback, useMemo } from "react"; +import type { ReactNode } from "react"; import { View, Text, Pressable } from "react-native"; import { StyleSheet, useUnistyles } from "react-native-unistyles"; import Animated from "react-native-reanimated"; @@ -11,7 +12,7 @@ import { X } from "lucide-react-native"; import type { ToolCallDetail } from "@server/server/agent/agent-sdk-types"; import { IsolatedBottomSheetModal, - type IsolatedBottomSheetModalRef, + useIsolatedBottomSheetVisibility, } from "@/components/ui/isolated-bottom-sheet-modal"; import { resolveToolCallIcon } from "@/utils/tool-call-icon"; import { ToolCallDetailsContent } from "./tool-call-details"; @@ -63,32 +64,33 @@ interface ToolCallSheetProviderProps { export function ToolCallSheetProvider({ children }: ToolCallSheetProviderProps) { const { theme } = useUnistyles(); - const bottomSheetRef = useRef(null); - const hasPresentedRef = useRef(false); const [sheetData, setSheetData] = React.useState(null); + const [isSheetOpen, setIsSheetOpen] = React.useState(false); const snapPoints = useMemo(() => ["60%", "95%"], []); const openToolCall = useCallback((data: ToolCallSheetData) => { setSheetData(data); - if (hasPresentedRef.current) { - requestAnimationFrame(() => bottomSheetRef.current?.snapToIndex(0)); - return; - } - - hasPresentedRef.current = true; - bottomSheetRef.current?.present(); + setIsSheetOpen(true); }, []); const closeToolCall = useCallback(() => { - bottomSheetRef.current?.close(); + setIsSheetOpen(false); }, []); - const handleSheetChange = useCallback((index: number) => { - if (index === -1) { - setSheetData(null); - } - }, []); + const { + sheetRef: bottomSheetRef, + handleSheetChange, + handleSheetDismiss, + } = useIsolatedBottomSheetVisibility({ + visible: isSheetOpen, + onClose: closeToolCall, + }); + + const handleToolCallSheetDismiss = useCallback(() => { + handleSheetDismiss(); + setSheetData(null); + }, [handleSheetDismiss]); const renderBackdrop = useCallback( (props: React.ComponentProps) => ( @@ -116,6 +118,7 @@ export function ToolCallSheetProvider({ children }: ToolCallSheetProviderProps) index={0} enableDynamicSizing={false} onChange={handleSheetChange} + onDismiss={handleToolCallSheetDismiss} backdropComponent={renderBackdrop} enablePanDownToClose backgroundComponent={CustomSheetBackground} diff --git a/packages/app/src/components/ui/combobox.tsx b/packages/app/src/components/ui/combobox.tsx index 556b88f00..610e106c1 100644 --- a/packages/app/src/components/ui/combobox.tsx +++ b/packages/app/src/components/ui/combobox.tsx @@ -910,10 +910,13 @@ function resolveInitialActiveIndex( return selectedIndex >= 0 ? selectedIndex : fallbackIndex; } +type BottomSheetVisibility = ReturnType; + interface MobileBodyProps { - bottomSheetRef: ReturnType["sheetRef"]; + bottomSheetRef: BottomSheetVisibility["sheetRef"]; snapPoints: string[]; - handleSheetChange: ReturnType["handleSheetChange"]; + handleSheetChange: BottomSheetVisibility["handleSheetChange"]; + handleSheetDismiss: BottomSheetVisibility["handleSheetDismiss"]; handleIndicatorStyle: { backgroundColor: string }; titleColor: string; title: string; @@ -971,6 +974,7 @@ function MobileComboboxBody(props: MobileBodyProps): ReactElement { index={0} enableDynamicSizing={false} onChange={props.handleSheetChange} + onDismiss={props.handleSheetDismiss} backdropComponent={renderBackdrop} enablePanDownToClose backgroundComponent={ComboboxSheetBackground} @@ -1298,7 +1302,11 @@ export function Combobox({ referenceWidth, }); - const { sheetRef: bottomSheetRef, handleSheetChange } = useIsolatedBottomSheetVisibility({ + const { + sheetRef: bottomSheetRef, + handleSheetChange, + handleSheetDismiss, + } = useIsolatedBottomSheetVisibility({ visible: isOpen, isEnabled: isMobile, onClose: handleClose, @@ -1454,6 +1462,7 @@ export function Combobox({ bottomSheetRef={bottomSheetRef} snapPoints={snapPoints} handleSheetChange={handleSheetChange} + handleSheetDismiss={handleSheetDismiss} handleIndicatorStyle={handleIndicatorStyle} titleColor={titleColor} title={title} diff --git a/packages/app/src/components/ui/context-menu.tsx b/packages/app/src/components/ui/context-menu.tsx index 22b1670cc..a5f7fa4c8 100644 --- a/packages/app/src/components/ui/context-menu.tsx +++ b/packages/app/src/components/ui/context-menu.tsx @@ -400,7 +400,11 @@ export function ContextMenuContent({ setOpen(false); }, [setOpen]); - const { sheetRef: bottomSheetRef, handleSheetChange } = useIsolatedBottomSheetVisibility({ + const { + sheetRef: bottomSheetRef, + handleSheetChange, + handleSheetDismiss, + } = useIsolatedBottomSheetVisibility({ visible: open, isEnabled: useMobileSheet, onClose: handleClose, @@ -534,6 +538,7 @@ export function ContextMenuContent({ snapPoints={sheetSnapPoints} enableDynamicSizing={false} onChange={handleSheetChange} + onDismiss={handleSheetDismiss} backdropComponent={renderSheetBackdrop} enablePanDownToClose backgroundStyle={sheetBackgroundStyle} diff --git a/packages/app/src/components/ui/isolated-bottom-sheet-modal.test.tsx b/packages/app/src/components/ui/isolated-bottom-sheet-modal.test.tsx index efb52ae06..19a3bf034 100644 --- a/packages/app/src/components/ui/isolated-bottom-sheet-modal.test.tsx +++ b/packages/app/src/components/ui/isolated-bottom-sheet-modal.test.tsx @@ -14,7 +14,12 @@ const SNAP_POINTS_90: (string | number)[] = ["90%"]; function noop(): void {} -const { modalMethods, modalProps } = vi.hoisted(() => ({ +interface CapturedModalProps { + onChange?: (index: number) => void; + onDismiss?: () => void; +} + +const { modalMethods, modalProps, shouldExposeModalRef } = vi.hoisted(() => ({ modalMethods: { present: vi.fn(), close: vi.fn(), @@ -22,6 +27,7 @@ const { modalMethods, modalProps } = vi.hoisted(() => ({ dismiss: vi.fn(), }, modalProps: vi.fn(), + shouldExposeModalRef: { current: true }, })); vi.mock("@gorhom/bottom-sheet", async () => { @@ -29,7 +35,7 @@ vi.mock("@gorhom/bottom-sheet", async () => { const MockBottomSheetModal = React.forwardRef( (props: Record, ref: React.ForwardedRef) => { modalProps(props); - React.useImperativeHandle(ref, () => modalMethods); + React.useImperativeHandle(ref, () => (shouldExposeModalRef.current ? modalMethods : null)); return React.createElement( "div", { "data-testid": "bottom-sheet" }, @@ -54,7 +60,7 @@ vi.mock("@gorhom/portal", async () => { }); function Harness({ visible, onClose }: { visible: boolean; onClose: () => void }) { - const { sheetRef, handleSheetChange } = useIsolatedBottomSheetVisibility({ + const { sheetRef, handleSheetChange, handleSheetDismiss } = useIsolatedBottomSheetVisibility({ visible, onClose, }); @@ -65,26 +71,34 @@ function Harness({ visible, onClose }: { visible: boolean; onClose: () => void } index={0} snapPoints={SNAP_POINTS_50} onChange={handleSheetChange} + onDismiss={handleSheetDismiss} >
Sheet content
); } +function latestModalProps(): CapturedModalProps { + const props = modalProps.mock.lastCall?.[0]; + expect(props).toBeDefined(); + return props as CapturedModalProps; +} + describe("IsolatedBottomSheetModal", () => { afterEach(() => { cleanup(); vi.clearAllMocks(); + shouldExposeModalRef.current = true; }); - it("forces sheet isolation and keeps modal content mounted while hidden", () => { + it("forces sheet isolation and uses the modal dismissal lifecycle", () => { const onClose = vi.fn(); const { getByTestId, rerender } = render(); expect(getByTestId("app-portal").getAttribute("data-host")).toBe("root"); expect(modalProps).toHaveBeenLastCalledWith( expect.objectContaining({ - enableDismissOnClose: false, + enableDismissOnClose: true, stackBehavior: "replace", }), ); @@ -94,28 +108,61 @@ describe("IsolatedBottomSheetModal", () => { expect(modalMethods.present).toHaveBeenCalledTimes(1); rerender(); - expect(modalMethods.close).toHaveBeenCalledTimes(1); - expect(modalMethods.dismiss).not.toHaveBeenCalled(); + expect(modalMethods.dismiss).toHaveBeenCalledTimes(1); + expect(modalMethods.close).not.toHaveBeenCalled(); rerender(); - expect(modalMethods.present).toHaveBeenCalledTimes(1); - expect(modalMethods.snapToIndex).toHaveBeenCalledWith(0); + expect(modalMethods.present).toHaveBeenCalledTimes(2); + expect(modalMethods.snapToIndex).not.toHaveBeenCalled(); }); it("only reports a user close when the sheet was visible", () => { const onClose = vi.fn(); const { rerender } = render(); - const latestProps = modalProps.mock.lastCall?.[0] as { onChange: (index: number) => void }; - latestProps.onChange(-1); + latestModalProps().onChange?.(-1); expect(onClose).toHaveBeenCalledTimes(1); rerender(); - const closedProps = modalProps.mock.lastCall?.[0] as { onChange: (index: number) => void }; - closedProps.onChange(-1); + latestModalProps().onChange?.(-1); expect(onClose).toHaveBeenCalledTimes(1); }); + it("reports a dismiss while visible as a close request", () => { + const onClose = vi.fn(); + render(); + + latestModalProps().onDismiss?.(); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("deduplicates close notifications from change and dismiss callbacks", () => { + const onClose = vi.fn(); + render(); + + const props = latestModalProps(); + props.onChange?.(-1); + props.onDismiss?.(); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("presents when the sheet ref becomes available after opening", () => { + shouldExposeModalRef.current = false; + const onClose = vi.fn(); + const { rerender } = render(); + + expect(modalMethods.present).not.toHaveBeenCalled(); + expect(modalMethods.snapToIndex).not.toHaveBeenCalled(); + + shouldExposeModalRef.current = true; + rerender(); + + expect(modalMethods.present).toHaveBeenCalledTimes(1); + expect(modalMethods.snapToIndex).not.toHaveBeenCalled(); + }); + it("allows nested sheets inside a parent sheet without creating a sibling provider", () => { const { getAllByTestId } = render( diff --git a/packages/app/src/components/ui/isolated-bottom-sheet-modal.tsx b/packages/app/src/components/ui/isolated-bottom-sheet-modal.tsx index 5ff1757a5..6236974a0 100644 --- a/packages/app/src/components/ui/isolated-bottom-sheet-modal.tsx +++ b/packages/app/src/components/ui/isolated-bottom-sheet-modal.tsx @@ -41,7 +41,7 @@ export const IsolatedBottomSheetModal = forwardRef< {scopedChildren} @@ -68,39 +68,92 @@ export function useIsolatedBottomSheetVisibility({ isEnabled?: boolean; onClose: () => void; }) { - const sheetRef = useRef(null); - const hasPresentedRef = useRef(false); + const sheetRef = useRef(null); + const visibleRef = useRef(visible); + const isEnabledRef = useRef(isEnabled); + const isPresentedRef = useRef(false); + const hasNotifiedCloseRef = useRef(false); + + visibleRef.current = visible; + isEnabledRef.current = isEnabled; + + const presentSheet = useCallback((sheet: IsolatedBottomSheetModalRef) => { + if (isPresentedRef.current) { + return; + } + + isPresentedRef.current = true; + hasNotifiedCloseRef.current = false; + sheet.present(); + }, []); + + const dismissSheet = useCallback((sheet: IsolatedBottomSheetModalRef) => { + if (!isPresentedRef.current) { + return; + } + + isPresentedRef.current = false; + sheet.dismiss(); + }, []); + + const notifyClose = useCallback(() => { + if (hasNotifiedCloseRef.current) { + return; + } + + hasNotifiedCloseRef.current = true; + onClose(); + }, [onClose]); + + const handleSheetDismiss = useCallback(() => { + isPresentedRef.current = false; + if (visibleRef.current) { + notifyClose(); + return; + } + hasNotifiedCloseRef.current = false; + }, [notifyClose]); + + const handleSheetChange = useCallback( + (index: number) => { + if (index === -1 && visibleRef.current) { + notifyClose(); + } + }, + [notifyClose], + ); + + const setSheetRef = useCallback( + (instance: IsolatedBottomSheetModalRef | null) => { + sheetRef.current = instance; + if (instance && visibleRef.current && isEnabledRef.current !== false) { + presentSheet(instance); + } + }, + [presentSheet], + ); useEffect(() => { if (isEnabled === false) return; + const sheet = sheetRef.current; if (visible) { - if (hasPresentedRef.current) { - sheetRef.current?.snapToIndex(0); + if (!sheet) { return; } - hasPresentedRef.current = true; - sheetRef.current?.present(); + presentSheet(sheet); return; } - if (hasPresentedRef.current) { - sheetRef.current?.close(); + if (sheet) { + dismissSheet(sheet); } - }, [isEnabled, visible]); - - const handleSheetChange = useCallback( - (index: number) => { - if (index === -1 && visible) { - onClose(); - } - }, - [onClose, visible], - ); + }, [dismissSheet, isEnabled, presentSheet, visible]); return { - sheetRef, + sheetRef: setSheetRef, handleSheetChange, + handleSheetDismiss, }; }