Skip to content

Conversation

@crutch12
Copy link
Contributor

@crutch12 crutch12 commented Dec 3, 2025

Summary

Fixes #6605

useGesture's ref shouldn't be destroyed before onDragEnd call, it breaks useGesture's controller behaviour.

Behaviour changed: now the modal closes only when user releases finger (as I suppose it should be, every another app implements the same behaviour)

To Test

  1. Click on Connect Wallet
  2. Swipe the modal down swiftly, so modal should close
  3. Click on Connect Wallet again

Modal opens with origin Y position(0). You are able to swipe it again.

Background

  • y.interpolate is deprecated, use y.to
  • call y.set(0) to restore original Y position

Summary by CodeRabbit

  • New Features
    • Users can now drag modals downward to dismiss them on mobile devices, with support for both gesture movement and velocity-based detection.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Dec 3, 2025

@crutch12 is attempting to deploy a commit to the cow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Fixed a bug in the modal component where the Y position was not reset after dismissing via swipe gesture on mobile, preventing subsequent swipes. Implemented onDragEnd handler to detect drag intent and reset spring value to ensure proper modal state on reopening.

Changes

Cohort / File(s) Summary
Mobile Modal Gesture Handling
apps/cowswap-frontend/src/common/pure/Modal/index.tsx
Added onDragEnd handler to detect dismissal via swipe with velocity/movement threshold; resets spring value y to 0 after dismiss. Updated mobile transform calculation from interpolate-based to conditional translateY to constrain positive-only movement.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Focus on gesture binding logic: verify onDragEnd threshold conditions (movement/velocity) are appropriate for dismissal intent
  • Confirm spring value reset (y = 0) is called at the correct point in the dismiss flow to prevent state persistence
  • Validate mobile transform calculation handles edge cases (negative y values, initial state)

Suggested reviewers

  • elena-zh
  • cowdan

Poem

🐰 A swipe, a close, a lingering ghost—
The modal's Y forgot to reset its post!
Now onDragEnd brings the spring back home,
Fresh and ready to play and roam.
Bounce it open, swipe it down once more! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly identifies the fix for the modal swipe-close issue and references the linked issue number.
Description check ✅ Passed Description follows template with Summary, To Test, and Background sections; includes issue reference and clear explanation of changes.
Linked Issues check ✅ Passed Code changes directly address issue #6605 by fixing modal position reset and adding onDragEnd handler for proper gesture controller behavior.
Out of Scope Changes check ✅ Passed All changes are focused on the modal gesture handling and directly related to fixing the reported swipe-close issue; no out-of-scope modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd05bb7 and 00962a2.

📒 Files selected for processing (1)
  • apps/cowswap-frontend/src/common/pure/Modal/index.tsx (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
🔇 Additional comments (2)
apps/cowswap-frontend/src/common/pure/Modal/index.tsx (2)

53-64: Swipe-to-dismiss behavior and gesture fix look solid

Moving the dismiss logic into onDragEnd with distance/velocity thresholds and resetting y to 0 should prevent the gesture controller from getting stuck and ensures the modal reopens at the correct position. I don’t see functional issues with this setup.


88-93: Transform interpolation update is correct and constrained

Using y.to here is the right API for react-spring and the y > 0 ? y : 0 clamp avoids translating the modal upward while preserving the downward drag. This aligns with the intended mobile UX.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@elena-zh
Copy link
Contributor

elena-zh commented Dec 3, 2025

Hi @crutch12 , thank you for testing the app and fixing issues.
But could you please clarify an exact type of the device + OS version where this issue can be reproducible?
For now, I checked it on iOS 18.7.1 and Android 16 +Chrome, the issue is not reproducible there, so I'm not sure how to test your PR.
Thanks

@crutch12
Copy link
Contributor Author

crutch12 commented Dec 3, 2025

Hi @elena-zh

I use Chrome 142.0.7444.171, Android 16, incognito mode.

But I can reproduce with any mobile/emulated device (hope you've checked the video from related issue).

I just swipe this modal fast enough so I'm not able to see it again (it changes it's Y position). I'm not sure how to clarify it better 😅

Just don't release the finger till it reaches the bottom of the screen

Copy link
Collaborator

@shoom3301 shoom3301 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to reproduce the bug, but the changes look relevant

@crutch12
Copy link
Contributor Author

crutch12 commented Dec 4, 2025

Have checked on iphone safari, incognito

Caught the same problem 🥸

@crutch12
Copy link
Contributor Author

crutch12 commented Dec 4, 2025

Another video (android)

screen-20251204-104643.mp4
https://disk.yandex.ru/i/2bkCj9xhMglBaQ

@vercel
Copy link

vercel bot commented Dec 4, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
swap-dev Ready Ready Preview Dec 4, 2025 2:40pm

Copy link
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Managed to reproduce. Thanks for fixing it.

@shoom3301 shoom3301 merged commit 3ea15ca into cowprotocol:develop Dec 5, 2025
9 of 15 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mobile: Connect Wallet modal breaks if was closed using swipe

3 participants