Back to Tamagui

RNGH Sheet Integration - TDD Plan

plans/rngh-sheet.md

1.144.422.0 KB
Original Source

RNGH Sheet Integration - TDD Plan

Current Status: ✅ COMPLETE - READY FOR MERGE

Last Update (Jan 24): All tests pass, code review complete, docs written.

Final Implementation Summary

What was implemented:

  • Native gesture handler integration for Sheet + ScrollView coordination
  • Smooth scroll-to-drag handoffs without jitter
  • Rubber band resistance at top
  • Direction change support mid-gesture
  • Zero dependencies on react-native-reanimated
  • Clean fallback to PanResponder on web and when RNGH not set up

Test Results:

  • ✅ 11 native Detox tests pass (all RNGH coordination scenarios)
  • ✅ Web Playwright tests pass (CSS and Motion drivers)
  • ✅ Video analysis confirms no visible jumping

User Feedback:

  • "not jittering or locking as much"
  • "feels less jittery"

Test Cases Covered

  1. Scroll at top - swipe up scrolls content, swipe down drags sheet
  2. Drag from middle - dragging up/down moves sheet, no scroll
  3. Handoff scroll→drag - scroll to 0 then continue dragging sheet
  4. Handoff drag→scroll - drag to top then continue into scrolling
  5. Direction changes - multiple up/down changes without getting stuck
  6. Rubber band - dragging past top resists progressively

Rubber Band Test Cases (added late)

  1. Drag up on handle from top - should go "past" the top but resist progressively (already implemented via resisted())
  2. Sheet with content < full height - should resist the same way on drag up (no scrollable content)
  3. Hard mode: Sheet with scrollable content - should resist/move once you hit bottom of scrollable content and keep pulling up

The Core Problem

When sheet is at middle position and user does FAST swipe up:

  • Native scroll fires BEFORE our JS-thread onBegin callback
  • Scroll reaches 20-30px before we can lock it
  • If we use scrollEnabled={false}, scroll is locked BUT handoff breaks (Case 9 fails)
  • If we keep scroll enabled, handoff works BUT fast swipes leak scroll

The Solution: Worklets

Like gorhom/bottom-sheet and react-native-actions-sheet, we need synchronous native-thread control. Instead of requiring full Reanimated, we can use the lighter react-native-worklets-core package.

New Architecture - @tamagui/native package:

tsx
// Entry points (side-effect imports only, no setup() functions):
import '@tamagui/native/setup-gesture-handler' // RNGH
import '@tamagui/native/setup-worklets' // react-native-worklets-core

// In your app:
import '@tamagui/native/setup-gesture-handler'
import '@tamagui/native/setup-worklets'
// That's it! Sheet will automatically use worklets when available

See: https://docs.swmansion.com/react-native-worklets/docs/


Previous Status: ALL 10 TESTS PASS ✓

All 10 Detox Tests PASS

✓ should show RNGH enabled
✓ should open sheet at position 0
✓ Case 1: swipe DOWN at scrollY=0 should drag sheet, NOT scroll
✓ Case 2: at top snap, swipe UP should scroll content
✓ Case 3: drag sheet up from position 1 should NOT scroll simultaneously
✓ Case 4: scroll down, then swipe down should scroll back to 0 first
✓ Case 5: HANDOFF - scroll to 0 then drag sheet in one gesture
✓ Case 6: multiple direction changes without getting stuck
✓ Case 7: drag UP from position 1 should NOT scroll content (new!)
✓ Case 8: rubber band at top - dragging up keeps sheet at top (new!)

Latest Bug Fix (Jan 23 session)

Problem: When sheet was at position 1 (non-top) and user dragged UP, scroll content was firing even though pan should handle the gesture. Max scroll Y was reaching 400+ pixels.

Root Cause: Scroll events were being processed BEFORE pan gesture's onBegin callback had a chance to set the scroll lock. The simultaneousHandlers pattern means both gestures run, but JS callbacks can't prevent native scroll events in time.

