Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mobile): auto close memories on scrolling beyond first / last memory #3476

Merged
merged 2 commits into from
Jul 31, 2023
Merged

feat(mobile): auto close memories on scrolling beyond first / last memory #3476

merged 2 commits into from
Jul 31, 2023

Conversation

shenlong-tanwen
Copy link
Member

Changes made

  • Scroll Physics for Android in Memory lane is made similar to the one used in iOS. It looks visually better and also fixes the jarring animation when a user tries to scroll past the last asset / memory.
  • When a user tries to scroll above the first memory or scroll past the last memory, the memory lane is closed and the user is brought back to the timeline view. The same behavior is used if the user tries to scroll to the previous asset from the very first asset of the first memory or if a user tries to scroll past the last asset of the last memory
Before After
Before-C.mp4
After-C.mp4

How Has This Been Tested?

  • Made sure that the scrolling physics is used for both horizontal and vertical scrolls in Single asset memory as well as Multi asset memories
  • Made sure that the user is brought back to the timeline view only when scrolling past the first and last memory. Scrolling in all the other memories takes the user to the previous or next asset accordingly

@vercel
Copy link

vercel bot commented Jul 30, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Jul 31, 2023 3:40am

@shenlong-tanwen
Copy link
Member Author

Returning back to the timeline if a user goes beyond the last assest of the last memory seems normal. I'm not entirely sure if having the Vertical OverScroll handling for popping out of Memory lane if a user scrolls above or below the first / last memory is intuitive. Seems normal to me, but others might feel different.

Returning back to timeline if a user scrolls to the left from the first asset of the first memory might also seem weird to few.

Should we keep all four handling. i.e, Returning back to timeline if a user tries one of the following:

  • Scrolls to the left of first asset of the first memory
  • Scrolls above the first memory
  • Scrolls beyond the last memory
  • Scrolls to the right of the last asset of the last memory

Or should we only keep the last one and remove the handling for the other 3 scenarios?

@alextran1502
Copy link
Contributor

@shalong-tanwen Hey man, thank you for your high-quality contribution. I think we should only return to the timeline in two scenarios.

  1. When the user clicks the exit button on the top left corner of any memories
  2. When the user reach the last memory of the last memory lane

What do you think?

And by the way, do you have a Discord handler?

@shenlong-tanwen
Copy link
Member Author

shenlong-tanwen commented Jul 31, 2023

What do you think?

Yeah that might be the most natural behavior and the only one most users would've expected. I've changed it now to handle only OverScroll on the last asset of the last memory

And by the way, do you have a Discord handler?

Yes, I do. You can contact me @ _osorin

@alextran1502 alextran1502 merged commit e0a3e5a into immich-app:main Jul 31, 2023
19 checks passed
@shenlong-tanwen shenlong-tanwen deleted the feat/mobile-memory-auto-close branch August 2, 2023 13:27
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.

2 participants