update bottom sheet backdrop close (DEV-2541)

libs/expo/shared/ui-components/src/lib/BottomSheet/BottomSheetModalControlled.spec.tsx

@@ -1,6 +1,7 @@
 import { act, render } from '@testing-library/react-native';
 import { ReactNode } from 'react';
 import { Text } from 'react-native';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { BottomSheetModalControlled } from './BottomSheetModalControlled';
 
 const mocks = vi.hoisted(() => ({
@@ -11,18 +12,43 @@ vi.mock('./providers/BottomSheetModal/useBottomSheet', () => ({
   useBottomSheet: () => ({ showBottomSheet: mocks.showBottomSheet }),
 }));
 
-type TRenderArgs = { closeSheet: () => void };
+type TRenderArgs = { closeSheet: () => void; id: string };
 
-function Controlled({ isOpen }: { isOpen: boolean }) {
+type TShowCall = {
+  render: (args: TRenderArgs) => ReactNode;
+  options: { onClose?: (id: string) => void };
+};
+
+function Controlled({
+  isOpen,
+  onClose,
+}: {
+  isOpen: boolean;
+  onClose?: () => void;
+}) {
   return (
-    <BottomSheetModalControlled isOpen={isOpen}>
+    <BottomSheetModalControlled isOpen={isOpen} onClose={onClose}>
       <Text>menu</Text>
     </BottomSheetModalControlled>
   );
 }
 
-function mountSheetRender(): (args: TRenderArgs) => ReactNode {
-  return mocks.showBottomSheet.mock.calls[0][0].render;
+function showCalls(): TShowCall[] {
+  return mocks.showBottomSheet.mock.calls.map((call) => call[0] as TShowCall);
+}
+
+/** Simulates the provider mounting the sheet that owns `closeSheet`. */
+function mountSheet(index: number, id: string, closeSheet: () => void) {
+  act(() => {
+    showCalls()[index].render({ id, closeSheet });
+  });
+}
+
+/** Simulates Gorhom reporting that a sheet fully dismissed. */
+function completeDismissal(index: number, id: string) {
+  act(() => {
+    showCalls()[index].options.onClose?.(id);
+  });
 }
 
 describe('BottomSheetModalControlled', () => {
@@ -39,9 +65,7 @@ describe('BottomSheetModalControlled', () => {
 
     // Sheet mounts while isOpen is true → it stays open.
     const closeSheet = vi.fn();
-    act(() => {
-      mountSheetRender()({ closeSheet });
-    });
+    mountSheet(0, 'sheet-1', closeSheet);
     expect(closeSheet).not.toHaveBeenCalled();
   });
 
@@ -50,9 +74,7 @@ describe('BottomSheetModalControlled', () => {
     rerender(<Controlled isOpen={true} />);
 
     const closeSheet = vi.fn();
-    act(() => {
-      mountSheetRender()({ closeSheet });
-    });
+    mountSheet(0, 'sheet-1', closeSheet);
 
     rerender(<Controlled isOpen={false} />);
     expect(closeSheet).toHaveBeenCalled();
@@ -68,7 +90,7 @@ describe('BottomSheetModalControlled', () => {
 
     const closeSheet = vi.fn();
     act(() => {
-      mountSheetRender()({ closeSheet });
+      showCalls()[0].render({ id: 'sheet-1', closeSheet });
     });
 
     // The close is scheduled on a microtask to avoid a render-phase setState.
@@ -78,4 +100,70 @@ describe('BottomSheetModalControlled', () => {
 
     expect(closeSheet).toHaveBeenCalled();
   });
+
+  it('reopening during a close keeps the NEW sheet functional (stale onClose ignored)', () => {
+    const parentClose = vi.fn();
+    const { rerender } = render(
+      <Controlled isOpen={false} onClose={parentClose} />,
+    );
+
+    // Open #1.
+    rerender(<Controlled isOpen={true} onClose={parentClose} />);
+    const close1 = vi.fn();
+    mountSheet(0, 'sheet-1', close1);
+
+    // Close via state (like Cancel).
+    rerender(<Controlled isOpen={false} onClose={parentClose} />);
+    expect(close1).toHaveBeenCalledTimes(1);
+
+    // Rapid reopen → a brand-new sheet is presented.
+    rerender(<Controlled isOpen={true} onClose={parentClose} />);
+    expect(mocks.showBottomSheet).toHaveBeenCalledTimes(2);
+    const close2 = vi.fn();
+    mountSheet(1, 'sheet-2', close2);
+
+    // The OLD sheet finishes dismissing AFTER the reopen. Its onClose is
+    // stale and must not notify the parent or break the new sheet.
+    completeDismissal(0, 'sheet-1');
+    expect(parentClose).not.toHaveBeenCalled();
+
+    // The new sheet must still be closable.
+    rerender(<Controlled isOpen={false} onClose={parentClose} />);
+    expect(close2).toHaveBeenCalledTimes(1);
+    expect(close1).toHaveBeenCalledTimes(1);
+  });
+
+  it('notifies the parent when the active sheet is dismissed externally', () => {
+    const parentClose = vi.fn();
+    const { rerender } = render(
+      <Controlled isOpen={false} onClose={parentClose} />,
+    );
+
+    rerender(<Controlled isOpen={true} onClose={parentClose} />);
+    mountSheet(0, 'sheet-1', vi.fn());
+
+    // Provider surfaces a user-initiated dismissal (backdrop / pan-down /
+    // header) via options.onClose with the active sheet's id.
+    completeDismissal(0, 'sheet-1');
+
+    expect(parentClose).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not notify the parent when the close originated from state', () => {
+    const parentClose = vi.fn();
+    const { rerender } = render(
+      <Controlled isOpen={false} onClose={parentClose} />,
+    );
+
+    rerender(<Controlled isOpen={true} onClose={parentClose} />);
+    mountSheet(0, 'sheet-1', vi.fn());
+
+    // State-driven close (isOpen false) — the parent already knows.
+    rerender(<Controlled isOpen={false} onClose={parentClose} />);
+
+    // Gorhom reports the dismissal completing.
+    completeDismissal(0, 'sheet-1');
+
+    expect(parentClose).not.toHaveBeenCalled();
+  });
 });

libs/expo/shared/ui-components/src/lib/BottomSheet/BottomSheetModalControlled.tsx

@@ -39,6 +39,11 @@ export function BottomSheetModalControlled(props: TProps) {
   const closeSheetRef = useRef<(() => void) | null>(null);
   const closingFromStateRef = useRef(false);
   const isOpenRef = useRef(isOpen);
+  const didQueueCloseRef = useRef(false);
+
+  // Id of the sheet this component currently considers "open". Lets each
+  // per-sheet onClose verify it belongs to the active sheet.
+  const activeSheetIdRef = useRef<string | null>(null);
 
   // Mutable ref container to stabilize sheet inputs by render + lifecycle callbacks
   const stableInputsRef = useRef({
@@ -59,35 +64,62 @@ export function BottomSheetModalControlled(props: TProps) {
     if (!isOpen) {
       if (closeSheetRef.current) {
         closingFromStateRef.current = true;
+        // Dying renders of this sheet must not re-queue a close.
+        didQueueCloseRef.current = true;
         closeSheetRef.current();
         closeSheetRef.current = null;
       }
 
+      // No active sheet while closed, so a reopen during the dismiss
+      // animation starts a fresh presentation.
+      activeSheetIdRef.current = null;
+
       return;
     }
 
-    if (closeSheetRef.current) {
+    // Only one active sheet per open-cycle.
+    if (activeSheetIdRef.current) {
       return;
     }
 
+    closingFromStateRef.current = false;
+    didQueueCloseRef.current = false;
+
     showBottomSheet({
-      render: ({ closeSheet }) => {
-        closeSheetRef.current = closeSheet;
+      render: ({ closeSheet, id }) => {
+        if (isOpenRef.current) {
+          // Normal open render — claim this sheet.
+          activeSheetIdRef.current = id;
+          closeSheetRef.current = closeSheet;
+          return stableInputsRef.current.children;
+        }
 
-        // The sheet can mount after `isOpen` has already flipped back to
-        // false (e.g. a selection closed the picker while the sheet was still
-        // presenting). Dismiss it right away so it never lingers open.
-        if (!isOpenRef.current) {
+        // Component is closed but this sheet mounted: either a mount-race
+        // (presented right as isOpen flipped false) or a dying sheet. Only
+        // the race queues a close, once.
+        if (!didQueueCloseRef.current) {
+          didQueueCloseRef.current = true;
           closingFromStateRef.current = true;
+          activeSheetIdRef.current = id;
+          closeSheetRef.current = closeSheet;
           queueMicrotask(() => closeSheetRef.current?.());
         }
 
         return stableInputsRef.current.children;
       },
       options: {
         ...(stableInputsRef.current.options ?? {}),
-        onClose: () => {
+        onClose: (closingId: string) => {
+          // A dismissal from a sheet that is no longer the active one (e.g. a
+          // superseded sheet whose dismissal finished after a new one opened)
+          // must not touch the shared refs or notify the parent.
+          if (closingId !== activeSheetIdRef.current) {
+            return;
+          }
+
+          activeSheetIdRef.current = null;
           closeSheetRef.current = null;
+          didQueueCloseRef.current = false;
 
           // only notify parent if sheet initiated the close
           if (!closingFromStateRef.current) {

libs/expo/shared/ui-components/src/lib/BottomSheet/core/BottomSheetBackdrop.spec.tsx

@@ -0,0 +1,83 @@
+import { fireEvent, render } from '@testing-library/react-native';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { BottomSheetBackdrop } from './BottomSheetBackdrop';
+
+/**
+ * BottomSheetBackdrop
+ *
+ * Documents the dismissal contract:
+ * - a tap is forwarded to onRequestClose (the app's reliable dismiss path)
+ * - Gorhom's own close is NOT used (pressBehavior must not be 'close'),
+ *   otherwise the provider's forceClose and Gorhom's close() would double-dismiss
+ * - a numeric pressBehavior keeps the tap gesture attached while doing nothing
+ */
+
+const mocks = vi.hoisted(() => ({
+  onGorhomProps: vi.fn(),
+}));
+
+vi.mock('@gorhom/bottom-sheet', () => {
+  const { Pressable } = require('react-native');
+
+  return {
+    BottomSheetBackdrop: (props: {
+      onPress?: () => void;
+      pressBehavior?: unknown;
+    }) => {
+      mocks.onGorhomProps(props);
+      return (
+        <Pressable
+          testID="gorhom-backdrop"
+          accessibilityRole="button"
+          onPress={props.onPress}
+        />
+      );
+    },
+  };
+});
+
+describe('BottomSheetBackdrop', () => {
+  // Gorhom passes animatedIndex/animatedPosition into backdrop components; the
+  // component only forwards them, so a plain object suffices here.
+  const fakeSharedValue = { value: 0 } as never;
+
+  beforeEach(() => {
+    mocks.onGorhomProps.mockClear();
+  });
+
+  it('forwards taps to onRequestClose as the single dismissal path', () => {
+    const onRequestClose = vi.fn();
+    const { getByTestId } = render(
+      <BottomSheetBackdrop
+        animatedIndex={fakeSharedValue}
+        animatedPosition={fakeSharedValue}
+        onRequestClose={onRequestClose}
+      />,
+    );
+
+    const gorhomProps = mocks.onGorhomProps.mock.calls[0][0];
+
+    // Gorhom must not close the sheet itself — the provider force-closes via
+    // onRequestClose. Keeping 'close' here would double-dismiss.
+    expect(gorhomProps.pressBehavior).not.toBe('close');
+
+    // A numeric pressBehavior keeps the tap gesture (onPress fires) but is a
+    // no-op snap, so dismissal happens exactly once, through onRequestClose.
+    expect(gorhomProps.pressBehavior).toBe(0);
+    expect(gorhomProps.onPress).toBe(onRequestClose);
+
+    fireEvent.press(getByTestId('gorhom-backdrop'));
+    expect(onRequestClose).toHaveBeenCalledTimes(1);
+  });
+
+  it('renders nothing when disableBackdrop is set', () => {
+    const { queryByTestId } = render(
+      <BottomSheetBackdrop
+        animatedIndex={fakeSharedValue}
+        animatedPosition={fakeSharedValue}
+        disableBackdrop
+      />,
+    );
+    expect(queryByTestId('gorhom-backdrop')).toBeNull();
+  });
+});

libs/expo/shared/ui-components/src/lib/BottomSheet/core/BottomSheetBackdrop.tsx

@@ -41,6 +41,12 @@ type BottomSheetBackdropWrapperProps = BottomSheetBackdropProps & {
    * If provided, it fully replaces the default implementation.
    */
   component?: ComponentType<BottomSheetBackdropProps>;
+
+  /**
+   * App-level dismiss request (dismissSheetById → modal dismiss/forceClose).
+   * A tap routes through here as the single dismissal path.
+   */
+  onRequestClose?: () => void;
 };
 
 export function BottomSheetBackdrop(
@@ -50,6 +56,7 @@ export function BottomSheetBackdrop(
     disableBackdrop,
     opacity = 0.5,
     component: CustomComponent,
+    onRequestClose,
     ...rest
   } = props;
 
@@ -67,7 +74,13 @@ export function BottomSheetBackdrop(
       appearsOnIndex={0}
       disappearsOnIndex={-1}
       opacity={opacity}
-      pressBehavior="close"
+      // Keep the tap gesture attached (onPress fires only when pressBehavior
+      // is not 'none'), but make Gorhom's own action a no-op: sheets here open
+      // at index 0, so snapping to 0 does nothing. Dismissal happens through
+      // onRequestClose — NOT Gorhom's internal close() — to avoid a double
+      // dismissal when the provider force-closes.
+      pressBehavior={0}
+      onPress={onRequestClose}
     />
   );
 }

libs/expo/shared/ui-components/src/lib/BottomSheet/core/BottomSheetBase.tsx

@@ -112,7 +112,11 @@ const BottomSheetBase = forwardRef<GbsBottomSheetModal, TBottomSheetModal>(
 
     if (!disableBackdrop) {
       backdropComponent = (backdropProps) => (
-        <BottomSheetBackdrop {...backdropProps} opacity={backdropOpacity} />
+        <BottomSheetBackdrop
+          {...backdropProps}
+          opacity={backdropOpacity}
+          onRequestClose={onRequestClose}
+        />
       );
     }