Solution: Preemptive scroll state management based on sheet position:

  1. In useAnimatedNumberReaction, track when sheet reaches/leaves top position
  2. When sheet leaves top → immediately disable scroll via setScrollEnabled(false) and setNativeProps
  3. When sheet reaches top → enable scroll
  4. This ensures scroll is disabled BEFORE any gesture starts, not reactively
tsx
// In SheetImplementationCustom.tsx useAnimatedNumberReaction callback:
const nowAtTop = value <= minY + 5
if (wasAtTop !== nowAtTop) {
  scrollBridge.isAtTop = nowAtTop
  if (nowAtTop) {
    scrollBridge.scrollLockY = undefined
    scrollBridge.setScrollEnabled?.(true)
  } else {
    scrollBridge.scrollLockY = 0
    scrollBridge.setScrollEnabled?.(false)
  }
}

Test Comparison with Reference Implementations

gorhom/bottom-sheet:

  • Uses Reanimated worklets for synchronous native thread control
  • SCROLLABLE_STATUS.LOCKED sets decelerationRate: 0 to effectively freeze scroll
  • Uses animatedScrollableState shared value for immediate state changes

react-native-actions-sheet:

  • Uses scrollable(true/false) function called in onChange handler
  • Relies on setNativeProps({ scrollEnabled: value }) for fast updates
  • Has same simultaneousHandlers pattern we use

Our approach (without Reanimated):

  • Preemptively set scroll state based on sheet position
  • Use both setNativeProps and React state as backup
  • Track isAtTop flag that changes when animated position crosses threshold
  • Works because state is set BEFORE gesture starts, not during

Architecture Notes

  • No RNGH on web - falls back to PanResponder/touch events
  • No Reanimated dependency - all JS-based
  • Setup via setupGestureHandler({ Gesture, GestureDetector, ScrollView }) to avoid double registration
  • Clean separation in gestureState.ts module

Remaining Work

  • Move setup to @tamagui/native-gestures package
  • Write docs and blog post
  • Test web sheets haven't regressed
  • Run release dry-run

Delta-based Multi-Handoff Pattern

Manual Testing Needed

The "big test" from user requirements:

  1. Start at position 2 (halfway)
  2. Swipe up to drag sheet to top (pan handles)
  3. Keep swiping up → scroll content (handoff TO scroll)
  4. Change direction down → scroll back to 0 (scroll handles)
  5. Continue down after scroll hits 0 → drag sheet (handoff FROM scroll)
  6. Change direction up → drag sheet up (pan handles)
  7. Hit top, continue up → scroll again (handoff TO scroll again)

ALL WITHOUT LIFTING FINGER

Critical Bug Fixed (This Session)

Problem: require('react-native-gesture-handler') inside setupGestureHandler() caused:

Invariant Violation: Tried to register two views with the same name RNGestureHandlerButton

This happened because the app imports RNGH at the top (import 'react-native-gesture-handler') and then our setup tried to require it again.

Solution: Changed API to accept config object instead of auto-detecting:

tsx
// OLD (broken):
import 'react-native-gesture-handler'
import { setupGestureHandler } from '@tamagui/sheet/setup-gesture-handler'
setupGestureHandler() // ❌ tries to require RNGH again, double registration

// NEW (fixed):
import {
  Gesture,
  GestureDetector,
  GestureHandlerRootView,
} from 'react-native-gesture-handler'
import { setupGestureHandler } from '@tamagui/sheet/setup-gesture-handler'
setupGestureHandler({ Gesture, GestureDetector }) // ✅ uses already-imported refs

Test Results (5 PASS, 1 SKIPPED)

  • ✓ should render the test case screen
  • ✓ should open sheet and show scroll Y at 0
  • should scroll content when swiping up at top snap point
  • should drag sheet down when at scrollY=0 and swiping down - NO SCROLL
  • BUG TEST: should NOT scroll while dragging sheet down
  • ○ skipped: should hand off from scroll to sheet drag when scrollY reaches 0 (advanced case)

