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

Fixes to animated lightbox #6088

Closed
wants to merge 11 commits into from
Closed

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Nov 3, 2024

Stacked on #6047, #6073, #6048, and #6081


Known issues

  • Slow in prod on low-end Android
    • I have a few ideas to try, otherwise might have to disable it on Android
  • Incorrect positioning on Android device
    • Doesn’t happen on emulator.
  • Flash of borders on the thumbnail after dismiss via swipe
  • Extra high aspect is behaving weirdly https://bsky.app/profile/schlagteslinks.bsky.social/post/3l7y4l6yur72e
  • I’ve had it lock up on iOS

Fixes for known issues in the preceding stack. It would've been a pain to rebase and I had to be very careful not to break other things. This seems to be an equilibrium of features working but it's quite fragile. See commit by commit.

  • Fixes swipe-up gesture stopping working if you spam it too fast during a pager slide on iOS.
  • Fixes being unable to drag or perform gestures with finger(s) on the black bars on Android.
  • Fixes unintended ability to paginate while in the middle of an opening or closing animation.
  • A single tap in zoomed state now always zooms out and shows controls. It was too hard to make the close animation (if you pressed close after bringing up controls in a zoomed state) work well. Now it just pops you back before showing controls. Twitter does the same.
  • Cropped images no longer flash their cropped edges on the first animation frame. Now they continuously expand. This needs device testing because it definitely affects perf a bit. Mostly relevant for multi-image layouts.
  • The avatar first frame is now precisely positioned.

Test Plan

All of the above. Plus the usual. Try portraits/landscapes in different layouts (1, 2, 3, 4). Slow down if needed to verify animation.

ok.mov
okay.mov

Copy link

github-actions bot commented Nov 3, 2024

Old size New size Diff
8.12 MB 8.12 MB 136 B (0.00%)

Reanimated values and React props were out of sync. Prepare each image's styles separately to avoid that.
Also fix handling of missing aspectRatio
@gaearon gaearon force-pushed the images-followup-alt branch from 42d917e to d8eaf8b Compare November 3, 2024 05:11
@gaearon gaearon marked this pull request as draft November 7, 2024 01:11
@gaearon
Copy link
Collaborator Author

gaearon commented Nov 8, 2024

#6159

@gaearon gaearon closed this Nov 8, 2024
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.

1 participant