From 0887405b10cc4a945cf2763085a328ba7819915c Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 9 Apr 2024 16:04:02 +0300 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F=20Fix=20carousel=20sc?= =?UTF-8?q?rolling=20in=20AttachmentCarousel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fixed an issue with carousel scrolling in the AttachmentCarousel component - Implemented a new pan gesture for smoother scrolling experience - Adjusted layout calculations for optimal rendering of attachments Related to: - #23546 - #39833 - #22318 - #21177 - #31166 --- .../Attachments/AttachmentCarousel/index.tsx | 92 +++++++++++-------- 1 file changed, 55 insertions(+), 37 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/index.tsx b/src/components/Attachments/AttachmentCarousel/index.tsx index 3a7e0f19c4cd..ff1c5fabf01f 100644 --- a/src/components/Attachments/AttachmentCarousel/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/index.tsx @@ -1,7 +1,9 @@ import isEqual from 'lodash/isEqual'; -import React, {useCallback, useEffect, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useState} from 'react'; import type {ListRenderItemInfo} from 'react-native'; -import {FlatList, Keyboard, PixelRatio, View} from 'react-native'; +import {Keyboard, PixelRatio, View} from 'react-native'; +import {Gesture, GestureDetector} from 'react-native-gesture-handler'; +import Animated, {scrollTo, useAnimatedRef} from 'react-native-reanimated'; import {withOnyx} from 'react-native-onyx'; import type {Attachment, AttachmentSource} from '@components/Attachments/types'; import BlockingView from '@components/BlockingViews/BlockingView'; @@ -15,7 +17,7 @@ import Navigation from '@libs/Navigation/Navigation'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import AttachmentCarouselCellRenderer from './AttachmentCarouselCellRenderer'; +import useWindowDimensions from '@hooks/useWindowDimensions'; import CarouselActions from './CarouselActions'; import CarouselButtons from './CarouselButtons'; import CarouselItem from './CarouselItem'; @@ -29,16 +31,21 @@ const viewabilityConfig = { itemVisiblePercentThreshold: 95, }; +const MIN_FLING_VELOCITY = 500; + function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility}: AttachmentCarouselProps) { const theme = useTheme(); const {translate} = useLocalize(); + const {isSmallScreenWidth, windowWidth} = useWindowDimensions(); const styles = useThemeStyles(); const {isFullScreenRef} = useFullScreenContext(); - const scrollRef = useRef(null); + const scrollRef = useAnimatedRef>>(); const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen(); - const [containerWidth, setContainerWidth] = useState(0); + const modalStyles = styles.centeredModalStyles(isSmallScreenWidth, true); + const cellWidth = useMemo(() => PixelRatio.roundToNearestPixel(windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2), [windowWidth]); + const [page, setPage] = useState(0); const [attachments, setAttachments] = useState([]); const [activeSource, setActiveSource] = useState(source); @@ -120,7 +127,7 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, scrollRef.current.scrollToIndex({index: nextIndex, animated: canUseTouchScreen}); }, - [attachments, canUseTouchScreen, isFullScreenRef, page], + [attachments, canUseTouchScreen, isFullScreenRef, page, scrollRef], ); const extractItemKey = useCallback( @@ -132,35 +139,56 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, /** Calculate items layout information to optimize scrolling performance */ const getItemLayout = useCallback( (data: ArrayLike | null | undefined, index: number) => ({ - length: containerWidth, - offset: containerWidth * index, + length: cellWidth, + offset: cellWidth * index, index, }), - [containerWidth], + [cellWidth], ); /** Defines how a single attachment should be rendered */ const renderItem = useCallback( ({item}: ListRenderItemInfo) => ( - setShouldShowArrows((oldState: boolean) => !oldState) : undefined} - isModalHovered={shouldShowArrows} - /> + + setShouldShowArrows((oldState) => !oldState) : undefined} + isModalHovered={shouldShowArrows} + /> + ), - [activeSource, canUseTouchScreen, setShouldShowArrows, shouldShowArrows], + [activeSource, canUseTouchScreen, cellWidth, setShouldShowArrows, shouldShowArrows, styles.h100], + ); + + /** Pan gesture handing swiping through attachments on touch screen devices */ + const pan = useMemo( + () => + Gesture.Pan() + .enabled(canUseTouchScreen) + .onUpdate(({translationX}) => scrollTo(scrollRef, page * cellWidth - translationX, 0, false)) + .onEnd(({translationX, velocityX}) => { + let newIndex; + if (velocityX > MIN_FLING_VELOCITY) { + // User flung to the right + newIndex = Math.max(0, page - 1); + } else if (velocityX < -MIN_FLING_VELOCITY) { + // User flung to the left + newIndex = Math.min(attachments.length - 1, page + 1); + } else { + // snap scroll position to the nearest cell (making sure it's within the bounds of the list) + const delta = Math.round(-translationX / cellWidth) + newIndex = Math.min(attachments.length - 1, Math.max(0, page + delta)); + } + + scrollTo(scrollRef, newIndex * cellWidth, 0, true); + }), + [attachments.length, canUseTouchScreen, cellWidth, page, scrollRef], ); return ( { - if (isFullScreenRef.current) { - return; - } - setContainerWidth(PixelRatio.roundToNearestPixel(nativeEvent.layout.width)); - }} onMouseEnter={() => !canUseTouchScreen && setShouldShowArrows(true)} onMouseLeave={() => !canUseTouchScreen && setShouldShowArrows(false)} > @@ -184,36 +212,26 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, cancelAutoHideArrow={cancelAutoHideArrows} /> - {containerWidth > 0 && ( - + - )} + From 155bf0e7bdd1eaa8aef9066560475b9609cd47de Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 9 Apr 2024 16:28:37 +0300 Subject: [PATCH 2/5] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20chore(AttachmentCarous?= =?UTF-8?q?el):=20fix=20formatting=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Attachments/AttachmentCarousel/index.tsx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/index.tsx b/src/components/Attachments/AttachmentCarousel/index.tsx index ff1c5fabf01f..c863edc1dc07 100644 --- a/src/components/Attachments/AttachmentCarousel/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/index.tsx @@ -3,8 +3,8 @@ import React, {useCallback, useEffect, useMemo, useState} from 'react'; import type {ListRenderItemInfo} from 'react-native'; import {Keyboard, PixelRatio, View} from 'react-native'; import {Gesture, GestureDetector} from 'react-native-gesture-handler'; -import Animated, {scrollTo, useAnimatedRef} from 'react-native-reanimated'; import {withOnyx} from 'react-native-onyx'; +import Animated, {scrollTo, useAnimatedRef} from 'react-native-reanimated'; import type {Attachment, AttachmentSource} from '@components/Attachments/types'; import BlockingView from '@components/BlockingViews/BlockingView'; import * as Illustrations from '@components/Icon/Illustrations'; @@ -12,12 +12,12 @@ import {useFullScreenContext} from '@components/VideoPlayerContexts/FullScreenCo import useLocalize from '@hooks/useLocalize'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; +import useWindowDimensions from '@hooks/useWindowDimensions'; import * as DeviceCapabilities from '@libs/DeviceCapabilities'; import Navigation from '@libs/Navigation/Navigation'; import variables from '@styles/variables'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import useWindowDimensions from '@hooks/useWindowDimensions'; import CarouselActions from './CarouselActions'; import CarouselButtons from './CarouselButtons'; import CarouselItem from './CarouselItem'; @@ -44,8 +44,10 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, const canUseTouchScreen = DeviceCapabilities.canUseTouchScreen(); const modalStyles = styles.centeredModalStyles(isSmallScreenWidth, true); - const cellWidth = useMemo(() => PixelRatio.roundToNearestPixel(windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2), [windowWidth]); - + const cellWidth = useMemo( + () => PixelRatio.roundToNearestPixel(windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2), + [modalStyles.borderWidth, modalStyles.marginHorizontal, windowWidth], + ); const [page, setPage] = useState(0); const [attachments, setAttachments] = useState([]); const [activeSource, setActiveSource] = useState(source); @@ -160,7 +162,6 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, ), [activeSource, canUseTouchScreen, cellWidth, setShouldShowArrows, shouldShowArrows, styles.h100], ); - /** Pan gesture handing swiping through attachments on touch screen devices */ const pan = useMemo( () => @@ -177,7 +178,7 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, newIndex = Math.min(attachments.length - 1, page + 1); } else { // snap scroll position to the nearest cell (making sure it's within the bounds of the list) - const delta = Math.round(-translationX / cellWidth) + const delta = Math.round(-translationX / cellWidth); newIndex = Math.min(attachments.length - 1, Math.max(0, page + delta)); } From a02bb100ecdf7d40e9bcc18bf65af028796707ef Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 22 Apr 2024 20:42:54 +0300 Subject: [PATCH 3/5] Remove unused AttachmentCarouselCellRenderer.tsx --- .../AttachmentCarouselCellRenderer.tsx | 29 ------------------- 1 file changed, 29 deletions(-) delete mode 100644 src/components/Attachments/AttachmentCarousel/AttachmentCarouselCellRenderer.tsx diff --git a/src/components/Attachments/AttachmentCarousel/AttachmentCarouselCellRenderer.tsx b/src/components/Attachments/AttachmentCarousel/AttachmentCarouselCellRenderer.tsx deleted file mode 100644 index 839e05c419df..000000000000 --- a/src/components/Attachments/AttachmentCarousel/AttachmentCarouselCellRenderer.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import React from 'react'; -import type {StyleProp, ViewStyle} from 'react-native'; -import {PixelRatio, View} from 'react-native'; -import useThemeStyles from '@hooks/useThemeStyles'; -import useWindowDimensions from '@hooks/useWindowDimensions'; - -type AttachmentCarouselCellRendererProps = { - /** Cell Container styles */ - style?: StyleProp; -}; - -function AttachmentCarouselCellRenderer(props: AttachmentCarouselCellRendererProps) { - const styles = useThemeStyles(); - const {windowWidth, isSmallScreenWidth} = useWindowDimensions(); - const modalStyles = styles.centeredModalStyles(isSmallScreenWidth, true); - const style = [props.style, styles.h100, {width: PixelRatio.roundToNearestPixel(windowWidth - (modalStyles.marginHorizontal + modalStyles.borderWidth) * 2)}]; - - return ( - - ); -} - -AttachmentCarouselCellRenderer.displayName = 'AttachmentCarouselCellRenderer'; - -export default React.memo(AttachmentCarouselCellRenderer); From 3a1e687de0db447a7be7e2a74a04ad1e6e1b48e1 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 7 May 2024 15:29:54 +0300 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=94=84=20Adjust=20scroll=20position?= =?UTF-8?q?=20on=20window=20width=20change=20in=20AttachmentCarousel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Added useEffect hook to readjust scroll position in AttachmentCarousel component when window width is resized to ensure correct display of images. - Updated logic to automatically scroll to the correct index when window width changes. --- .../Attachments/AttachmentCarousel/index.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/components/Attachments/AttachmentCarousel/index.tsx b/src/components/Attachments/AttachmentCarousel/index.tsx index c863edc1dc07..7b1f0010ebc8 100644 --- a/src/components/Attachments/AttachmentCarousel/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/index.tsx @@ -84,6 +84,16 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, } }, [reportActions, parentReportActions, compareImage, report.parentReportActionID, attachments, setDownloadButtonVisibility, onNavigate]); + // Scroll position is affected when window width is resized, so we readjust it on width changes + useEffect(() => { + if (attachments.length === 0 || scrollRef.current == null) { + return; + } + + scrollRef.current.scrollToIndex({index: page, animated: false}); + // The hook is not supposed to run on page change, so we keep the page out of the dependencies + }, [cellWidth]); + /** Updates the page state when the user navigates between attachments */ const updatePage = useCallback( ({viewableItems}: UpdatePageProps) => { From 30893992895a6fccf6933ec4788ea18ef0f76e45 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 7 May 2024 19:52:32 +0300 Subject: [PATCH 5/5] Disable eslint rule for exhaustive-deps in AttachmentCarousel --- src/components/Attachments/AttachmentCarousel/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Attachments/AttachmentCarousel/index.tsx b/src/components/Attachments/AttachmentCarousel/index.tsx index 7b1f0010ebc8..a636e315bf04 100644 --- a/src/components/Attachments/AttachmentCarousel/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/index.tsx @@ -92,6 +92,7 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, scrollRef.current.scrollToIndex({index: page, animated: false}); // The hook is not supposed to run on page change, so we keep the page out of the dependencies + // eslint-disable-next-line react-hooks/exhaustive-deps }, [cellWidth]); /** Updates the page state when the user navigates between attachments */