Skip to content

fix(mock): fire onDismiss/onClose callbacks in BottomSheetModal and BottomSheet - #2714

Open
LucasWerey wants to merge 1 commit into
gorhom:masterfrom
LucasWerey:fix/mock-missing-callbacks
Open

fix(mock): fire onDismiss/onClose callbacks in BottomSheetModal and BottomSheet#2714
LucasWerey wants to merge 1 commit into
gorhom:masterfrom
LucasWerey:fix/mock-missing-callbacks

Conversation

@LucasWerey

Copy link
Copy Markdown

Problem

BottomSheetModal.dismiss(), BottomSheetModal.close(), BottomSheetModal.forceClose(), BottomSheet.close(), and BottomSheet.forceClose() in mock.js silently drop their respective onDismiss/onClose props.

Any component that relies on these callbacks for cleanup — e.g. queued drawer systems that need onDismiss to dequeue a sheet — will silently hang in tests: the queue entry stays stuck in a "dismissing" state and blocks subsequent sheets from opening.

This forces consumers to patch the mock themselves on a per-test basis, which is fragile and obscures the real issue.

Reproduction

const ref = React.createRef();
const onDismiss = jest.fn();

render(<BottomSheetModal ref={ref} onDismiss={onDismiss} />);
act(() => ref.current.dismiss());

expect(onDismiss).toHaveBeenCalled(); // ❌ fails — onDismiss is never called

Fix

Call this.props?.onDismiss?.() in dismiss(), and this.props?.onClose?.() in close() / forceClose() for both BottomSheetModal and BottomSheet.

  dismiss() {
    this.data = null;
+   this.props?.onDismiss?.();
  }

  close() {
    this.data = null;
+   this.props?.onClose?.();
  }

  forceClose() {
    this.data = null;
+   this.props?.onClose?.();
  }

…ottomSheet

dismiss(), close(), and forceClose() in the mock silently drop their
respective onDismiss/onClose props. Any component that relies on these
callbacks for cleanup (e.g. queued drawer systems) silently hangs in
tests — queue entries stay stuck in a 'dismissing' state and block
subsequent sheets from opening.

Fix: call this.props?.onDismiss?.() in dismiss(), and
this.props?.onClose?.() in close()/forceClose() for both
BottomSheetModal and BottomSheet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant