From 2618686677bab9286e308f145d49572193cf00b7 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 30 Aug 2026 09:15:20 +0200 Subject: [PATCH] fix(app): stabilize image lightbox gestures (#4064) Android reports one terminal pinch update with a stale two-pointer count after a finger has lifted. Track the real touch lifecycle so that shifted focal point cannot move the image on release.\n\nKeep initial presentation padding separate from the full zoom canvas, and make double-tap reset and outside-image dismissal consistent across native and web. --- .../browser/agent-tab-image-stability.spec.ts | 18 ++++- .../src/components/attachment-lightbox.tsx | 7 +- .../zoomable-viewport/geometry.test.ts | 44 ++++++++++++ .../components/zoomable-viewport/geometry.ts | 47 ++++++++++++- .../components/zoomable-viewport/image.tsx | 8 ++- .../components/zoomable-viewport/index.tsx | 68 +++++++++++++++++-- .../zoomable-viewport/index.web.tsx | 35 +++++++++- .../src/components/zoomable-viewport/types.ts | 4 +- 8 files changed, 212 insertions(+), 19 deletions(-) diff --git a/packages/app/e2e/browser/agent-tab-image-stability.spec.ts b/packages/app/e2e/browser/agent-tab-image-stability.spec.ts index f03bd66f1e..320e52ba28 100644 --- a/packages/app/e2e/browser/agent-tab-image-stability.spec.ts +++ b/packages/app/e2e/browser/agent-tab-image-stability.spec.ts @@ -66,13 +66,25 @@ test("opens a timeline image in a zoomable lightbox", async ({ await canvas.hover(); const transformedContent = canvas.locator(":scope > div").first(); const initialBox = await transformedContent.boundingBox(); + const canvasBox = await canvas.boundingBox(); expect(initialBox).not.toBeNull(); - await page.getByRole("button", { name: "Zoom in", exact: true }).click(); + expect(canvasBox).not.toBeNull(); + expect(initialBox!.width).toBeLessThan(canvasBox!.width); + const zoomIn = page.getByRole("button", { name: "Zoom in", exact: true }); + await zoomIn.click(); + await zoomIn.click(); + await zoomIn.click(); + await zoomIn.click(); await expect .poll(async () => (await transformedContent.boundingBox())?.width ?? 0) - .toBeGreaterThan(initialBox!.width * 1.2); + .toBeGreaterThan(canvasBox!.width); - await page.getByTestId("attachment-lightbox-backdrop").click({ position: { x: 4, y: 4 } }); + await canvas.dblclick({ position: { x: canvasBox!.width / 2, y: canvasBox!.height / 2 } }); + await expect + .poll(async () => Math.round((await transformedContent.boundingBox())?.width ?? 0)) + .toBe(Math.round(initialBox!.width)); + + await canvas.click({ position: { x: 4, y: 4 } }); await expect(lightboxImage).toHaveCount(0); }); diff --git a/packages/app/src/components/attachment-lightbox.tsx b/packages/app/src/components/attachment-lightbox.tsx index 5b00fa82cd..ff99c65f6d 100644 --- a/packages/app/src/components/attachment-lightbox.tsx +++ b/packages/app/src/components/attachment-lightbox.tsx @@ -8,6 +8,7 @@ import { useTranslation } from "react-i18next"; import type { AttachmentMetadata } from "@/attachments/types"; import { useAttachmentPreviewUrl } from "@/attachments/use-attachment-preview-url"; import { isNative, isWeb } from "@/constants/platform"; +import { SPACING } from "@/styles/theme"; import { WindowChromeRootRegion } from "@/utils/desktop-window"; import { ZoomableImage } from "@/components/zoomable-viewport/image"; import type { ViewportSize } from "@/components/zoomable-viewport/geometry"; @@ -22,6 +23,7 @@ interface AttachmentLightboxProps { } const ModalRoot = isNative ? GestureHandlerRootView : View; +const LIGHTBOX_FIT = { padding: SPACING[4], maxWidth: 960, maxHeight: 640 }; export function AttachmentLightbox({ source, onClose }: AttachmentLightboxProps) { const { t } = useTranslation(); @@ -102,7 +104,9 @@ export function AttachmentLightbox({ source, onClose }: AttachmentLightboxProps) accessibilityLabel={t("composer.attachments.openImage")} actions={actions} contentSize={contentSize} + fit={LIGHTBOX_FIT} onError={handleImageError} + onPressOutsideContent={onClose} style={styles.imageViewport} testID="attachment-lightbox" uri={url} @@ -142,14 +146,11 @@ const styles = StyleSheet.create((theme) => ({ flex: 1, alignItems: "center", justifyContent: "center", - padding: theme.spacing[4], }, imageViewport: { flex: 1, width: "100%", alignSelf: "center", - maxWidth: 960, - maxHeight: 640, }, errorText: { color: theme.colors.foregroundMuted, diff --git a/packages/app/src/components/zoomable-viewport/geometry.test.ts b/packages/app/src/components/zoomable-viewport/geometry.test.ts index 11a810e5ca..68f4c1286e 100644 --- a/packages/app/src/components/zoomable-viewport/geometry.test.ts +++ b/packages/app/src/components/zoomable-viewport/geometry.test.ts @@ -3,6 +3,8 @@ import { FIT_TRANSFORM, clampTransform, fitContentSize, + isActivePinchUpdate, + isPointInsideTransformedContent, panContent, zoomContentAtPoint, } from "./geometry"; @@ -11,6 +13,11 @@ const viewport = { width: 800, height: 600 }; const fittedContent = { width: 800, height: 400 }; describe("zoomable viewport geometry", () => { + it("rejects Android's terminal pinch update after a touch has lifted", () => { + expect(isActivePinchUpdate(1, 2)).toBe(false); + expect(isActivePinchUpdate(2, 2)).toBe(true); + }); + it("fits content inside the viewport without changing its aspect ratio", () => { expect(fitContentSize({ width: 1600, height: 800 }, viewport)).toEqual(fittedContent); expect(fitContentSize({ width: 400, height: 800 }, viewport)).toEqual({ @@ -19,6 +26,43 @@ describe("zoomable viewport geometry", () => { }); }); + it("fits within presentation padding and caps without shrinking the zoom viewport", () => { + expect( + fitContentSize({ width: 1600, height: 800 }, viewport, { + padding: 20, + maxWidth: 600, + maxHeight: 500, + }), + ).toEqual({ width: 600, height: 300 }); + }); + + it("distinguishes backdrop taps from taps on transformed content", () => { + expect( + isPointInsideTransformedContent({ + point: { x: 400, y: 300 }, + transform: FIT_TRANSFORM, + fittedContent, + viewport, + }), + ).toBe(true); + expect( + isPointInsideTransformedContent({ + point: { x: 400, y: 50 }, + transform: FIT_TRANSFORM, + fittedContent, + viewport, + }), + ).toBe(false); + expect( + isPointInsideTransformedContent({ + point: { x: 10, y: 10 }, + transform: { scale: 2, x: 0, y: 0 }, + fittedContent, + viewport, + }), + ).toBe(true); + }); + it("returns to the centered fit transform at the minimum scale", () => { expect( clampTransform({ scale: 0.5, x: 200, y: -200 }, fittedContent, viewport, { diff --git a/packages/app/src/components/zoomable-viewport/geometry.ts b/packages/app/src/components/zoomable-viewport/geometry.ts index fa46873073..6bbfb410af 100644 --- a/packages/app/src/components/zoomable-viewport/geometry.ts +++ b/packages/app/src/components/zoomable-viewport/geometry.ts @@ -19,10 +19,53 @@ export interface ViewportScaleLimits { maxScale: number; } +export interface ViewportFitOptions { + padding?: number; + maxWidth?: number; + maxHeight?: number; +} + export const FIT_TRANSFORM: ViewportTransform = { scale: 1, x: 0, y: 0 }; -export function fitContentSize(content: ViewportSize, viewport: ViewportSize): ViewportSize { - const fitScale = Math.min(viewport.width / content.width, viewport.height / content.height); +export function isActivePinchUpdate(activeTouches: number, reportedPointers: number): boolean { + "worklet"; + return activeTouches >= 2 && reportedPointers >= 2; +} + +export function isPointInsideTransformedContent(input: { + point: ViewportPoint; + transform: ViewportTransform; + fittedContent: ViewportSize; + viewport: ViewportSize; +}): boolean { + "worklet"; + const centerX = input.viewport.width / 2 + input.transform.x; + const centerY = input.viewport.height / 2 + input.transform.y; + const halfWidth = (input.fittedContent.width * input.transform.scale) / 2; + const halfHeight = (input.fittedContent.height * input.transform.scale) / 2; + return ( + input.point.x >= centerX - halfWidth && + input.point.x <= centerX + halfWidth && + input.point.y >= centerY - halfHeight && + input.point.y <= centerY + halfHeight + ); +} + +export function fitContentSize( + content: ViewportSize, + viewport: ViewportSize, + options: ViewportFitOptions = {}, +): ViewportSize { + const padding = options.padding ?? 0; + const availableWidth = Math.max( + 0, + Math.min(viewport.width - padding * 2, options.maxWidth ?? Number.POSITIVE_INFINITY), + ); + const availableHeight = Math.max( + 0, + Math.min(viewport.height - padding * 2, options.maxHeight ?? Number.POSITIVE_INFINITY), + ); + const fitScale = Math.min(availableWidth / content.width, availableHeight / content.height); return { width: content.width * fitScale, height: content.height * fitScale, diff --git a/packages/app/src/components/zoomable-viewport/image.tsx b/packages/app/src/components/zoomable-viewport/image.tsx index e1597deb57..3c59eee4bf 100644 --- a/packages/app/src/components/zoomable-viewport/image.tsx +++ b/packages/app/src/components/zoomable-viewport/image.tsx @@ -1,7 +1,7 @@ import { useEffect, useMemo, useState } from "react"; import { Image, View, type StyleProp, type ViewStyle } from "react-native"; import { StyleSheet } from "react-native-unistyles"; -import type { ViewportSize } from "./geometry"; +import type { ViewportFitOptions, ViewportSize } from "./geometry"; import { ZoomableViewport } from "./index"; import type { ZoomableViewportAction } from "./types"; @@ -10,9 +10,11 @@ interface ZoomableImageProps { accessibilityLabel?: string; actions?: ZoomableViewportAction[]; contentSize?: ViewportSize; + fit?: ViewportFitOptions; maxScale?: number; minScale?: number; onError?: () => void; + onPressOutsideContent?: () => void; style?: StyleProp; testID?: string; wheelActivation?: "always" | "modifier"; @@ -23,9 +25,11 @@ export function ZoomableImage({ accessibilityLabel, actions, contentSize, + fit, maxScale, minScale = 1, onError, + onPressOutsideContent, style, testID = "zoomable-image", wheelActivation = "always", @@ -73,8 +77,10 @@ export function ZoomableImage({ accessibilityLabel={accessibilityLabel} actions={actions} contentSize={resolvedSize} + fit={fit} maxScale={maxScale} minScale={minScale} + onPressOutsideContent={onPressOutsideContent} style={style} testID={testID} wheelActivation={wheelActivation} diff --git a/packages/app/src/components/zoomable-viewport/index.tsx b/packages/app/src/components/zoomable-viewport/index.tsx index 946d266d18..fd0c9d939a 100644 --- a/packages/app/src/components/zoomable-viewport/index.tsx +++ b/packages/app/src/components/zoomable-viewport/index.tsx @@ -3,7 +3,14 @@ import { View, type LayoutChangeEvent } from "react-native"; import { Gesture, GestureDetector } from "react-native-gesture-handler"; import Animated, { runOnJS, useAnimatedStyle, useSharedValue } from "react-native-reanimated"; import { StyleSheet } from "react-native-unistyles"; -import { FIT_TRANSFORM, fitContentSize, zoomContentAtPoint, type ViewportSize } from "./geometry"; +import { + FIT_TRANSFORM, + fitContentSize, + isActivePinchUpdate, + isPointInsideTransformedContent, + zoomContentAtPoint, + type ViewportSize, +} from "./geometry"; import { ViewportToolbar } from "./toolbar"; import type { ZoomableViewportProps } from "./types"; @@ -15,8 +22,10 @@ export function ZoomableViewport({ children, actions = [], accessibilityLabel, + fit, maxScale = DEFAULT_MAX_SCALE, minScale = DEFAULT_MIN_SCALE, + onPressOutsideContent, style, testID, }: ZoomableViewportProps) { @@ -31,9 +40,10 @@ export function ZoomableViewport({ const pinchFocalX = useSharedValue(0); const pinchFocalY = useSharedValue(0); const pinchReady = useSharedValue(false); + const pinchTouchCount = useSharedValue(0); const fittedContent = useMemo( - () => (viewport ? fitContentSize(contentSize, viewport) : null), - [contentSize, viewport], + () => (viewport ? fitContentSize(contentSize, viewport, fit) : null), + [contentSize, fit, viewport], ); const reset = useCallback(() => { @@ -66,7 +76,14 @@ export function ZoomableViewport({ const pinchGesture = useMemo( () => Gesture.Pinch() - .onBegin(() => { + .onTouchesDown((event) => { + pinchTouchCount.value = event.numberOfTouches; + }) + .onTouchesUp((event) => { + pinchTouchCount.value = event.numberOfTouches; + }) + .onBegin((event) => { + pinchTouchCount.value = event.numberOfPointers; startScale.value = scale.value; startX.value = translateX.value; startY.value = translateY.value; @@ -76,7 +93,7 @@ export function ZoomableViewport({ if (!fittedContent || !viewport) return; // Android emits one final pinch update after a finger lifts. Its focal point belongs to // the remaining pointer, so applying it makes the content jump as the gesture ends. - if (event.numberOfPointers < 2) return; + if (!isActivePinchUpdate(pinchTouchCount.value, event.numberOfPointers)) return; if (!pinchReady.value) { pinchFocalX.value = event.focalX; pinchFocalY.value = event.focalY; @@ -95,7 +112,10 @@ export function ZoomableViewport({ translateX.value = Math.min(maxX, Math.max(-maxX, nextX)); translateY.value = Math.min(maxY, Math.max(-maxY, nextY)); }) - .onFinalize(() => runOnJS(setToolbarScale)(scale.value)), + .onFinalize(() => { + pinchTouchCount.value = 0; + runOnJS(setToolbarScale)(scale.value); + }), [ fittedContent, maxScale, @@ -103,6 +123,7 @@ export function ZoomableViewport({ pinchFocalX, pinchFocalY, pinchReady, + pinchTouchCount, scale, startScale, startX, @@ -112,10 +133,43 @@ export function ZoomableViewport({ viewport, ], ); - const gesture = useMemo( + const doubleTapGesture = useMemo( + () => + Gesture.Tap() + .numberOfTaps(2) + .onEnd((_event, success) => { + if (success) runOnJS(reset)(); + }), + [reset], + ); + const backdropTapGesture = useMemo( + () => + Gesture.Tap() + .numberOfTaps(1) + .onEnd((event, success) => { + if (!success || !fittedContent || !viewport || !onPressOutsideContent) return; + const isInside = isPointInsideTransformedContent({ + point: { x: event.x, y: event.y }, + transform: { scale: scale.value, x: translateX.value, y: translateY.value }, + fittedContent, + viewport, + }); + if (!isInside) runOnJS(onPressOutsideContent)(); + }), + [fittedContent, onPressOutsideContent, scale, translateX, translateY, viewport], + ); + const tapGesture = useMemo( + () => Gesture.Exclusive(doubleTapGesture, backdropTapGesture), + [backdropTapGesture, doubleTapGesture], + ); + const transformGesture = useMemo( () => Gesture.Simultaneous(panGesture, pinchGesture), [panGesture, pinchGesture], ); + const gesture = useMemo( + () => Gesture.Exclusive(transformGesture, tapGesture), + [tapGesture, transformGesture], + ); const zoomFromCenter = useCallback( (factor: number) => { diff --git a/packages/app/src/components/zoomable-viewport/index.web.tsx b/packages/app/src/components/zoomable-viewport/index.web.tsx index c77b24691a..69f4bef871 100644 --- a/packages/app/src/components/zoomable-viewport/index.web.tsx +++ b/packages/app/src/components/zoomable-viewport/index.web.tsx @@ -4,6 +4,7 @@ import { StyleSheet } from "react-native-unistyles"; import { FIT_TRANSFORM, fitContentSize, + isPointInsideTransformedContent, panContent, zoomContentAtPoint, type ViewportPoint, @@ -47,8 +48,10 @@ export function ZoomableViewport({ children, actions = [], accessibilityLabel, + fit, maxScale = DEFAULT_MAX_SCALE, minScale = DEFAULT_MIN_SCALE, + onPressOutsideContent, style, testID, wheelActivation = "modifier", @@ -56,6 +59,7 @@ export function ZoomableViewport({ const canvasRef = useRef(null); const pointersRef = useRef(new Map()); const gestureRef = useRef(null); + const suppressClickRef = useRef(false); const transformRef = useRef(FIT_TRANSFORM); const [viewport, setViewport] = useState(null); const [transform, setTransform] = useState(FIT_TRANSFORM); @@ -64,8 +68,8 @@ export function ZoomableViewport({ const [isFocusWithin, setIsFocusWithin] = useState(false); const [hasTouchInput, setHasTouchInput] = useState(false); const fittedContent = useMemo( - () => (viewport ? fitContentSize(contentSize, viewport) : null), - [contentSize, viewport], + () => (viewport ? fitContentSize(contentSize, viewport, fit) : null), + [contentSize, fit, viewport], ); const limits = useMemo(() => ({ minScale, maxScale }), [maxScale, minScale]); @@ -139,6 +143,7 @@ export function ZoomableViewport({ const beginPointer = useCallback((event: React.PointerEvent) => { if (event.pointerType === "mouse" && event.button !== 0) return; if (event.pointerType !== "mouse") setHasTouchInput(true); + if (pointersRef.current.size === 0) suppressClickRef.current = false; event.currentTarget.setPointerCapture(event.pointerId); pointersRef.current.set(event.pointerId, { x: event.clientX, y: event.clientY }); const points = Array.from(pointersRef.current.values()); @@ -167,6 +172,7 @@ export function ZoomableViewport({ const gesture = gestureRef.current; const points = Array.from(pointersRef.current.values()); if (gesture?.type === "pinch" && points.length >= 2) { + suppressClickRef.current = true; const nextMidpoint = midpoint(points); const nextDistance = distance(points); const panned = panContent({ @@ -192,6 +198,11 @@ export function ZoomableViewport({ return; } if (gesture?.type !== "drag" || gesture.pointerId !== event.pointerId) return; + if ( + Math.hypot(event.clientX - gesture.startPoint.x, event.clientY - gesture.startPoint.y) > 3 + ) { + suppressClickRef.current = true; + } commitTransform( panContent({ transform: gesture.startTransform, @@ -229,6 +240,25 @@ export function ZoomableViewport({ setIsDragging(false); }, []); + const handleCanvasClick = useCallback( + (event: React.MouseEvent) => { + if (suppressClickRef.current) { + suppressClickRef.current = false; + return; + } + if (!fittedContent || !viewport || !onPressOutsideContent) return; + const bounds = event.currentTarget.getBoundingClientRect(); + const isInside = isPointInsideTransformedContent({ + point: { x: event.clientX - bounds.left, y: event.clientY - bounds.top }, + transform: transformRef.current, + fittedContent, + viewport, + }); + if (!isInside) onPressOutsideContent(); + }, + [fittedContent, onPressOutsideContent, viewport], + ); + const renderedCanvasStyle = useMemo(() => { let cursor: React.CSSProperties["cursor"] = "default"; if (transform.scale > 1) cursor = isDragging ? "grabbing" : "grab"; @@ -254,6 +284,7 @@ export function ZoomableViewport({
; @@ -14,8 +14,10 @@ export interface ZoomableViewportProps { children: ReactNode; actions?: ZoomableViewportAction[]; accessibilityLabel?: string; + fit?: ViewportFitOptions; maxScale?: number; minScale?: number; + onPressOutsideContent?: () => void; style?: StyleProp; testID?: string; wheelActivation?: "always" | "modifier";