Fix isolated bottom sheet modal lifecycle
This commit is contained in:
@@ -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}
|
||||
|
||||
@@ -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<IsolatedBottomSheetModalRef>(null);
|
||||
const hasPresentedRef = useRef(false);
|
||||
const [sheetData, setSheetData] = React.useState<ToolCallSheetData | null>(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<typeof BottomSheetBackdrop>) => (
|
||||
@@ -116,6 +118,7 @@ export function ToolCallSheetProvider({ children }: ToolCallSheetProviderProps)
|
||||
index={0}
|
||||
enableDynamicSizing={false}
|
||||
onChange={handleSheetChange}
|
||||
onDismiss={handleToolCallSheetDismiss}
|
||||
backdropComponent={renderBackdrop}
|
||||
enablePanDownToClose
|
||||
backgroundComponent={CustomSheetBackground}
|
||||
|
||||
@@ -910,10 +910,13 @@ function resolveInitialActiveIndex(
|
||||
return selectedIndex >= 0 ? selectedIndex : fallbackIndex;
|
||||
}
|
||||
|
||||
type BottomSheetVisibility = ReturnType<typeof useIsolatedBottomSheetVisibility>;
|
||||
|
||||
interface MobileBodyProps {
|
||||
bottomSheetRef: ReturnType<typeof useIsolatedBottomSheetVisibility>["sheetRef"];
|
||||
bottomSheetRef: BottomSheetVisibility["sheetRef"];
|
||||
snapPoints: string[];
|
||||
handleSheetChange: ReturnType<typeof useIsolatedBottomSheetVisibility>["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}
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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<string, unknown>, ref: React.ForwardedRef<unknown>) => {
|
||||
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}
|
||||
>
|
||||
<div>Sheet content</div>
|
||||
</IsolatedBottomSheetModal>
|
||||
);
|
||||
}
|
||||
|
||||
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(<Harness visible={false} onClose={onClose} />);
|
||||
|
||||
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(<Harness visible={false} onClose={onClose} />);
|
||||
expect(modalMethods.close).toHaveBeenCalledTimes(1);
|
||||
expect(modalMethods.dismiss).not.toHaveBeenCalled();
|
||||
expect(modalMethods.dismiss).toHaveBeenCalledTimes(1);
|
||||
expect(modalMethods.close).not.toHaveBeenCalled();
|
||||
|
||||
rerender(<Harness visible onClose={onClose} />);
|
||||
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(<Harness visible onClose={onClose} />);
|
||||
|
||||
const latestProps = modalProps.mock.lastCall?.[0] as { onChange: (index: number) => void };
|
||||
latestProps.onChange(-1);
|
||||
latestModalProps().onChange?.(-1);
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
|
||||
rerender(<Harness visible={false} onClose={onClose} />);
|
||||
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(<Harness visible onClose={onClose} />);
|
||||
|
||||
latestModalProps().onDismiss?.();
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("deduplicates close notifications from change and dismiss callbacks", () => {
|
||||
const onClose = vi.fn();
|
||||
render(<Harness visible onClose={onClose} />);
|
||||
|
||||
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(<Harness visible onClose={onClose} />);
|
||||
|
||||
expect(modalMethods.present).not.toHaveBeenCalled();
|
||||
expect(modalMethods.snapToIndex).not.toHaveBeenCalled();
|
||||
|
||||
shouldExposeModalRef.current = true;
|
||||
rerender(<Harness visible onClose={onClose} />);
|
||||
|
||||
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(
|
||||
<IsolatedBottomSheetModal index={0} snapPoints={SNAP_POINTS_90}>
|
||||
|
||||
@@ -41,7 +41,7 @@ export const IsolatedBottomSheetModal = forwardRef<
|
||||
<GorhomBottomSheetModal
|
||||
{...bottomSheetProps}
|
||||
ref={ref}
|
||||
enableDismissOnClose={false}
|
||||
enableDismissOnClose
|
||||
stackBehavior={isNestedSheet ? "push" : "replace"}
|
||||
>
|
||||
{scopedChildren}
|
||||
@@ -68,39 +68,92 @@ export function useIsolatedBottomSheetVisibility({
|
||||
isEnabled?: boolean;
|
||||
onClose: () => void;
|
||||
}) {
|
||||
const sheetRef = useRef<IsolatedBottomSheetModalRef>(null);
|
||||
const hasPresentedRef = useRef(false);
|
||||
const sheetRef = useRef<IsolatedBottomSheetModalRef | null>(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,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user