fix(mobile): bottom sheet drag gesture jump#25425
Closed
ByteSizedMarius wants to merge 3 commits intoimmich-app:mainfrom
Closed
fix(mobile): bottom sheet drag gesture jump#25425ByteSizedMarius wants to merge 3 commits intoimmich-app:mainfrom
ByteSizedMarius wants to merge 3 commits intoimmich-app:mainfrom
Conversation
- Add threshold for drag direction to prevent accidental closes from jitter - Only jump to snap extent before close if currently above it - Defer image animation when sheet closes during active drag - Add absolute close extent so slow dragging still closes the sheet - Prevent sheet from growing when dragging past origin point
blockGestures is set for both zoomed scrolling and sheet-close-during-drag, so the animate-to-center was incorrectly triggering on zoomed scrolls. Further testing seemed to show that this animation wasn't needed anymore after I added auto-close on minimumExtend
Contributor
Author
|
I found some strange interactions while working on a different issue and will convert to draft until I figure both out. Should I split this in 2 PRs? – in hindsight, issue 1 is a fix and then the rest is more of a feature/interaction change |
Member
If it does not introduce any new bugs or UX issues, you can split it into two to fasttrack the issue fix |
Member
|
A more elaborate refactor is made in #25952 which also addresses the issues addressed in this PR. Closing this in favour of the linked PR. Thanks a lot for working on this |
Collaborator
|
Yeah I noticed this too. As @shenlong-tanwen mentioned, this is addressed by #25952, and more :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Issue 1: Sheet Jumps Up Before Closing
When opening and closing the sheet in a single gesture (drag up, then back down without releasing), the sheet would jump UP to 67% before animating closed - even if below that point. This only happened in one continuous gesture because when the user releases first, the sheet animates up to 67% (the snap point), so it's already at 67% before any close attempt and the jump isn't visible.
I attached a before/after video in the spoiler. Before is slowed to 1/4 so you can see the jump/flicker better.
Fix: Only jump if currently above 67%:
Issue 2: Accidental Closes
Any 1px downward movement triggered a close when below the 67% threshold - whether from a still position or while dragging. This was especially noticeable when dragging up slowly, because your finger naturally shifts around while touching the screen, and if that shift registered as 1px lower than the current position, it counted as "dragging down" and closed the sheet immediately.
Fix: Add a 2% threshold so only intentional movement counts:
This caused another small issue, as with the threshold in place, slow dragging downwards never exceeded 2% per notification, so
isDraggingDownwas never true. The sheet would shrink but never actually close.Fix for that: Also close when reaching minimum extent (40%), regardless of drag speed:
This makes the interaction feel much better IMO; before, the sheet closing on a 1px jitter didn't feel intentional at all. Now it only closes when you've either made a deliberate swipe or dragged it all the way down, so it always feels intentional
Follow-up Fix: Image Jumps Back When Sheet Closes During Drag
This edgecase only appeared after the first two changes. Before, any 1px movement triggered instant close - the finger could still be on screen, but the close happened so immediately that it didn't matter. After the previous changes, slow dragging became possible: now the bottomsheet only closes after gradually dragging the sheet down to 40% with a finger still on screen. This created a new scenario where the close animation runs while the user still has the finger on the screen.
When this happened, the image (behind the sheet) animated down (back to the center of the screen) then sometimes jumped back up if the user hadn't let go of the drag yet. I believe the
animateMultiplein_handleSheetCloseconflicted with the active drag gesture.Fix: Defer animation until drag ends:
Follow-up Fix: Sheet Grows When Dragging Past Origin
Also only possible after prev. changes; before, the sheet would close immediately when dragging down, so you couldn't drag past the origin.
When dragging up to open the sheet, then back down past the starting point (without releasing), the sheet grew again instead of staying small. Sheet extent was calculated using
position.distancewhich is absolute, so distance increased whether you dragged up OR down past origin.Fix: Only use upward movement (negative Y) for extent:
How Has This Been Tested?
I installed the app on my phone and did all kinds of dragging. Slow, fast, without releasing, with releasing, etc.
Screenshots (if appropriate)
Before:
https://github.com/user-attachments/assets/b5471f3f-9a1f-4a6c-acf8-bc4f04020b96
After:
https://github.com/user-attachments/assets/f3081e58-26eb-4e1e-b340-c468ce27b0e0
Checklist:
src/services/uses repositories implementations for database calls, filesystem operations, etc.src/repositories/is pretty basic/simple and does not have any immich specific logic (that belongs insrc/services/)Please describe to which degree, if any, an LLM was used in creating this pull request.
Claude was used during debugging the mentioned behaviors