fix: support bidirectional (up & down) swipe-to-dismiss#156
Conversation
fix: support bidirectional (up & down) swipe-to-dismiss
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdated dismiss gesture and backdrop fade in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/useGestureViewer.ts`:
- Around line 478-479: The docs in src/types.ts for the dismiss API (references:
onDismiss and fadeBackdrop) still describe downward-only dismissal, but the
runtime (useGestureViewer.ts — check the canDismiss branch and
scheduleOnRN(handleDismiss) using event.translationY and
dismissOptions.threshold) now dismisses on both upward and downward swipes;
update the comment block describing onDismiss/fadeBackdrop (the existing doc
lines ~185-211) to state that dismissal is bidirectional (both positive and
negative translationY beyond threshold) and mention any related behavior (e.g.,
backdrop fade applies for either direction) so the API docs match the
implemented logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
@cljamal That said, changing the current behavior from downward-only dismiss to bidirectional dismiss by default could be a little confusing from a library/API perspective, since it changes the existing behavior users may already rely on. So I’m going to merge this PR, and then follow it up with an API update so users can choose the dismiss direction explicitly through: dismiss?: {
direction?: 'down' | 'up' | 'both';
}This way, users will be able to choose the dismiss direction explicitly, while we keep the behavior clear and predictable. This is planned for v2.3.0. Once that work is finished and released, I’ll leave another comment here with the update. |
|
Quick update: this has now been released in v2.3.0. As a follow-up, I added configurable dismiss directions via: dismiss.direction?: 'down' | 'up' | 'both'The default remains Thanks again for the contribution! |
|
Really glad my suggestion was helpful! The changes you made are honestly really elegant — thanks for that. And thank you for the package itself, it's been super useful to me. It felt great to finally contribute even a small piece back! 🙌 |
2 participants
Problem
Currently the dismiss gesture only triggers when swiping down (
translationY > threshold). Swiping up does nothing, which feels unnatural — most image viewers (Instagram, Telegram, etc.) dismiss on both directions.Fix
Two minimal changes in
useGestureViewer.ts:Math.abs(event.translationY)in the dismiss condition so both up and down swipes trigger dismiss.Math.abs(translateY.value)in the backdrop opacity interpolation so the background fades correctly when swiping upward.Changes
onEnd:event.translationY > threshold→Math.abs(event.translationY) > thresholdbackdropStyle:interpolate(translateY.value, ...)→interpolate(Math.abs(translateY.value), ...)Summary by CodeRabbit