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

Image zoom for mobile browser apps #43620

Merged
merged 34 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
bd8910c
Add Handle image zoom for mobile browser apps
ZhenjaHorbach Jun 12, 2024
2bd1a2a
Make a little refactoring
ZhenjaHorbach Jun 12, 2024
ca69b0f
Fix bug with close modal on send attachment screen
ZhenjaHorbach Jun 12, 2024
2e34376
Reset image position when onSwipeDown is undefined
ZhenjaHorbach Jun 12, 2024
5894906
Refactor code
ZhenjaHorbach Jun 13, 2024
e30a3bb
Make some minnor changes
ZhenjaHorbach Jun 13, 2024
2f40b54
Update animation for changing attachment item
ZhenjaHorbach Jun 13, 2024
ebb0eb4
Remove unnecessary type
ZhenjaHorbach Jun 13, 2024
fec109e
Fix minnor issues related with arrows
ZhenjaHorbach Jun 14, 2024
6c6390b
Fix bug with animation for carousel
ZhenjaHorbach Jun 14, 2024
4fa559f
Fix bug with animation for carousel x2
ZhenjaHorbach Jun 15, 2024
1d5f485
Fix conflicts and update branch
ZhenjaHorbach Jun 20, 2024
e303843
Update maxDelay for doubleTapGesture for mobile browsers
ZhenjaHorbach Jun 20, 2024
0bfd329
Merge branch 'main' into handle-image-zoom
ZhenjaHorbach Jun 23, 2024
7345cf8
Fix bug with double tap for ios chrome
ZhenjaHorbach Jun 24, 2024
5029ed3
Make some code optimization
ZhenjaHorbach Jun 24, 2024
b16ab81
Merge branch 'main' into handle-image-zoom
ZhenjaHorbach Jun 25, 2024
a5795c4
Fix comments
ZhenjaHorbach Jun 25, 2024
b2b7ab8
Fix comments x2
ZhenjaHorbach Jun 25, 2024
f1c8297
Fix bug with arrows
ZhenjaHorbach Jun 25, 2024
20cb61c
Hide arrows when item carousel is zoomed
ZhenjaHorbach Jun 25, 2024
52cd759
Fix bug with image lines after zoom
ZhenjaHorbach Jun 27, 2024
8a38e0a
Refactor code
ZhenjaHorbach Jun 27, 2024
bd0b6f1
Revert unnecessary changes
ZhenjaHorbach Jun 27, 2024
d17d1ee
Fix bug with image lines after zoom for IOS web
ZhenjaHorbach Jun 27, 2024
ece63e7
Remove unnecessary style
ZhenjaHorbach Jun 27, 2024
ec7ffbe
Merge branch 'main' into handle-image-zoom
ZhenjaHorbach Jun 27, 2024
b4a70bf
Fix bug with one finger swipe when the image is zoomed
ZhenjaHorbach Jun 28, 2024
fb32193
Merge branch 'main' into handle-image-zoom
ZhenjaHorbach Jul 1, 2024
531e975
Refactor the code to make it similar to native
ZhenjaHorbach Jul 1, 2024
14175f9
Refactor the code to make it similar to native x2
ZhenjaHorbach Jul 1, 2024
7d0bc47
Merge branch 'main' into handle-image-zoom
ZhenjaHorbach Jul 1, 2024
edd2751
Merge branch 'main' into handle-image-zoom
ZhenjaHorbach Jul 2, 2024
8511f5f
Fix comments
ZhenjaHorbach Jul 3, 2024
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
41 changes: 27 additions & 14 deletions src/components/AttachmentModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {Str} from 'expensify-common';
import React, {memo, useCallback, useEffect, useMemo, useState} from 'react';
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {Animated, Keyboard, View} from 'react-native';
import type {GestureType} from 'react-native-gesture-handler';
import {GestureHandlerRootView} from 'react-native-gesture-handler';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
Expand Down Expand Up @@ -184,6 +185,8 @@ function AttachmentModal({
const nope = useSharedValue(false);
const isOverlayModalVisible = (isReceiptAttachment && isDeleteReceiptConfirmModalVisible) || (!isReceiptAttachment && isAttachmentInvalid);
const iouType = useMemo(() => (isTrackExpenseAction ? CONST.IOU.TYPE.TRACK : CONST.IOU.TYPE.SUBMIT), [isTrackExpenseAction]);
const pagerRef = useRef<GestureType>(null);
const [zoomScale, setZoomScale] = useState(1);

const [file, setFile] = useState<FileObject | undefined>(
originalFileName
Expand Down Expand Up @@ -442,18 +445,23 @@ function AttachmentModal({
shouldShowDownloadButton = allowDownload && isDownloadButtonReadyToBeShown && !shouldShowNotFoundPage && !isReceiptAttachment && !isOffline;
shouldShowThreeDotsButton = isReceiptAttachment && isModalOpen && threeDotsMenuItems.length !== 0;
}

const attachmentCarouselRef = useRef<{onChangeArrowsState: (enabled: boolean) => void}>(null);

const context = useMemo(
() => ({
pagerItems: [{source: sourceForAttachmentView, index: 0, isActive: true}],
activePage: 0,
pagerRef: undefined,
pagerRef,
isPagerScrolling: nope,
isScrollEnabled: nope,
onTap: () => {},
onScaleChanged: () => {},
onTap: () => {
attachmentCarouselRef.current?.onChangeArrowsState(zoomScale === 1);
},
onScaleChanged: setZoomScale,
onSwipeDown: closeModal,
}),
[closeModal, nope, sourceForAttachmentView],
[sourceForAttachmentView, nope, closeModal, zoomScale],
);

return (
Expand Down Expand Up @@ -505,15 +513,20 @@ function AttachmentModal({
)}
{!shouldShowNotFoundPage &&
(!isEmptyObject(report) && !isReceiptAttachment ? (
<AttachmentCarousel
accountID={accountID}
type={type}
report={report}
onNavigate={onNavigate}
onClose={closeModal}
source={source}
setDownloadButtonVisibility={setDownloadButtonVisibility}
/>
<AttachmentCarouselPagerContext.Provider value={context}>
<AttachmentCarousel
accountID={accountID}
type={type}
pagerRef={pagerRef}
attachmentCarouselRef={attachmentCarouselRef}
report={report}
onNavigate={onNavigate}
onClose={closeModal}
source={source}
zoomScale={zoomScale}
setDownloadButtonVisibility={setDownloadButtonVisibility}
/>
</AttachmentCarouselPagerContext.Provider>
) : (
!!sourceForAttachmentView &&
shouldLoadAttachment &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {ForwardedRef} from 'react';
import {createContext} from 'react';
import type {GestureType} from 'react-native-gesture-handler';
import type PagerView from 'react-native-pager-view';
import type {SharedValue} from 'react-native-reanimated';
import type {AttachmentSource} from '@components/Attachments/types';
Expand All @@ -17,16 +18,28 @@ type AttachmentCarouselPagerItems = {
};

type AttachmentCarouselPagerContextValue = {
/** The list of items that are shown in the pager */
/** List of items displayed in the attachment */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/** List of items displayed in the attachment */
/** List of attachments displayed in the pager */

Is this more accurate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think yes
But the pager sounds a little unclear 😅
But I don't mind
I'll update now

pagerItems: AttachmentCarouselPagerItems[];

/** The index of the active page */
/** Index of the currently active page */
activePage: number;
pagerRef?: ForwardedRef<PagerView>;

/** Ref to the active attachment */
pagerRef?: ForwardedRef<PagerView | GestureType>;

/** Indicates if the pager is currently scrolling */
isPagerScrolling: SharedValue<boolean>;

/** Indicates if scrolling is enabled for the attachment */
isScrollEnabled: SharedValue<boolean>;

/** Function to call after a tap event */
onTap: () => void;

/** Function to call when the scale changes */
onScaleChanged: (scale: number) => void;

/** Function to call after a swipe down event */
onSwipeDown: () => void;
};

Expand Down
46 changes: 38 additions & 8 deletions src/components/Attachments/AttachmentCarousel/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import isEqual from 'lodash/isEqual';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import type {MutableRefObject} from 'react';
import React, {useCallback, useEffect, useImperativeHandle, useMemo, useState} from 'react';
import type {ListRenderItemInfo} from 'react-native';
import {Keyboard, PixelRatio, View} from 'react-native';
import type {GestureType} from 'react-native-gesture-handler';
import {Gesture, GestureDetector} from 'react-native-gesture-handler';
import {withOnyx} from 'react-native-onyx';
import Animated, {scrollTo, useAnimatedRef} from 'react-native-reanimated';
Expand Down Expand Up @@ -33,7 +35,19 @@ const viewabilityConfig = {

const MIN_FLING_VELOCITY = 500;

function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, type, accountID}: AttachmentCarouselProps) {
function AttachmentCarousel({
report,
reportActions,
parentReportActions,
source,
onNavigate,
setDownloadButtonVisibility,
type,
accountID,
pagerRef,
zoomScale,
attachmentCarouselRef,
}: AttachmentCarouselProps) {
const theme = useTheme();
const {translate} = useLocalize();
const {isSmallScreenWidth, windowWidth} = useWindowDimensions();
Expand All @@ -51,7 +65,18 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
const [page, setPage] = useState(0);
const [attachments, setAttachments] = useState<Attachment[]>([]);
const [activeSource, setActiveSource] = useState<AttachmentSource | null>(source);
const {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows} = useCarouselArrows();
const {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows, onChangeArrowsState} = useCarouselArrows();

useEffect(() => {
if (!canUseTouchScreen) {
return;
}
if (zoomScale !== 1) {
setShouldShowArrows(false);
return;
}
setShouldShowArrows(true);
}, [canUseTouchScreen, page, setShouldShowArrows, zoomScale]);

const compareImage = useCallback((attachment: Attachment) => attachment.source === source, [source]);

Expand Down Expand Up @@ -104,6 +129,10 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [cellWidth]);

useImperativeHandle(attachmentCarouselRef, () => ({
onChangeArrowsState,
}));

/** Updates the page state when the user navigates between attachments */
const updatePage = useCallback(
({viewableItems}: UpdatePageProps) => {
Expand Down Expand Up @@ -176,18 +205,18 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
<CarouselItem
item={item}
isFocused={activeSource === item.source}
onPress={canUseTouchScreen ? () => setShouldShowArrows((oldState) => !oldState) : undefined}
onPress={canUseTouchScreen ? () => onChangeArrowsState(zoomScale === 1) : undefined}
isModalHovered={shouldShowArrows}
/>
</View>
),
[activeSource, canUseTouchScreen, cellWidth, setShouldShowArrows, shouldShowArrows, styles.h100],
[activeSource, canUseTouchScreen, cellWidth, zoomScale, onChangeArrowsState, shouldShowArrows, styles.h100],
);
/** Pan gesture handing swiping through attachments on touch screen devices */
const pan = useMemo(
() =>
Gesture.Pan()
.enabled(canUseTouchScreen)
.enabled(canUseTouchScreen && zoomScale === 1)
.onUpdate(({translationX}) => scrollTo(scrollRef, page * cellWidth - translationX, 0, false))
.onEnd(({translationX, velocityX}) => {
let newIndex;
Expand All @@ -204,8 +233,9 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
}

scrollTo(scrollRef, newIndex * cellWidth, 0, true);
}),
[attachments.length, canUseTouchScreen, cellWidth, page, scrollRef],
})
.withRef(pagerRef as MutableRefObject<GestureType | undefined>),
[attachments.length, canUseTouchScreen, cellWidth, page, pagerRef, scrollRef, zoomScale],
);

return (
Expand Down
13 changes: 13 additions & 0 deletions src/components/Attachments/AttachmentCarousel/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type {ForwardedRef} from 'react';
import type {ViewToken} from 'react-native';
import type {GestureType} from 'react-native-gesture-handler';
import type {OnyxEntry} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {Attachment, AttachmentSource} from '@components/Attachments/types';
Expand Down Expand Up @@ -38,6 +40,17 @@ type AttachmentCarouselProps = AttachmentCaraouselOnyxProps & {

/** A callback that is called when swipe-down-to-close gesture happens */
onClose: () => void;

/** The ref of the pager */
pagerRef: ForwardedRef<GestureType>;

/** The zoom scale of the attachment */
zoomScale?: number;

/** The ref of changeArrows */
attachmentCarouselRef?: React.RefObject<{
onChangeArrowsState: (enabled: boolean) => void;
}>;
};

export type {AttachmentCarouselProps, UpdatePageProps, AttachmentCaraouselOnyxProps};
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ function useCarouselArrows() {
const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();
const [shouldShowArrows, setShouldShowArrowsInternal] = useState(canUseTouchScreen);
const autoHideArrowTimeout = useRef<NodeJS.Timeout | null>(null);
const singleTapRef = useRef<NodeJS.Timeout | null>(null);

/**
* Cancels the automatic hiding of the arrows.
Expand Down Expand Up @@ -45,7 +46,27 @@ function useCarouselArrows() {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows};
const onChangeArrowsState = useCallback(
(enabled: boolean) => {
if (!enabled) {
return;
}

if (singleTapRef.current) {
clearTimeout(singleTapRef.current);
singleTapRef.current = null;
return;
}

singleTapRef.current = setTimeout(() => {
setShouldShowArrows((oldState) => !oldState);
singleTapRef.current = null;
}, 200);
},
[setShouldShowArrows],
);

return {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows, onChangeArrowsState};
}

export default useCarouselArrows;
25 changes: 6 additions & 19 deletions src/components/ImageView/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import Image from '@components/Image';
import RESIZE_MODES from '@components/Image/resizeModes';
import type {ImageOnLoadEvent} from '@components/Image/types';
import Lightbox from '@components/Lightbox';
import PressableWithoutFeedback from '@components/Pressable/PressableWithoutFeedback';
import useNetwork from '@hooks/useNetwork';
import useStyleUtils from '@hooks/useStyleUtils';
Expand Down Expand Up @@ -200,25 +201,11 @@ function ImageView({isAuthTokenRequired = false, url, fileName, onError}: ImageV

if (canUseTouchScreen) {
return (
<View
style={[styles.imageViewContainer, styles.overflowHidden]}
onLayout={onContainerLayoutChanged}
>
<Image
source={{uri: url}}
isAuthTokenRequired={isAuthTokenRequired}
// Hide image until finished loading to prevent showing preview with wrong dimensions.
style={isLoading || zoomScale === 0 ? undefined : [styles.w100, styles.h100]}
// When Image dimensions are lower than the container boundary(zoomscale <= 1), use `contain` to render the image with natural dimensions.
// Both `center` and `contain` keeps the image centered on both x and y axis.
resizeMode={zoomScale > 1 ? RESIZE_MODES.center : RESIZE_MODES.contain}
onLoadStart={imageLoadingStart}
onLoad={imageLoad}
onError={onError}
/>
{((isLoading && (!isOffline || isLocalFile)) || (!isLoading && zoomScale === 0)) && <FullscreenLoadingIndicator style={[styles.opacity1, styles.bgTransparent]} />}
{isLoading && !isLocalFile && <AttachmentOfflineIndicator />}
</View>
<Lightbox
uri={url}
isAuthTokenRequired={isAuthTokenRequired}
onError={onError}
/>
);
}
return (
Expand Down
7 changes: 5 additions & 2 deletions src/components/MultiGestureCanvas/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {ForwardedRef} from 'react';
import React, {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';
Expand Down Expand Up @@ -40,14 +41,15 @@ type MultiGestureCanvasProps = ChildrenProps & {
shouldDisableTransformationGestures?: 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>; // TODO: For TS migration: Exclude<GestureRef, number>
pagerRef?: ForwardedRef<PagerView | GestureType>; // TODO: For TS migration: Exclude<GestureRef, number>

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

/** Handles scale changed event */
onTap?: OnTapCallback;

/** Handles swipe down event */
onSwipeDown?: OnSwipeDownCallback;
};

Expand Down Expand Up @@ -242,11 +244,12 @@ function MultiGestureCanvas({
<GestureDetector gesture={Gesture.Simultaneous(pinchGesture, Gesture.Race(singleTapGesture, doubleTapGesture, panGesture))}>
<View
collapsable={false}
onTouchEnd={(e) => e.preventDefault()}
style={StyleUtils.getFullscreenCenteredContentStyles()}
>
<Animated.View
collapsable={false}
style={animatedStyles}
style={[animatedStyles, styles.canvasContainer]}
>
{children}
</Animated.View>
Expand Down
5 changes: 4 additions & 1 deletion src/components/MultiGestureCanvas/usePanGesture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ 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 * as Browser from '@libs/Browser';
import {SPRING_CONFIG} from './constants';
import type {MultiGestureCanvasVariables} from './types';
import * as MultiGestureCanvasUtils from './utils';
Expand Down Expand Up @@ -57,6 +58,8 @@ const usePanGesture = ({
const panVelocityX = useSharedValue(0);
const panVelocityY = useSharedValue(0);

const isMobileBrowser = Browser.isMobile();

// Disable "swipe down to close" gesture when content is bigger than the canvas
const enableSwipeDownToClose = useDerivedValue(() => canvasSize.height < zoomedContentHeight.value, [canvasSize.height]);

Expand Down Expand Up @@ -206,7 +209,7 @@ const usePanGesture = ({
panVelocityX.value = evt.velocityX;
panVelocityY.value = evt.velocityY;

if (!isSwipingDownToClose.value) {
if (!isSwipingDownToClose.value && !isMobileBrowser) {
panTranslateX.value += evt.changeX;
}

Expand Down
5 changes: 5 additions & 0 deletions src/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1512,6 +1512,11 @@ const styles = (theme: ThemeColors) =>
height: '100%',
},

canvasContainer: {
borderWidth: 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this border do exactly? Can you point it out to me in a screenshot? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Visually this does not affect anything
Because the border color is the same as the background

2024-06-27.12.11.57.mov

But this fixes a bug related to lines during zoom

2024-06-26.09.58.47.mov

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah interesting, that works for me. Just curious though, how does it fix the line bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps due to a lack of performance during zooming, the phone's browser does not have time to re-render the last line of the image

But since now the last line is border

The bug will be fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Neat! Thanks for explaining :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Not A Blocker; but maybe worth adding a comment here in the code summarizing this

borderColor: theme.appBG,
},

sidebarHeaderContainer: {
flexDirection: 'row',
paddingHorizontal: 20,
Expand Down
Loading