Key Implementation Details

1. manualActivation Pattern Instead of activeOffsetY([-5, 5]) which always activates after 5px movement, we use:

tsx
Gesture.Pan()
  .manualActivation(true)
  .onTouchesMove((event, stateManager) => {
    // decide whether to activate pan or let scroll handle it
    if (isFullOpen && hasScrollableContent && !nodeIsScrolling) {
      stateManager.fail() // let native scroll work
      return
    }
    if (nodeIsScrolling) {
      stateManager.fail() // scroll is handling it
      return
    }
    stateManager.activate() // activate pan for sheet drag
  })

2. Dynamic scrollBridge Checks Read scrollBridge.hasScrollableContent inside gesture handlers (not captured props) so it reflects current state.

3. simultaneousWithExternalGesture The ScrollView uses Gesture.Native().simultaneousWithExternalGesture(panGesture) to coordinate with the sheet's pan gesture.

Files Modified

  • useGestureHandlerPan.tsx - manualActivation pattern, removed hasScrollView prop
  • SheetScrollView.tsx - cleaned up debug code
  • SheetImplementationCustom.tsx - removed hasScrollView prop
  • e2e/SheetScrollableDrag.test.ts - fixed assertions, added item visibility checks

TODO: Advanced Handoff

The scroll→drag handoff (scroll back to 0 then drag sheet) needs dynamic re-evaluation during gesture. This is an advanced feature for future work.

Problem Summary

Sheet + Sheet.ScrollView gesture coordination on iOS is fundamentally broken with PanResponder because iOS's UIScrollView gesture recognizers fire BEFORE React Native's responder system.

Expected Native-Quality Behavior

  1. At top snap point + swipe up → scroll content naturally
  2. At top snap point + swipe down → rubber band bounce at top
  3. Scrolled down + drag down → scroll to top THEN SEAMLESSLY hand off to sheet drag
  4. Dragging up + hit sheet top → SEAMLESSLY continue into scrolling

Critical Bug Found (This Iteration)

Issue: scrollBridge.y is always 0 when using RNGH path.

Root Cause: The onScroll handler in SheetScrollView only fires during normal scroll events, but when RNGH takes over the gesture coordination, the native scroll events may not fire the same way.

Evidence: User testing on iOS simulator showed "it doesn't do nice handoff" and "scrollview y btw isn't updating how you'd expect - always 0"

Files Implemented So Far

Core Infrastructure (DONE)

  • code/ui/sheet/src/gestureState.ts - Global state (no native deps)
  • code/ui/sheet/src/setupGestureHandler.ts - Auto-detects RNGH via require()
  • code/ui/sheet/src/useGestureHandlerPan.tsx - Pan gesture hook with blockPan
  • code/ui/sheet/src/GestureDetectorWrapper.tsx - Conditional wrapper
  • code/ui/sheet/src/GestureSheetContext.tsx - Context for gesture refs

Integration (DONE)

  • code/ui/sheet/src/SheetImplementationCustom.tsx - Uses hook, falls back to PanResponder
  • code/ui/sheet/src/SheetScrollView.tsx - simultaneousWithExternalGesture
  • code/ui/sheet/package.json - Export and optional peer dep

Kitchen Sink (DONE)

  • code/kitchen-sink/src/App.native.tsx - Calls setupGestureHandler()
  • code/kitchen-sink/src/usecases/SheetScrollableDrag.tsx - Test case
  • code/kitchen-sink/src/features/home/screen.tsx - RNGH status indicator

Next Steps (Priority Order)

1. FIX: ScrollView Y Tracking with RNGH

The scrollBridge.y must be updated continuously even when RNGH is handling gestures.

