Make "double tap to zoom" precise across platforms (#1482)

* Implement double tap for Android

* Match the new behavior on iOS

src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx

@@ -32,6 +32,7 @@ const SWIPE_CLOSE_VELOCITY = 1
 const SCREEN = Dimensions.get('screen')
 const SCREEN_WIDTH = SCREEN.width
 const SCREEN_HEIGHT = SCREEN.height
+const MAX_SCALE = 2
 
 type Props = {
   imageSrc: ImageSource
@@ -58,13 +59,18 @@ const ImageItem = ({
   const [loaded, setLoaded] = useState(false)
   const [scaled, setScaled] = useState(false)
   const imageDimensions = useImageDimensions(imageSrc)
-  const handleDoubleTap = useDoubleTapToZoom(scrollViewRef, scaled, SCREEN)
+  const handleDoubleTap = useDoubleTapToZoom(
+    scrollViewRef,
+    scaled,
+    SCREEN,
+    imageDimensions,
+  )
 
   const [translate, scale] = getImageTransform(imageDimensions, SCREEN)
   const scrollValueY = new Animated.Value(0)
   const scaleValue = new Animated.Value(scale || 1)
   const translateValue = new Animated.ValueXY(translate)
-  const maxScale = scale && scale > 0 ? Math.max(1 / scale, 1) : 1
+  const maxScrollViewZoom = MAX_SCALE / (scale || 1)
 
   const imageOpacity = scrollValueY.interpolate({
     inputRange: [-SWIPE_CLOSE_OFFSET, 0, SWIPE_CLOSE_OFFSET],
@@ -118,7 +124,7 @@ const ImageItem = ({
         pinchGestureEnabled
         showsHorizontalScrollIndicator={false}
         showsVerticalScrollIndicator={false}
-        maximumZoomScale={maxScale}
+        maximumZoomScale={maxScrollViewZoom}
         contentContainerStyle={styles.imageScrollContainer}
         scrollEnabled={swipeToCloseEnabled}
         onScrollEndDrag={onScrollEndDrag}

src/view/com/lightbox/ImageViewing/hooks/useDoubleTapToZoom.ts

@@ -12,6 +12,8 @@ import {ScrollView, NativeTouchEvent, NativeSyntheticEvent} from 'react-native'
 import {Dimensions} from '../@types'
 
 const DOUBLE_TAP_DELAY = 300
+const MIN_ZOOM = 2
+
 let lastTapTS: number | null = null
 
 /**
@@ -22,41 +24,124 @@ function useDoubleTapToZoom(
   scrollViewRef: React.RefObject<ScrollView>,
   scaled: boolean,
   screen: Dimensions,
+  imageDimensions: Dimensions | null,
 ) {
   const handleDoubleTap = useCallback(
     (event: NativeSyntheticEvent<NativeTouchEvent>) => {
       const nowTS = new Date().getTime()
       const scrollResponderRef = scrollViewRef?.current?.getScrollResponder()
 
-      if (lastTapTS && nowTS - lastTapTS < DOUBLE_TAP_DELAY) {
+      const getZoomRectAfterDoubleTap = (
-        const {pageX, pageY} = event.nativeEvent
+        touchX: number,
-        let targetX = 0
+        touchY: number,
-        let targetY = 0
+      ): {
-        let targetWidth = screen.width
+        x: number
-        let targetHeight = screen.height
+        y: number
+        width: number
+        height: number
+      } => {
+        if (!imageDimensions) {
+          return {
+            x: 0,
+            y: 0,
+            width: screen.width,
+            height: screen.height,
+          }
+        }
 
-        // Zooming in
+        // First, let's figure out how much we want to zoom in.
-        // TODO: Add more precise calculation of targetX, targetY based on touch
+        // We want to try to zoom in at least close enough to get rid of black bars.
-        if (!scaled) {
+        const imageAspect = imageDimensions.width / imageDimensions.height
-          targetX = pageX / 2
+        const screenAspect = screen.width / screen.height
-          targetY = pageY / 2
+        const zoom = Math.max(
-          targetWidth = screen.width / 2
+          imageAspect / screenAspect,
-          targetHeight = screen.height / 2
+          screenAspect / imageAspect,
+          MIN_ZOOM,
+        )
+        // Unlike in the Android version, we don't constrain the *max* zoom level here.
+        // Instead, this is done in the ScrollView props so that it constraints pinch too.
+
+        // Next, we'll be calculating the rectangle to "zoom into" in screen coordinates.
+        // We already know the zoom level, so this gives us the rectangle size.
+        let rectWidth = screen.width / zoom
+        let rectHeight = screen.height / zoom
+
+        // Before we settle on the zoomed rect, figure out the safe area it has to be inside.
+        // We don't want to introduce new black bars or make existing black bars unbalanced.
+        let minX = 0
+        let minY = 0
+        let maxX = screen.width - rectWidth
+        let maxY = screen.height - rectHeight
+        if (imageAspect >= screenAspect) {
+          // The image has horizontal black bars. Exclude them from the safe area.
+          const renderedHeight = screen.width / imageAspect
+          const horizontalBarHeight = (screen.height - renderedHeight) / 2
+          minY += horizontalBarHeight
+          maxY -= horizontalBarHeight
+        } else {
+          // The image has vertical black bars. Exclude them from the safe area.
+          const renderedWidth = screen.height * imageAspect
+          const verticalBarWidth = (screen.width - renderedWidth) / 2
+          minX += verticalBarWidth
+          maxX -= verticalBarWidth
+        }
+
+        // Finally, we can position the rect according to its size and the safe area.
+        let rectX
+        if (maxX >= minX) {
+          // Content fills the screen horizontally so we have horizontal wiggle room.
+          // Try to keep the tapped point under the finger after zoom.
+          rectX = touchX - touchX / zoom
+          rectX = Math.min(rectX, maxX)
+          rectX = Math.max(rectX, minX)
+        } else {
+          // Keep the rect centered on the screen so that black bars are balanced.
+          rectX = screen.width / 2 - rectWidth / 2
+        }
+        let rectY
+        if (maxY >= minY) {
+          // Content fills the screen vertically so we have vertical wiggle room.
+          // Try to keep the tapped point under the finger after zoom.
+          rectY = touchY - touchY / zoom
+          rectY = Math.min(rectY, maxY)
+          rectY = Math.max(rectY, minY)
+        } else {
+          // Keep the rect centered on the screen so that black bars are balanced.
+          rectY = screen.height / 2 - rectHeight / 2
+        }
+
+        return {
+          x: rectX,
+          y: rectY,
+          height: rectHeight,
+          width: rectWidth,
+        }
+      }
+
+      if (lastTapTS && nowTS - lastTapTS < DOUBLE_TAP_DELAY) {
+        let nextZoomRect = {
+          x: 0,
+          y: 0,
+          width: screen.width,
+          height: screen.height,
+        }
+
+        const willZoom = !scaled
+        if (willZoom) {
+          const {pageX, pageY} = event.nativeEvent
+          nextZoomRect = getZoomRectAfterDoubleTap(pageX, pageY)
         }
 
         // @ts-ignore
         scrollResponderRef?.scrollResponderZoomTo({
-          x: targetX,
+          ...nextZoomRect, // This rect is in screen coordinates
-          y: targetY,
-          width: targetWidth,
-          height: targetHeight,
           animated: true,
         })
       } else {
         lastTapTS = nowTS
       }
     },
-    [scaled, screen.height, screen.width, scrollViewRef],
+    [imageDimensions, scaled, screen.height, screen.width, scrollViewRef],
   )
 
   return handleDoubleTap

src/view/com/lightbox/ImageViewing/hooks/usePanResponder.ts

@@ -29,8 +29,10 @@ const SCREEN = Dimensions.get('window')
 const SCREEN_WIDTH = SCREEN.width
 const SCREEN_HEIGHT = SCREEN.height
 const MIN_DIMENSION = Math.min(SCREEN_WIDTH, SCREEN_HEIGHT)
+const ANDROID_BAR_HEIGHT = 24
 
-const SCALE_MAX = 2
+const MIN_ZOOM = 2
+const MAX_SCALE = 2
 const DOUBLE_TAP_DELAY = 300
 const OUT_BOUND_MULTIPLIER = 0.75
 
@@ -87,23 +89,56 @@ const usePanResponder = ({
     return [top, left, bottom, right]
   }
 
-  const getTranslateInBounds = (translate: Position, scale: number) => {
+  const getTransformAfterDoubleTap = (
-    const inBoundTranslate = {x: translate.x, y: translate.y}
+    touchX: number,
-    const [topBound, leftBound, bottomBound, rightBound] = getBounds(scale)
+    touchY: number,
+  ): [number, Position] => {
+    let nextScale = initialScale
+    let nextTranslateX = initialTranslate.x
+    let nextTranslateY = initialTranslate.y
 
-    if (translate.x > leftBound) {
+    // First, let's figure out how much we want to zoom in.
-      inBoundTranslate.x = leftBound
+    // We want to try to zoom in at least close enough to get rid of black bars.
-    } else if (translate.x < rightBound) {
+    const imageAspect = imageDimensions.width / imageDimensions.height
-      inBoundTranslate.x = rightBound
+    const screenAspect = SCREEN.width / SCREEN.height
+    let zoom = Math.max(
+      imageAspect / screenAspect,
+      screenAspect / imageAspect,
+      MIN_ZOOM,
+    )
+    // Don't zoom so hard that the original image's pixels become blurry.
+    zoom = Math.min(zoom, MAX_SCALE / initialScale)
+    nextScale = initialScale * zoom
+
+    // Next, let's see if we need to adjust the scaled image translation.
+    // Ideally, we want the tapped point to stay under the finger after the scaling.
+    const dx = SCREEN.width / 2 - touchX
+    const dy = SCREEN.height / 2 - (touchY - ANDROID_BAR_HEIGHT)
+    // Before we try to adjust the translation, check how much wiggle room we have.
+    // We don't want to introduce new black bars or make existing black bars unbalanced.
+    const [topBound, leftBound, bottomBound, rightBound] = getBounds(nextScale)
+    if (leftBound > rightBound) {
+      // Content fills the screen horizontally so we have horizontal wiggle room.
+      // Try to keep the tapped point under the finger after zoom.
+      nextTranslateX += dx * zoom - dx
+      nextTranslateX = Math.min(nextTranslateX, leftBound)
+      nextTranslateX = Math.max(nextTranslateX, rightBound)
+    }
+    if (topBound > bottomBound) {
+      // Content fills the screen vertically so we have vertical wiggle room.
+      // Try to keep the tapped point under the finger after zoom.
+      nextTranslateY += dy * zoom - dy
+      nextTranslateY = Math.min(nextTranslateY, topBound)
+      nextTranslateY = Math.max(nextTranslateY, bottomBound)
     }
 
-    if (translate.y > topBound) {
+    return [
-      inBoundTranslate.y = topBound
+      nextScale,
-    } else if (translate.y < bottomBound) {
+      {
-      inBoundTranslate.y = bottomBound
+        x: nextTranslateX,
-    }
+        y: nextTranslateY,
-
+      },
-    return inBoundTranslate
+    ]
   }
 
   const fitsScreenByWidth = () =>
@@ -157,25 +192,18 @@ const usePanResponder = ({
       )
 
       if (doubleTapToZoomEnabled && isDoubleTapPerformed) {
-        const isScaled = currentTranslate.x !== initialTranslate.x // currentScale !== initialScale;
+        let nextScale = initialScale
-        const {pageX: touchX, pageY: touchY} = event.nativeEvent.touches[0]
+        let nextTranslate = initialTranslate
-        const targetScale = SCALE_MAX
-        const nextScale = isScaled ? initialScale : targetScale
-        const nextTranslate = isScaled
-          ? initialTranslate
-          : getTranslateInBounds(
-              {
-                x:
-                  initialTranslate.x +
-                  (SCREEN_WIDTH / 2 - touchX) * (targetScale / currentScale),
-                y:
-                  initialTranslate.y +
-                  (SCREEN_HEIGHT / 2 - touchY) * (targetScale / currentScale),
-              },
-              targetScale,
-            )
 
-        onZoom(!isScaled)
+        const willZoom = currentScale === initialScale
+        if (willZoom) {
+          const {pageX: touchX, pageY: touchY} = event.nativeEvent.touches[0]
+          ;[nextScale, nextTranslate] = getTransformAfterDoubleTap(
+            touchX,
+            touchY,
+          )
+        }
+        onZoom(willZoom)
 
         Animated.parallel(
           [
@@ -336,8 +364,8 @@ const usePanResponder = ({
       }
 
       if (tmpScale > 0) {
-        if (tmpScale < initialScale || tmpScale > SCALE_MAX) {
+        if (tmpScale < initialScale || tmpScale > MAX_SCALE) {
-          tmpScale = tmpScale < initialScale ? initialScale : SCALE_MAX
+          tmpScale = tmpScale < initialScale ? initialScale : MAX_SCALE
           Animated.timing(scaleValue, {
             toValue: tmpScale,
             duration: 100,