Skip to content

Conversation

@LunrEclipse
Copy link
Contributor

@LunrEclipse LunrEclipse commented Jun 1, 2023

Description

Adds features to have Mini Portfolio close when users on mobile scroll down.

Adds an event listener to AccountDrawer to see if a user swipes down and if so, will trigger toggleWalletDrawer to close the Mini Portfolio.

Linear ticket: WEB-1801

Screen capture

Before

Screen.Recording.2023-06-01.at.3.40.12.PM.mov

After

Screen.Recording.2023-06-02.at.3.24.56.PM.mov

Test plan

QA (ie manual testing)

  • Manually testing on mobile (or web simulator) to make sure that swipe downs properly close the drawer

Automated testing

  • Unit test N/A

@LunrEclipse LunrEclipse requested review from a team and zzmp June 1, 2023 22:48
@vercel
Copy link

vercel bot commented Jun 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2023 8:57pm
interface-node18 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 17, 2023 8:57pm

@cypress
Copy link

cypress bot commented Jun 1, 2023

Passing run #12772 ↗︎

0 89 6 0 Flakiness 0

Details:

feat: MP closes on scroll down
Project: Uniswap Interface Commit: a9aeea98ea
Status: Passed Duration: 04:40 💡
Started: Jul 17, 2023 9:00 PM Ended: Jul 17, 2023 9:04 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@infredible
Copy link
Collaborator

Tested this on my iphone and I'm seeing some issues. The sensitivity seems to be a bit off which causes some jittery behavior. If the app is registering my gesture to pull down the sheet, the sheet should track the movement of my finger.

In the second video, you can also see that sometimes the content in the sheet will get stuck in a scrolled state.
https://github.com/Uniswap/interface/assets/3887562/9b284595-cf0b-4cd4-9ba7-393beae8adf6
https://github.com/Uniswap/interface/assets/3887562/f5ac5fae-b90f-4b0b-b960-87504af22ddd

@codecov
Copy link

codecov bot commented Jun 2, 2023

Codecov Report

Merging #6682 (a9aeea9) into main (95f6148) will increase coverage by 8.74%.
The diff coverage is 16.12%.

Flag Coverage Δ
e2e-tests 61.94% <16.12%> (+4.30%) ⬆️
unit-tests 29.59% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Copy link
Contributor

@zzmp zzmp left a comment

Choose a reason for hiding this comment

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

Looks good! Can you just add comments within the useGesture so it's more apparent what the logic there is doing?

@LunrEclipse LunrEclipse requested review from a team and cartcrom and removed request for a team June 14, 2023 02:51
Copy link
Contributor

@cartcrom cartcrom left a comment

Choose a reason for hiding this comment

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

I'm unfortunately still getting the scrolling/sliding at the same time behaviour I sent to you on slack, on mobile chrome and safari

@LunrEclipse LunrEclipse merged commit 708a813 into main Aug 2, 2023
@LunrEclipse LunrEclipse deleted the brendanwong/web-1801-mweb-swipe-down-to-dismiss-mp branch August 2, 2023 21:15
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.

5 participants