Approach A: Use ScrollView's onScroll with scrollEventThrottle={1} for maximum frequency Approach B: Use a native scroll handler attached via ref Approach C: Track scroll offset via Gesture.Native() event handlers

Look at how gorhom/bottom-sheet tracks animatedScrollableState.contentOffsetY:

tsx
// They use Reanimated's scrollTo and track via worklets
// We need a JS-based equivalent since we don't require Reanimated

2. Write Failing Detox Tests First

Create code/kitchen-sink/tests/SheetScrollableDrag.detox.test.ts:

typescript
describe('Sheet + ScrollView RNGH Integration', () => {
  beforeAll(async () => {
    await device.launchApp()
    // Navigate to SheetScrollableDrag test case
  })

  it('should show RNGH enabled indicator', async () => {
    // Verify setupGestureHandler() worked
    await expect(element(by.text('RNGH: ✓ enabled'))).toBeVisible()
  })

  it('scrolls content when at top snap point and swiping up', async () => {
    // Open sheet
    await element(by.id('sheet-scrollable-drag-trigger')).tap()
    await waitFor(element(by.id('sheet-scrollable-drag-frame'))).toBeVisible()

    // Swipe up on content
    await element(by.id('sheet-scrollable-drag-scrollview')).swipe('up', 'slow', 0.5)

    // Take screenshot to verify scroll position changed
    // Check scroll-y indicator shows > 0
    await expect(element(by.id('sheet-scrollable-drag-scroll-y'))).not.toHaveText(
      'ScrollView Y: 0'
    )

    // Sheet position should still be 0 (top snap)
    await expect(element(by.id('sheet-scrollable-drag-position'))).toHaveText(
      'Sheet position: 0'
    )
  })

  it('drags sheet when swiping down from scrollY=0', async () => {
    // Reset and open sheet
    await element(by.id('sheet-scrollable-drag-reset')).tap()
    await element(by.id('sheet-scrollable-drag-trigger')).tap()

    // Swipe down on handle/content when at scroll top
    await element(by.id('sheet-scrollable-drag-handle')).swipe('down', 'slow', 0.3)

    // Sheet should have moved to lower snap point
    await expect(element(by.id('sheet-scrollable-drag-position'))).toHaveText(
      'Sheet position: 1'
    )
  })

  it('hands off from scroll to sheet drag seamlessly', async () => {
    // Open sheet and scroll down
    await element(by.id('sheet-scrollable-drag-trigger')).tap()
    await element(by.id('sheet-scrollable-drag-scrollview')).swipe('up', 'slow', 0.5)

    // Now drag down - should scroll to top first, then drag sheet
    await element(by.id('sheet-scrollable-drag-scrollview')).swipe('down', 'slow', 0.8)

    // Check: scroll should be at 0
    await expect(element(by.id('sheet-scrollable-drag-scroll-y'))).toHaveText(
      'ScrollView Y: 0'
    )

    // Check: sheet should have moved down (position > 0 or dismissed)
    // This is the key handoff test
  })

  it('hands off from sheet drag to scroll seamlessly', async () => {
    // Open sheet at lower snap point
    await element(by.id('sheet-scrollable-drag-trigger')).tap()
    // ... drag to lower snap

    // Drag up past top snap point
    // Content should start scrolling
  })
})

3. Fix Implementation Based on Test Results

Once we have failing tests, we can properly debug with evidence.

How Reference Implementations Solve This

gorhom/bottom-sheet

Uses Reanimated worklets to track scroll offset in real-time:

tsx
const animatedScrollableState = useSharedValue({
  contentOffsetY: 0,
  // ...
})

// Updated via scrollHandler worklet
const scrollHandler = useAnimatedScrollHandler({
  onScroll: (event) => {
    animatedScrollableState.value = {
      contentOffsetY: event.contentOffset.y,
      // ...
    }
  },
})

react-native-actions-sheet

Tracks scroll via refs and direct measurements:

