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

Fix image moves permanently when swipe down #51921

Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/components/Attachments/AttachmentCarousel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
const [attachments, setAttachments] = useState<Attachment[]>([]);
const [activeSource, setActiveSource] = useState<AttachmentSource | null>(source);
const {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows} = useCarouselArrows();
const {handleTap, handleScaleChange, scale} = useCarouselContextEvents(setShouldShowArrows);
const {handleTap, handleScaleChange, isScrollEnabled} = useCarouselContextEvents(setShouldShowArrows);

useEffect(() => {
if (!canUseTouchScreen) {
Expand Down Expand Up @@ -201,12 +201,12 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
activePage: 0,
pagerRef,
isPagerScrolling: nope,
isScrollEnabled: nope,
isScrollEnabled,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Same with the previous issue.

onTap: handleTap,
onScaleChanged: handleScaleChange,
onSwipeDown: onClose,
}),
[source, nope, handleTap, handleScaleChange, onClose],
[source, nope, isScrollEnabled, handleTap, handleScaleChange, onClose],
);

/** Defines how a single attachment should be rendered */
Expand All @@ -229,14 +229,14 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
Gesture.Pan()
.enabled(canUseTouchScreen)
.onUpdate(({translationX}) => {
if (scale.current !== 1) {
if (!isScrollEnabled.value) {
return;
}

scrollTo(scrollRef, page * cellWidth - translationX, 0, false);
})
.onEnd(({translationX, velocityX}) => {
if (scale.current !== 1) {
if (!isScrollEnabled.value) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The scroll stuck because we swipe down while scrolling, making isScrollEnabled false. Now I update isPagerScrolling while scrolling.


Expand All @@ -257,7 +257,7 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
})
// eslint-disable-next-line react-compiler/react-compiler
.withRef(pagerRef as MutableRefObject<GestureType | undefined>),
[attachments.length, canUseTouchScreen, cellWidth, page, scale, scrollRef],
[attachments.length, canUseTouchScreen, cellWidth, page, isScrollEnabled, scrollRef],
);

return (
Expand Down
27 changes: 19 additions & 8 deletions src/components/AvatarCropModal/AvatarCropModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {LayoutChangeEvent} from 'react-native';
import {Gesture, GestureHandlerRootView} from 'react-native-gesture-handler';
import type {GestureUpdateEvent, PanGestureChangeEventPayload, PanGestureHandlerEventPayload} from 'react-native-gesture-handler';
import ImageSize from 'react-native-image-size';
import {interpolate, runOnUI, useSharedValue, useWorkletCallback} from 'react-native-reanimated';
import {interpolate, runOnUI, useSharedValue} from 'react-native-reanimated';
import Button from '@components/Button';
import HeaderGap from '@components/HeaderGap';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
Expand Down Expand Up @@ -144,12 +144,18 @@ function AvatarCropModal({imageUri = '', imageName = '', imageType = '', onClose
/**
* Validates that value is within the provided mix/max range.
*/
const clamp = useWorkletCallback((value: number, [min, max]) => interpolate(value, [min, max], [min, max], 'clamp'), []);
const clamp = useCallback((value: number, [min, max]: [number, number]) => {
'worklet';

return interpolate(value, [min, max], [min, max], 'clamp');
}, []);

/**
* Returns current image size taking into account scale and rotation.
*/
const getDisplayedImageSize = useWorkletCallback(() => {
const getDisplayedImageSize = useCallback(() => {
'worklet';

let height = imageContainerSize * scale.value;
let width = imageContainerSize * scale.value;

Expand All @@ -162,28 +168,33 @@ function AvatarCropModal({imageUri = '', imageName = '', imageType = '', onClose
}

return {height, width};
}, [imageContainerSize, scale]);
}, [imageContainerSize, scale, originalImageWidth, originalImageHeight]);

/**
* Validates the offset to prevent overflow, and updates the image offset.
*/
const updateImageOffset = useWorkletCallback(
const updateImageOffset = useCallback(
(offsetX: number, offsetY: number) => {
'worklet';

const {height, width} = getDisplayedImageSize();
const maxOffsetX = (width - imageContainerSize) / 2;
const maxOffsetY = (height - imageContainerSize) / 2;
translateX.value = clamp(offsetX, [maxOffsetX * -1, maxOffsetX]);
translateY.value = clamp(offsetY, [maxOffsetY * -1, maxOffsetY]);
// eslint-disable-next-line react-compiler/react-compiler
prevMaxOffsetX.value = maxOffsetX;
prevMaxOffsetY.value = maxOffsetY;
},
[imageContainerSize, scale, clamp],
[getDisplayedImageSize, imageContainerSize, translateX, translateY, prevMaxOffsetX, prevMaxOffsetY, clamp],
);

const newScaleValue = useWorkletCallback((newSliderValue: number, containerSize: number) => {
const newScaleValue = useCallback((newSliderValue: number, containerSize: number) => {
'worklet';

const {MAX_SCALE, MIN_SCALE} = CONST.AVATAR_CROP_MODAL;
return (newSliderValue / containerSize) * (MAX_SCALE - MIN_SCALE) + MIN_SCALE;
});
}, []);

/**
* Calculates new x & y image translate value on image panning
Expand Down
7 changes: 6 additions & 1 deletion src/components/Lightbox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
* we need to create a shared value that can be used in the render function.
*/
const isPagerScrollingFallback = useSharedValue(false);
const isScrollingEnabledFallback = useSharedValue(false);
const {isOffline} = useNetwork();

const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext);
Expand All @@ -63,12 +64,14 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
onScaleChanged: onScaleChangedContext,
onSwipeDown,
pagerRef,
isScrollEnabled,
} = useMemo(() => {
if (attachmentCarouselPagerContext === null) {
return {
isUsedInCarousel: false,
isSingleCarouselItem: true,
isPagerScrolling: isPagerScrollingFallback,
isScrollEnabled: isScrollingEnabledFallback,
page: 0,
activePage: 0,
onTap: () => {},
Expand All @@ -85,7 +88,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
isSingleCarouselItem: attachmentCarouselPagerContext.pagerItems.length === 1,
page: foundPage,
};
}, [attachmentCarouselPagerContext, isPagerScrollingFallback, uri]);
}, [attachmentCarouselPagerContext, isPagerScrollingFallback, isScrollingEnabledFallback, uri]);

/** Whether the Lightbox is used within an attachment carousel and there are more than one page in the carousel */
const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem;
Expand Down Expand Up @@ -215,7 +218,9 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan
contentSize={contentSize}
zoomRange={zoomRange}
pagerRef={pagerRef}
isUsedInCarousel={isUsedInCarousel}
shouldDisableTransformationGestures={isPagerScrolling}
isPagerScrollEnabled={isScrollEnabled}
onTap={onTap}
onScaleChanged={scaleChange}
onSwipeDown={onSwipeDown}
Expand Down
96 changes: 62 additions & 34 deletions src/components/MultiGestureCanvas/index.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type {ForwardedRef} from 'react';
import React, {useEffect, useMemo, useRef} from 'react';
import React, {useCallback, useEffect, useMemo, useRef} from 'react';
import {View} from 'react-native';
import type {GestureType} from 'react-native-gesture-handler';
import {Gesture, GestureDetector} from 'react-native-gesture-handler';
import type {GestureRef} from 'react-native-gesture-handler/lib/typescript/handlers/gestures/gesture';
import type PagerView from 'react-native-pager-view';
import type {SharedValue} from 'react-native-reanimated';
import Animated, {cancelAnimation, runOnUI, useAnimatedStyle, useDerivedValue, useSharedValue, useWorkletCallback, withSpring} from 'react-native-reanimated';
import Animated, {cancelAnimation, runOnUI, useAnimatedReaction, useAnimatedStyle, useDerivedValue, useSharedValue, withSpring} from 'react-native-reanimated';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import type ChildrenProps from '@src/types/utils/ChildrenProps';
Expand Down Expand Up @@ -40,9 +40,15 @@ type MultiGestureCanvasProps = ChildrenProps & {
/** A shared value of type boolean, that indicates disabled the transformation gestures (pinch, pan, double tap) */
shouldDisableTransformationGestures?: SharedValue<boolean>;

/** A shared value to enable/disable the pager scroll */
isPagerScrollEnabled: SharedValue<boolean>;

/** If there is a pager wrapping the canvas, we need to disable the pan gesture in case the pager is swiping */
pagerRef?: ForwardedRef<PagerView | GestureType>; // TODO: For TS migration: Exclude<GestureRef, number>

/** Whether the component is being used inside a carousel */
isUsedInCarousel: boolean;

/** Handles scale changed event */
onScaleChanged?: OnScaleChangedCallback;

Expand All @@ -62,7 +68,9 @@ function MultiGestureCanvas({
isActive = true,
children,
pagerRef,
isUsedInCarousel,
shouldDisableTransformationGestures: shouldDisableTransformationGesturesProp,
isPagerScrollEnabled,
onTap,
onScaleChanged,
onSwipeDown,
Expand Down Expand Up @@ -107,47 +115,65 @@ function MultiGestureCanvas({
const offsetX = useSharedValue(0);
const offsetY = useSharedValue(0);

useAnimatedReaction(
() => isSwipingDownToClose.value,
(current) => {
if (!isUsedInCarousel) {
return;
}
// eslint-disable-next-line react-compiler/react-compiler, no-param-reassign
isPagerScrollEnabled.value = !current;
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. In non-carousel, changing isScrollEnabled, changes the nope shared value here

isPagerScrolling: nope,
isScrollEnabled: nope,

which sets isPagerScrolling to true too, disabling the pan gesture.

);

/**
* Stops any currently running decay animation from panning
*/
const stopAnimation = useWorkletCallback(() => {
const stopAnimation = useCallback(() => {
'worklet';

cancelAnimation(offsetX);
cancelAnimation(offsetY);
});
}, [offsetX, offsetY]);

/**
* Resets the canvas to the initial state and animates back smoothly
*/
const reset = useWorkletCallback((animated: boolean, callback?: () => void) => {
stopAnimation();

// eslint-disable-next-line react-compiler/react-compiler
offsetX.value = 0;
offsetY.value = 0;
pinchScale.value = 1;

if (animated) {
panTranslateX.value = withSpring(0, SPRING_CONFIG);
panTranslateY.value = withSpring(0, SPRING_CONFIG);
pinchTranslateX.value = withSpring(0, SPRING_CONFIG);
pinchTranslateY.value = withSpring(0, SPRING_CONFIG);
zoomScale.value = withSpring(1, SPRING_CONFIG, callback);

return;
}

panTranslateX.value = 0;
panTranslateY.value = 0;
pinchTranslateX.value = 0;
pinchTranslateY.value = 0;
zoomScale.value = 1;

if (callback === undefined) {
return;
}

callback();
});
const reset = useCallback(
(animated: boolean, callback?: () => void) => {
'worklet';

stopAnimation();

// eslint-disable-next-line react-compiler/react-compiler
offsetX.value = 0;
offsetY.value = 0;
pinchScale.value = 1;

if (animated) {
panTranslateX.value = withSpring(0, SPRING_CONFIG);
panTranslateY.value = withSpring(0, SPRING_CONFIG);
pinchTranslateX.value = withSpring(0, SPRING_CONFIG);
pinchTranslateY.value = withSpring(0, SPRING_CONFIG);
zoomScale.value = withSpring(1, SPRING_CONFIG, callback);

return;
}

panTranslateX.value = 0;
panTranslateY.value = 0;
pinchTranslateX.value = 0;
pinchTranslateY.value = 0;
zoomScale.value = 1;

if (callback === undefined) {
return;
}

callback();
},
[stopAnimation, offsetX, offsetY, pinchScale, panTranslateX, panTranslateY, pinchTranslateX, pinchTranslateY, zoomScale],
);

const {singleTapGesture: baseSingleTapGesture, doubleTapGesture} = useTapGestures({
canvasSize,
Expand All @@ -164,6 +190,7 @@ function MultiGestureCanvas({
onTap,
shouldDisableTransformationGestures,
});
// eslint-disable-next-line react-compiler/react-compiler
const singleTapGesture = baseSingleTapGesture.requireExternalGestureToFail(doubleTapGesture, panGestureRef);

const panGestureSimultaneousList = useMemo(
Expand All @@ -186,6 +213,7 @@ function MultiGestureCanvas({
onSwipeDown,
})
.simultaneousWithExternalGesture(...panGestureSimultaneousList)
// eslint-disable-next-line react-compiler/react-compiler
.withRef(panGestureRef);

const pinchGesture = usePinchGesture({
Expand Down
24 changes: 15 additions & 9 deletions src/components/MultiGestureCanvas/usePanGesture.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/* eslint-disable no-param-reassign */
import {useCallback} from 'react';
import {Dimensions} from 'react-native';
import type {PanGesture} from 'react-native-gesture-handler';
import {Gesture} from 'react-native-gesture-handler';
import {runOnJS, useDerivedValue, useSharedValue, useWorkletCallback, withDecay, withSpring} from 'react-native-reanimated';
import {runOnJS, useDerivedValue, useSharedValue, withDecay, withSpring} from 'react-native-reanimated';
import * as Browser from '@libs/Browser';
import {SPRING_CONFIG} from './constants';
import type {MultiGestureCanvasVariables} from './types';
Expand Down Expand Up @@ -66,7 +67,9 @@ const usePanGesture = ({
// Calculates bounds of the scaled content
// Can we pan left/right/up/down
// Can be used to limit gesture or implementing tension effect
const getBounds = useWorkletCallback(() => {
const getBounds = useCallback(() => {
'worklet';

let horizontalBoundary = 0;
let verticalBoundary = 0;

Expand All @@ -87,32 +90,34 @@ const usePanGesture = ({
};

// If the horizontal/vertical offset is the same after clamping to the min/max boundaries, the content is within the boundaries
const isInHoriztontalBoundary = clampedOffset.x === offsetX.value;
const isInHorizontalBoundary = clampedOffset.x === offsetX.value;
const isInVerticalBoundary = clampedOffset.y === offsetY.value;

return {
horizontalBoundaries,
verticalBoundaries,
clampedOffset,
isInHoriztontalBoundary,
isInHorizontalBoundary,
isInVerticalBoundary,
};
}, [canvasSize.width, canvasSize.height]);
}, [canvasSize.width, canvasSize.height, zoomedContentWidth, zoomedContentHeight, offsetX, offsetY]);

// We want to smoothly decay/end the gesture by phasing out the pan animation
// In case the content is outside of the boundaries of the canvas,
// we need to move the content back into the boundaries
const finishPanGesture = useWorkletCallback(() => {
const finishPanGesture = useCallback(() => {
'worklet';

// If the content is centered within the canvas, we don't need to run any animations
if (offsetX.value === 0 && offsetY.value === 0 && panTranslateX.value === 0 && panTranslateY.value === 0) {
return;
}

const {clampedOffset, isInHoriztontalBoundary, isInVerticalBoundary, horizontalBoundaries, verticalBoundaries} = getBounds();
const {clampedOffset, isInHorizontalBoundary, isInVerticalBoundary, horizontalBoundaries, verticalBoundaries} = getBounds();

// If the content is within the horizontal/vertical boundaries of the canvas, we can smoothly phase out the animation
// If not, we need to snap back to the boundaries
if (isInHoriztontalBoundary) {
if (isInHorizontalBoundary) {
// If the (absolute) velocity is 0, we don't need to run an animation
if (Math.abs(panVelocityX.value) !== 0) {
// Phase out the pan animation
Expand Down Expand Up @@ -161,7 +166,7 @@ const usePanGesture = ({
// Reset velocity variables after we finished the pan gesture
panVelocityX.value = 0;
panVelocityY.value = 0;
});
}, [offsetX, offsetY, panTranslateX, panTranslateY, panVelocityX, panVelocityY, zoomScale, isSwipingDownToClose, getBounds, onSwipeDown]);

const panGesture = Gesture.Pan()
.manualActivation(true)
Expand All @@ -183,6 +188,7 @@ const usePanGesture = ({
if (Math.abs(velocityY) > velocityX && velocityY > 20) {
state.activate();

// eslint-disable-next-line react-compiler/react-compiler
isSwipingDownToClose.value = true;
previousTouch.value = null;

Expand Down
Loading
Loading