-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Make "double tap to zoom" precise across platforms #1482
Conversation
Thanks for fixing the issue I opened ! The zoom seems nice in your videos :) |
thank you for reporting :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is, indeed, no joke. I'm going to approve & merge so I can include it in tonight's testflight. The code at a glance looks fine; all that remains is testing on real devices, which I'll do shortly
* origin: Allow touch at the top of the lightbox (#1489) Bump ios build number Feeds tab fixes (#1486) Nicer 'post processing status' in the composer (#1472) Inline createPanResponder (#1483) Tree view threads experiment (#1480) Make "double tap to zoom" precise across platforms (#1482) Onboarding recommended follows (#1457) Add thread sort settings (#1475)
const willZoom = !scaled | ||
if (willZoom) { | ||
const {pageX, pageY} = event.nativeEvent | ||
nextZoomRect = getZoomRectAfterDoubleTap(pageX, pageY) | ||
} | ||
|
||
// @ts-ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just being curious here, what warning is this silencing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably scrollResponderZoomTo
not being in the type definitions?
Apply the fix from bluesky-social/social-app#1482
The problem
On Android, we were zooming in way too hard (#1275). I tried to fix it by zooming in "less", but I wasn't sure what scale and how to calculate the new position in a way that would feel intuitive. I studied what different apps do using a variety of different image sizes (test thread) and settled on the following principles that are mostly inspired by the iOS Photos app:
The new behavior is consistent between Android and iOS.
How the code works
Our lightbox appears to be based on a vendored version of https://github.com/jobtoday/react-native-image-viewing. The "double tap to zoom" gesture is implemented separately for Android and iOS:
The Android version is implemented in
ImageItem.android.ts
andusePanResponder.ts
. It scales and transforms the original image. When you pinch or double tap,usePanResponder
calculates the new scale and transform values so that the image zooms in.The iOS version is in
ImageItem.ios.ts
anduseDoubleTapToZoom.ts
. It also scales and transforms the original image. However, that scale and transform are never updated. Instead, it uses iOSUIScrollView
for pinching and double-tap. When you double tap,useDoubleTapToZoom
calculates the new "zoom rect" and asks the scroll view to zoom into it.I kept this divergence in the implementation details, so the code looks a bit duplicated (but not quite).
I prefer this piece heavily commented but I'm down to trim it down if it feels like much.
Before / After
Landscape aspect ratio
The original behavior on Android zooms in way too much.
The original behavior on iOS is decent but doesn't use the screen estate very well (black bars).
dril_before.mov
The new behavior consistently fills the screen:
dril_after.mov
Square aspect ratio
Before:
paul_before.mov
After:
paul_after.mov
Portrait aspect ratio
Before (notice the black bar on iOS when zooming in near the top):
kiki_before.mov
After (notice how there's no black bar on iOS when zooming in near the top):
kiki_after.mov
Edge cases
Not recording these, but https://bsky.app/profile/danabra.mov/post/3k7h5zss7bp2g is a good thread for testing.
Interesting edge cases include very tall images, very wide images, squares, high res and low res images.