tsx
// They store scroll offset per-node
const offsets = useRef<number[]>([])

// Updated in scroll handler
offsets.current[i] = scrollY

Our Solution (Without Reanimated)

Since we don't require Reanimated, we need to:

  1. Ensure onScroll fires frequently: Set scrollEventThrottle={1}
  2. Use ref to get scroll position imperatively: scrollRef.current?.scrollTo() can also read position
  3. Consider using native event listeners: Attach directly to scroll node
tsx
// In SheetScrollView
useEffect(() => {
  if (!isGestureHandlerEnabled()) return

  // For RNGH path, we may need to poll or use a different mechanism
  const scrollNode = scrollRef.current?.getScrollableNode()
  if (!scrollNode) return

  // Option: Native scroll listener
  const handleScroll = (e) => {
    scrollBridge.y = e.nativeEvent.contentOffset.y
  }

  // This might work better with RNGH
}, [])

Checklist

  • Infrastructure files created
  • Integration into Sheet components
  • Kitchen sink setup with indicator
  • FIX: Double-registration bug - Changed API to accept {Gesture, GestureDetector} config
  • Added quick-access test link on home screen
  • VERIFY: scrollBridge.y updates correctly
  • FIX: onScroll not firing issue
  • Write Detox tests that capture actual scroll position
  • Screenshot-based verification in tests
  • Run full test suite
  • Commit and push
  • Monitor CI

Session Notes (Jan 23, 2025)

Key Learning from Studying Local Repos

Studied both ~/github/react-native-bottom-sheet and ~/github/react-native-actions-sheet:

Critical Finding: Neither uses manualActivation!

Both use:

  1. simultaneousHandlers - let both gestures run simultaneously
  2. State-based decision logic in onChange - decide who "owns" the gesture
  3. blockPan flag - skip pan processing when scroll should handle it
  4. scrollable(true/false) - dynamically enable/disable scroll on nodes

react-native-actions-sheet Decision Matrix (lines 807-872)

Case 1: Sheet not fully open + swiping up
→ scrollable(false), blockPan = false  [PAN HANDLES IT]

Case 2: Sheet fully open + swiping up
→ scrollable(true), blockPan = true    [SCROLL HANDLES IT]

Case 3: Sheet not fully open + swiping down
→ if (nodeIsScrolling)
  - scrollable(true), blockPan = true  [SCROLL HANDLES IT]
  else
  - scrollable(false), blockPan = false [PAN HANDLES IT]

Case 4: Sheet fully open + swiping down + scroll offset > 0
→ if (nodeIsScrolling)
  - scrollable(true), blockPan = true  [SCROLL HANDLES IT]
  else
  - scrollable(false), blockPan = false [PAN HANDLES IT]

Key Implementation Pattern

tsx
// In onChange handler:
const isFullOpen = currentSheetPos <= minY + 5
const isSwipingDown = deltaY > 0
const nodeIsScrolling = scrollBridge.y > 0

if (!isFullOpen && !isSwipingDown) {
  // Case 1: sheet not fully open, swiping up -> pan handles
  scrollBridge.setScrollEnabled?.(false)
  // process pan...
} else if (isFullOpen && !isSwipingDown) {
  // Case 2: sheet fully open, swiping up -> scroll handles
  scrollBridge.setScrollEnabled?.(true)
  return // blockPan
} else if (!isFullOpen && isSwipingDown) {
  // Case 3: sheet not fully open, swiping down
  if (nodeIsScrolling) {
    scrollBridge.setScrollEnabled?.(true)
    return // blockPan
  }
  scrollBridge.setScrollEnabled?.(false)
  // process pan...
} else if (isFullOpen && isSwipingDown) {
  // Case 4: sheet fully open, swiping down
  if (nodeIsScrolling) {
    scrollBridge.setScrollEnabled?.(true)
    return // blockPan
  }
  scrollBridge.setScrollEnabled?.(false)
  // process pan...
}

