MetaMask / MetaMask/metamask-design-system

BottomSheet: evaluate adopting @gorhom/bottom-sheet to replace bespoke gesture implementation

Open
#1,183 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
TypeScript
Stars
37
Forks
14
Avg merge
1d 9h
Merged PRs (30d)
60

Description

## Background

PR #1165 migrated `BottomSheetDialog` from the RNGH v1 `PanGestureHandler` / `useAnimatedGestureHandler` API to the RNGH v2 `GestureDetector` / `Gesture.Pan()` API, required by the upgrade to `react-native-reanimated ~4.1.1` and `react-native-gesture-handler ~2.28.0`.

As part of that migration, `panGestureHandlerProps` was removed as a breaking change. It was a passthrough shim that gave consumers access to the full `PanGestureHandler` configuration surface — things like `simultaneousHandlers`, `activeOffsetY`, `failOffsetX`, `hitSlop`, `minDist`. The prop was never called in MetaMask Mobile or Extension consumer code, and the `simultaneousHandlers` use case (the only real one in the codebase) was already silently broken because `applyPanGestureProps` never mapped it to `gesture.simultaneousWithExternalGesture()`.

See MIGRATION.md for the breaking change documentation.

## The problem this creates

Without `panGestureHandlerProps`, consumers have no escape hatch to configure the underlying pan gesture. The most immediately impacted case is **scrollable content inside a BottomSheet on Android** — without gesture coordination, the bottom sheet's pan gesture intercepts scroll attempts and dismisses the sheet instead of scrolling the list.

The `ScrollableList` Storybook story demonstrates this: it still passes a `ref` to the `ScrollView` but that ref is now disconnected — there is nothing coordinating the scroll and dismiss gestures.

## Options explored

### Option A: Expose individual first-class props
Add explicit props for common configurations (`activeOffsetY`, `simultaneousWithRef`, etc.). Clean public API but the component becomes the gatekeeper — if a consumer needs something not exposed, they're stuck.

### Option B: `gestureModifier` callback
Give consumers a function that receives the base `Gesture.Pan()` object and returns it configured:

```tsx

gesture
.activeOffsetY([-5, 5])
.simultaneousWithExternalGesture(scrollRef)
}
>
```

This is the v2-idiomatic equivalent of `panGestureHandlerProps` — same full-surface access, type-safe, consumer owns any configuration layered on top of the component's core behaviour. The component creates the base gesture with its internal `onStart`/`onUpdate`/`onEnd` logic and passes it to the modifier before using it.

### Option C: Adopt `@gorhom/bottom-sheet` ← preferred direction
Replace the bespoke gesture management entirely. `@gorhom/bottom-sheet` v5 handles simultaneous gesture coordination with scroll views, keyboard avoidance, and sheet snapping as first-class concerns. The consumer never needs to think about gesture configuration.

`@gorhom/bottom-sheet` is already present as a devDependency in `apps/storybook-react-native`. The current `BottomSheetDialog` implementation is ~300 lines managing concerns that `@gorhom/bottom-sheet` handles out of the box.

## Primary goal: verify compatibility with MetaMask Mobile

Before committing to Option C, confirm that `@gorhom/bottom-sheet` v5 is compatible with the MetaMask Mobile stack:

- [ ] Check if MetaMask Mobile already uses `@gorhom/bottom-sheet` anywhere — if so, which version
- [ ] Verify peer dependency compatibility: `@gorhom/bottom-sheet` v5 requires `react-native-reanimated >= 3` and `react-native-gesture-handler >= 2` (both satisfied on our current stack)
- [ ] Audit the `BottomSheet` / `BottomSheetDialog` public API surface against `@gorhom/bottom-sheet`'s API — identify what maps cleanly and what would be a breaking change for consumers
- [ ] Check if MetaMask Mobile uses `BottomSheet` from this design system or its own copy in `app/component-library/`

## Intermediate path (if Option C is blocked)

If `@gorhom/bottom-sheet` adoption needs more time, implement Option B (`gestureModifier` callback) as a non-breaking addition to restore the escape hatch:

```tsx
export type BottomSheetDialogProps = {
// ...existing props
gestureModifier?: (
gesture: ReturnType,
) => ReturnType;
};
```

This unblocks the `simultaneousHandlers` scroll use case without re-introducing the v1 prop surface.

## Related

- PR #1165 — removes `panGestureHandlerProps`, documents breaking change in MIGRATION.md
- Issue #1181 — other BottomSheetDialog follow-up items (stale type, `combinedSheetStyle` dep array)
- [gorhom/bottom-sheet v5 docs](https://gorhom.dev/react-native-bottom-sheet/)
- [MetaMask Mobile component-library BottomSheet](https://github.com/MetaMask/metamask-mobile/tree/main/app/component-library/components/BottomSheets)

Contributor guide

Open the contributing guide

Research direction

Start by locating the current BottomSheetDialog implementation and the ScrollableList Storybook story, then inspect MIGRATION.md and the existing @gorhom/bottom-sheet dependency in apps/storybook-react-native. Check MetaMask Mobile's component-library BottomSheets path and audit the public API against the proposed dependency. Done means documenting compatibility, breaking changes, and whether adoption or gestureModifier is the appropriate path.

Written by the indexing model from the issue text.

Assessment

Tech stack
react-native, typescript
Domain
mobile
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.