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";