Current Issue

The manualActivation pattern I tried doesn't work - the onTouchesMove callbacks aren't firing on the content area. Need to switch to the state-based approach used by both reference implementations.

Current Debug Session

Testing with debug logs in:

  • SheetScrollView.tsx - logs [SheetScrollView] onScroll called, y: <number>
  • useGestureHandlerPan.tsx - logs gesture activation decisions
  • App.native.tsx / HomeScreen - logs RNGH status

To verify:

  1. Home screen should show "RNGH: ✓ enabled"
  2. Open sheet, swipe up → should see onScroll logs in Metro console
  3. ScrollView Y should update from 0

Debug Commands

bash
# Start Metro for kitchen-sink
cd code/kitchen-sink && yarn start --port 8081

# Run iOS app
npx expo run:ios

# Check RNGH status in app
# Look for "RNGH: ✓ enabled" on home screen

# Run Detox tests (when written)
yarn detox build -c ios.sim.debug
yarn detox test -c ios.sim.debug tests/SheetScrollableDrag.detox.test.ts

Session Notes (Jan 24, 2025 - Final Session)

What Worked Well

  1. TDD approach with Detox - Writing tests first helped define clear success criteria
  2. Reference implementations - Studying react-native-actions-sheet and gorhom/bottom-sheet was invaluable
  3. State-based decision pattern - Using delta-based direction detection and accumulated offsets
  4. Freeze at current position - Key insight: lock scroll at current Y, not always 0
  5. Continuous offset tracking - currentScrollOffset.current updated on every scroll event
  6. Sub-agent code reviews - Parallel reviews of different concerns caught issues early

What Didn't Work

  1. manualActivation pattern - onTouchesMove callbacks didn't fire reliably on scroll content
  2. Initial scroll lock at 0 - Caused jitter; needed to lock at current position
  3. Velocity-based direction detection - Too noisy; translation delta more reliable

Key Insights

  1. JS thread limitation - Native scroll fires before JS callbacks; must preemptively manage state
  2. simultaneousHandlers is key - Let both gestures run, decide per-frame who handles
  3. No Reanimated needed - runOnJS(true) works fine for our use case
  4. scrollTo for iOS freeze - iOS uses scrollTo to lock, Android uses setNativeProps

Architecture Decisions

  1. Setup via @tamagui/native - Follows portal pattern, side-effect imports only
  2. Optional peer dep - RNGH is optional, clean fallback to PanResponder
  3. No RNGH on web - Web always uses PanResponder (works fine)
  4. ScrollBridge for coordination - Centralized state between pan and scroll

Files Changed

  • code/ui/sheet/src/gestureState.ts - Re-exports from @tamagui/native
  • code/ui/sheet/src/useGestureHandlerPan.tsx - Pan gesture with scroll coordination
  • code/ui/sheet/src/SheetScrollView.tsx - RNGH ScrollView with simultaneousHandlers
  • code/ui/sheet/src/GestureDetectorWrapper.tsx - Conditional gesture wrapper
  • code/ui/sheet/src/GestureSheetContext.tsx - Context for gesture ref sharing
  • code/ui/sheet/src/SheetImplementationCustom.tsx - Integration with fallback
  • code/ui/sheet/src/types.tsx - Extended ScrollBridge type
  • code/core/native/src/setup-gesture-handler.ts - Side-effect setup

Ralph Loop Notes

This task was driven by a ralph loop prompt asking to achieve native-quality sheet gestures. Key requirements from the loop:

  1. ✅ Change directions works, snapping, fundamentals
  2. ✅ Rubber band resistance when dragging past top
  3. ✅ Handoff between scroll/drag - the "big test"
  4. ✅ Run Detox tests and record video
  5. ✅ No RNGH on web, no reanimated dependency
  6. ✅ Clean typed implementation
  7. ✅ Write docs

The session took approximately 4 hours of focused work across multiple iterations.