From 4ce49a31b793f21a671515f2f82c70ac7fad38c2 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Sun, 7 Jan 2024 19:11:54 +0100 Subject: [PATCH 01/33] improve --- .../AttachmentCarousel/CarouselItem.js | 17 +-- .../AttachmentCarousel/Pager/index.js | 106 +++++++++--------- .../AttachmentCarousel/index.native.js | 32 +----- .../AttachmentViewImage/index.js | 21 +--- .../Attachments/AttachmentView/index.js | 8 -- .../Attachments/AttachmentView/propTypes.js | 17 +-- src/components/ImageView/index.native.js | 11 +- src/components/ImageView/propTypes.js | 16 --- src/components/Lightbox.js | 60 +++++----- src/components/MultiGestureCanvas/index.js | 57 ++-------- .../MultiGestureCanvas/propTypes.js | 4 + .../MultiGestureCanvas/usePanGesture.js | 10 +- .../MultiGestureCanvas/usePinchGesture.js | 6 +- .../MultiGestureCanvas/useTapGestures.js | 23 +++- 14 files changed, 144 insertions(+), 244 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/CarouselItem.js b/src/components/Attachments/AttachmentCarousel/CarouselItem.js index 5552f15320f3..abe371768879 100644 --- a/src/components/Attachments/AttachmentCarousel/CarouselItem.js +++ b/src/components/Attachments/AttachmentCarousel/CarouselItem.js @@ -37,15 +37,6 @@ const propTypes = { transactionID: PropTypes.string, }).isRequired, - /** Whether there is only one element in the attachment carousel */ - isSingleItem: PropTypes.bool.isRequired, - - /** The index of the carousel item */ - index: PropTypes.number.isRequired, - - /** The index of the currently active carousel item */ - activeIndex: PropTypes.number.isRequired, - /** onPress callback */ onPress: PropTypes.func, }; @@ -54,7 +45,7 @@ const defaultProps = { onPress: undefined, }; -function CarouselItem({item, index, activeIndex, isSingleItem, onPress}) { +function CarouselItem({item, onPress}) { const styles = useThemeStyles(); const {translate} = useLocalize(); const {isAttachmentHidden} = useContext(ReportAttachmentsContext); @@ -103,12 +94,8 @@ function CarouselItem({item, index, activeIndex, isSingleItem, onPress}) { diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.js b/src/components/Attachments/AttachmentCarousel/Pager/index.js index e0f652e47e4c..84f77dae4953 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.js +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.js @@ -3,8 +3,9 @@ import React, {useEffect, useImperativeHandle, useMemo, useRef, useState} from ' import {View} from 'react-native'; import {createNativeWrapper} from 'react-native-gesture-handler'; import PagerView from 'react-native-pager-view'; -import Animated, {runOnJS, useAnimatedProps, useAnimatedReaction, useEvent, useHandler, useSharedValue} from 'react-native-reanimated'; +import Animated, {runOnJS, useAnimatedReaction, useEvent, useHandler, useSharedValue} from 'react-native-reanimated'; import _ from 'underscore'; +import CarouselItem from '@components/Attachments/AttachmentCarousel/CarouselItem'; import refPropTypes from '@components/refPropTypes'; import useThemeStyles from '@hooks/useThemeStyles'; import AttachmentCarouselPagerContext from './AttachmentCarouselPagerContext'; @@ -36,8 +37,9 @@ const pagerPropTypes = { url: PropTypes.string, }), ).isRequired, - renderItem: PropTypes.func.isRequired, - initialIndex: PropTypes.number, + activeSource: PropTypes.string.isRequired, + initialPage: PropTypes.number, + scrollEnabled: PropTypes.bool, onPageSelected: PropTypes.func, onTap: PropTypes.func, onScaleChanged: PropTypes.func, @@ -45,53 +47,61 @@ const pagerPropTypes = { }; const pagerDefaultProps = { - initialIndex: 0, + initialPage: 0, + scrollEnabled: true, onPageSelected: () => {}, onTap: () => {}, onScaleChanged: () => {}, forwardedRef: null, }; -function AttachmentCarouselPager({items, renderItem, initialIndex, onPageSelected, onTap, onScaleChanged, forwardedRef}) { +function AttachmentCarouselPager({items, activeSource, initialPage, scrollEnabled, onPageSelected, onTap, onScaleChanged, forwardedRef}) { const styles = useThemeStyles(); - const shouldPagerScroll = useSharedValue(true); const pagerRef = useRef(null); - const isSwipingInPager = useSharedValue(false); - const activeIndex = useSharedValue(initialIndex); + const activePage = useSharedValue(initialPage); + const [activePageState, setActivePageState] = useState(initialPage); + // Set active page initially and when initial page changes + useEffect(() => { + setActivePageState(initialPage); + activePage.value = initialPage; + }, [activePage, initialPage]); + + const itemsMeta = useMemo(() => _.map(items, (item, index) => ({source: item.source, index, isActive: index === activePageState})), [activePageState, items]); + + const isPagerSwiping = useSharedValue(false); const pageScrollHandler = usePageScrollHandler( { onPageScroll: (e) => { 'worklet'; - activeIndex.value = e.position; - isSwipingInPager.value = e.offset !== 0; + activePage.value = e.position; + isPagerSwiping.value = e.offset !== 0; }, }, [], ); - const [activePage, setActivePage] = useState(initialIndex); - - useEffect(() => { - setActivePage(initialIndex); - activeIndex.value = initialIndex; - }, [activeIndex, initialIndex]); - - // we use reanimated for this since onPageSelected is called - // in the middle of the pager animation + const [isPagerSwipingState, setPagerSwipingState] = useState(false); useAnimatedReaction( - () => isSwipingInPager.value, - (stillScrolling) => { - if (stillScrolling) { - return; - } - - runOnJS(setActivePage)(activeIndex.value); + () => [isPagerSwiping.value], + (isSwiping) => { + runOnJS(setPagerSwipingState)(isSwiping); }, ); + const contextValue = useMemo( + () => ({ + itemsMeta, + activePage: activePageState, + isPagerSwiping: isPagerSwipingState, + onTap, + onScaleChanged, + }), + [activePageState, isPagerSwipingState, itemsMeta, onScaleChanged, onTap], + ); + useImperativeHandle( forwardedRef, () => ({ @@ -100,19 +110,22 @@ function AttachmentCarouselPager({items, renderItem, initialIndex, onPageSelecte [], ); - const animatedProps = useAnimatedProps(() => ({ - scrollEnabled: shouldPagerScroll.value, - })); - - const contextValue = useMemo( - () => ({ - onTap, - onScaleChanged, - pagerRef, - shouldPagerScroll, - isSwipingInPager, - }), - [isSwipingInPager, shouldPagerScroll, onScaleChanged, onTap], + const Content = useMemo( + () => + _.map(items, (item, index) => ( + + + + )), + [activePageState, activeSource, items, styles.flex1], ); return ( @@ -120,21 +133,14 @@ function AttachmentCarouselPager({items, renderItem, initialIndex, onPageSelecte - {_.map(items, (item, index) => ( - - {renderItem({item, index, isActive: index === activePage})} - - ))} + {Content} ); diff --git a/src/components/Attachments/AttachmentCarousel/index.native.js b/src/components/Attachments/AttachmentCarousel/index.native.js index 8f168093c217..64262b38b0d1 100644 --- a/src/components/Attachments/AttachmentCarousel/index.native.js +++ b/src/components/Attachments/AttachmentCarousel/index.native.js @@ -13,7 +13,6 @@ import variables from '@styles/variables'; import ONYXKEYS from '@src/ONYXKEYS'; import {defaultProps, propTypes} from './attachmentCarouselPropTypes'; import CarouselButtons from './CarouselButtons'; -import CarouselItem from './CarouselItem'; import extractAttachmentsFromReport from './extractAttachmentsFromReport'; import AttachmentCarouselPager from './Pager'; import useCarouselArrows from './useCarouselArrows'; @@ -88,25 +87,6 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, [autoHideArrows, page, updatePage], ); - /** - * Defines how a single attachment should be rendered - * @param {{ reportActionID: String, isAuthTokenRequired: Boolean, source: String, file: { name: String }, hasBeenFlagged: Boolean }} item - * @returns {JSX.Element} - */ - const renderItem = useCallback( - ({item, index, isActive}) => ( - setShouldShowArrows(!shouldShowArrows)} - /> - ), - [activeSource, attachments.length, page, setShouldShowArrows, shouldShowArrows], - ); - const handleScaleChange = useCallback( (newScale) => { const newIsZoomedOut = newScale === 1; @@ -122,11 +102,7 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, ); return ( - setShouldShowArrows(true)} - onMouseLeave={() => setShouldShowArrows(false)} - > + {page == null ? ( ) : ( @@ -152,8 +128,10 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, setShouldShowArrows(!shouldShowArrows)} onPageSelected={({nativeEvent: {position: newPage}}) => updatePage(newPage)} onScaleChanged={handleScaleChange} ref={pagerRef} diff --git a/src/components/Attachments/AttachmentView/AttachmentViewImage/index.js b/src/components/Attachments/AttachmentView/AttachmentViewImage/index.js index f53b993f6053..05c27213557f 100755 --- a/src/components/Attachments/AttachmentView/AttachmentViewImage/index.js +++ b/src/components/Attachments/AttachmentView/AttachmentViewImage/index.js @@ -12,22 +12,7 @@ const propTypes = { ...withLocalizePropTypes, }; -function AttachmentViewImage({ - source, - file, - isAuthTokenRequired, - isUsedInCarousel, - isSingleCarouselItem, - carouselItemIndex, - carouselActiveItemIndex, - isFocused, - loadComplete, - onPress, - onError, - isImage, - onScaleChanged, - translate, -}) { +function AttachmentViewImage({source, file, isAuthTokenRequired, isFocused, loadComplete, onPress, onError, isImage, onScaleChanged, translate}) { const styles = useThemeStyles(); const children = ( ); diff --git a/src/components/Attachments/AttachmentView/index.js b/src/components/Attachments/AttachmentView/index.js index b0060afdb813..179d627a23fa 100755 --- a/src/components/Attachments/AttachmentView/index.js +++ b/src/components/Attachments/AttachmentView/index.js @@ -72,9 +72,6 @@ function AttachmentView({ translate, isFocused, isUsedInCarousel, - isSingleCarouselItem, - carouselItemIndex, - carouselActiveItemIndex, isUsedInAttachmentModal, isWorkspaceAvatar, fallbackSource, @@ -138,8 +135,6 @@ function AttachmentView({ isFocused={isFocused} isAuthTokenRequired={isAuthTokenRequired} encryptedSourceUrl={encryptedSourceUrl} - carouselItemIndex={carouselItemIndex} - carouselActiveItemIndex={carouselActiveItemIndex} onPress={onPress} onScaleChanged={onScaleChanged} onToggleKeyboard={onToggleKeyboard} @@ -169,9 +164,6 @@ function AttachmentView({ loadComplete={loadComplete} isFocused={isFocused} isUsedInCarousel={isUsedInCarousel} - isSingleCarouselItem={isSingleCarouselItem} - carouselItemIndex={carouselItemIndex} - carouselActiveItemIndex={carouselActiveItemIndex} isImage={isImage} onPress={onPress} onScaleChanged={onScaleChanged} diff --git a/src/components/Attachments/AttachmentView/propTypes.js b/src/components/Attachments/AttachmentView/propTypes.js index 286c903ccf5b..31750da53e77 100644 --- a/src/components/Attachments/AttachmentView/propTypes.js +++ b/src/components/Attachments/AttachmentView/propTypes.js @@ -17,18 +17,6 @@ const attachmentViewPropTypes = { /** Whether this AttachmentView is shown as part of a AttachmentCarousel */ isUsedInCarousel: PropTypes.bool, - /** When "isUsedInCarousel" is set to true, determines whether there is only one item in the carousel */ - isSingleCarouselItem: PropTypes.bool, - - /** Whether this AttachmentView is shown as part of an AttachmentModal */ - isUsedInAttachmentModal: PropTypes.bool, - - /** The index of the carousel item */ - carouselItemIndex: PropTypes.number, - - /** The index of the currently active carousel item */ - carouselActiveItemIndex: PropTypes.number, - /** Function for handle on press */ onPress: PropTypes.func, @@ -42,11 +30,8 @@ const attachmentViewDefaultProps = { name: '', }, isFocused: false, - isUsedInCarousel: false, - isSingleCarouselItem: false, - carouselItemIndex: 0, - carouselActiveItemIndex: 0, isSingleElement: false, + isUsedInCarousel: false, isUsedInAttachmentModal: false, onPress: undefined, onScaleChanged: () => {}, diff --git a/src/components/ImageView/index.native.js b/src/components/ImageView/index.native.js index 98349b213aa5..007bc753ec0d 100644 --- a/src/components/ImageView/index.native.js +++ b/src/components/ImageView/index.native.js @@ -11,9 +11,6 @@ const propTypes = { ...imageViewPropTypes, ...zoomRangePropTypes, - /** Function for handle on press */ - onPress: PropTypes.func, - /** Additional styles to add to the component */ style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]), }; @@ -26,20 +23,14 @@ const defaultProps = { style: {}, }; -function ImageView({isAuthTokenRequired, url, onScaleChanged, onPress, style, zoomRange, onError, isUsedInCarousel, isSingleCarouselItem, carouselItemIndex, carouselActiveItemIndex}) { - const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem; - +function ImageView({isAuthTokenRequired, url, onScaleChanged, style, zoomRange, onError}) { return ( ); diff --git a/src/components/ImageView/propTypes.js b/src/components/ImageView/propTypes.js index 3809d9aed043..359626e4bd69 100644 --- a/src/components/ImageView/propTypes.js +++ b/src/components/ImageView/propTypes.js @@ -19,28 +19,12 @@ const imageViewPropTypes = { /** Whether this view is the active screen */ isFocused: PropTypes.bool, - - /** Whether this AttachmentView is shown as part of a AttachmentCarousel */ - isUsedInCarousel: PropTypes.bool, - - /** When "isUsedInCarousel" is set to true, determines whether there is only one item in the carousel */ - isSingleCarouselItem: PropTypes.bool, - - /** The index of the carousel item */ - carouselItemIndex: PropTypes.number, - - /** The index of the currently active carousel item */ - carouselActiveItemIndex: PropTypes.number, }; const imageViewDefaultProps = { isAuthTokenRequired: false, onError: () => {}, isFocused: true, - isUsedInCarousel: false, - isSingleCarouselItem: false, - carouselItemIndex: 0, - carouselActiveItemIndex: 0, }; export {imageViewPropTypes, imageViewDefaultProps}; diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index 366759cc6cd7..6f1f7a0c6437 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -1,8 +1,10 @@ /* eslint-disable es/no-optional-chaining */ +import _ from 'lodash'; import PropTypes from 'prop-types'; -import React, {useCallback, useEffect, useMemo, useState} from 'react'; +import React, {useCallback, useContext, useEffect, useMemo, useState} from 'react'; import {ActivityIndicator, PixelRatio, StyleSheet, View} from 'react-native'; import useStyleUtils from '@hooks/useStyleUtils'; +import AttachmentCarouselPagerContext from './Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext'; import * as AttachmentsPropTypes from './Attachments/propTypes'; import Image from './Image'; import MultiGestureCanvas from './MultiGestureCanvas'; @@ -25,9 +27,6 @@ const propTypes = { /** Triggers whenever the zoom scale changes */ onScaleChanged: PropTypes.func, - /** Function for handle on press */ - onPress: PropTypes.func, - /** Handles errors while displaying the image */ onError: PropTypes.func, @@ -37,15 +36,6 @@ const propTypes = { /** Whether source url requires authentication */ isAuthTokenRequired: PropTypes.bool, - /** Whether the Lightbox is used within a carousel component and there are other sibling elements */ - hasSiblingCarouselItems: PropTypes.bool, - - /** The index of the carousel item */ - index: PropTypes.number, - - /** The index of the currently active carousel item */ - activeIndex: PropTypes.number, - /** Additional styles to add to the component */ style: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.object), PropTypes.object]), }; @@ -54,20 +44,39 @@ const defaultProps = { ...zoomRangeDefaultProps, isAuthTokenRequired: false, - index: 0, - activeIndex: 0, - hasSiblingCarouselItems: false, onScaleChanged: () => {}, - onPress: () => {}, onError: () => {}, style: {}, }; const DEFAULT_IMAGE_SIZE = 200; -function Lightbox({isAuthTokenRequired, source, onScaleChanged, onPress, onError, style, index, activeIndex, hasSiblingCarouselItems, zoomRange}) { +function Lightbox({isAuthTokenRequired, source, onScaleChanged, onError, style, zoomRange}) { const StyleUtils = useStyleUtils(); + const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext); + const {isUsedInCarousel, isSingleCarouselItem, page, activePage, onTap, isPagerSwiping} = useMemo(() => { + if (attachmentCarouselPagerContext == null) { + return { + isUsedInCarousel: false, + isSingleCarouselItem: true, + page: 0, + activePage: 0, + onTap: () => {}, + isPagerSwiping: false, + }; + } + + const foundPageIndex = _.findIndex(attachmentCarouselPagerContext.itemsMeta, (item) => item.source === source); + return { + ...attachmentCarouselPagerContext, + isUsedInCarousel: true, + isSingleCarouselItem: attachmentCarouselPagerContext.itemsMeta.length === 1, + page: foundPageIndex, + }; + }, [attachmentCarouselPagerContext, source]); + const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem; + const [containerSize, setContainerSize] = useState({width: 0, height: 0}); const isContainerLoaded = containerSize.width !== 0 && containerSize.height !== 0; @@ -77,8 +86,8 @@ function Lightbox({isAuthTokenRequired, source, onScaleChanged, onPress, onError cachedDimensions.set(source, newDimensions); }; - const isItemActive = index === activeIndex; - const [isActive, setActive] = useState(isItemActive); + const isActivePage = page === activePage; + const [isActive, setActive] = useState(isActivePage); const [isImageLoaded, setImageLoaded] = useState(false); const isInactiveCarouselItem = hasSiblingCarouselItems && !isActive; @@ -92,9 +101,9 @@ function Lightbox({isAuthTokenRequired, source, onScaleChanged, onPress, onError } const indexCanvasOffset = Math.floor((NUMBER_OF_CONCURRENT_LIGHTBOXES - 1) / 2) || 0; - const indexOutOfRange = index > activeIndex + indexCanvasOffset || index < activeIndex - indexCanvasOffset; + const indexOutOfRange = page > activePage + indexCanvasOffset || page < activePage - indexCanvasOffset; return !indexOutOfRange; - }, [activeIndex, index]); + }, [activePage, page]); const isLightboxVisible = isLightboxInRange && (isActive || isLightboxLoaded || isFallbackLoaded); // If the fallback image is currently visible, we want to hide the Lightbox until the fallback gets hidden, @@ -114,12 +123,12 @@ function Lightbox({isAuthTokenRequired, source, onScaleChanged, onPress, onError // to prevent the image transformer from flashing while still rendering // Instead, we show the fallback image while the image transformer is loading the image useEffect(() => { - if (isItemActive) { + if (isActivePage) { setTimeout(() => setActive(true), 1); } else { setActive(false); } - }, [isItemActive]); + }, [isActivePage]); useEffect(() => { if (isLightboxVisible) { @@ -171,7 +180,6 @@ function Lightbox({isAuthTokenRequired, source, onScaleChanged, onPress, onError return ( @@ -181,6 +189,8 @@ function Lightbox({isAuthTokenRequired, source, onScaleChanged, onPress, onError - attachmentCarouselPagerContext || { - onTap: () => {}, - onScaleChanged: () => {}, - pagerRef: pagerRefFallback, - shouldPagerScroll: false, - isSwipingInPager: false, - }, - [attachmentCarouselPagerContext], - ); - - const onScaleChanged = useCallback( - (newScale) => { - onScaleChangedProp(newScale); - onScaleChangedContext(newScale); - }, - [onScaleChangedContext, onScaleChangedProp], - ); - // Based on the (original) content size and the canvas size, we calculate the horizontal and vertical scale factors // to fit the content inside the canvas // We later use the lower of the two scale factors to fit the content inside the canvas @@ -123,7 +92,8 @@ function MultiGestureCanvas({canvasSize, isActive = true, onScaleChanged: onScal zoomScale.value = 1; }); - const {singleTapGesture: basicSingleTapGesture, doubleTapGesture} = useTapGestures({ + const {singleTapGesture: baseSingleTapGesture, doubleTapGesture} = useTapGestures({ + areTransformationsEnabled, canvasSize, contentSize, minContentScale, @@ -138,9 +108,10 @@ function MultiGestureCanvas({canvasSize, isActive = true, onScaleChanged: onScal onScaleChanged, onTap, }); - const singleTapGesture = basicSingleTapGesture.requireExternalGestureToFail(doubleTapGesture, panGestureRef); + const singleTapGesture = baseSingleTapGesture.requireExternalGestureToFail(doubleTapGesture, panGestureRef); const panGesture = usePanGesture({ + areTransformationsEnabled, canvasSize, contentSize, zoomScale, @@ -149,13 +120,13 @@ function MultiGestureCanvas({canvasSize, isActive = true, onScaleChanged: onScal offsetY, panTranslateX, panTranslateY, - isSwipingInPager, stopAnimation, }) - .simultaneousWithExternalGesture(pagerRef, singleTapGesture, doubleTapGesture) + .simultaneousWithExternalGesture(singleTapGesture, doubleTapGesture) .withRef(panGestureRef); const pinchGesture = usePinchGesture({ + areTransformationsEnabled, canvasSize, zoomScale, zoomRange, @@ -164,20 +135,10 @@ function MultiGestureCanvas({canvasSize, isActive = true, onScaleChanged: onScal pinchTranslateX, pinchTranslateY, pinchScale, - isSwipingInPager, stopAnimation, onScaleChanged, }).simultaneousWithExternalGesture(panGesture, singleTapGesture, doubleTapGesture); - // Enables/disables the pager scroll based on the zoom scale - // When the content is zoomed in/out, the pager should be disabled - useAnimatedReaction( - () => zoomScale.value, - () => { - shouldPagerScroll.value = zoomScale.value === 1; - }, - ); - // Trigger a reset when the canvas gets inactive, but only if it was already mounted before const mounted = useRef(false); useEffect(() => { diff --git a/src/components/MultiGestureCanvas/propTypes.js b/src/components/MultiGestureCanvas/propTypes.js index f1961ec0e156..744dd07db0c8 100644 --- a/src/components/MultiGestureCanvas/propTypes.js +++ b/src/components/MultiGestureCanvas/propTypes.js @@ -29,6 +29,9 @@ const multiGestureCanvasPropTypes = { */ isActive: PropTypes.bool, + /** Whether pan, pinch and double tap transformations are enabled */ + areTransformationsEnabled: PropTypes.bool, + /** Handles scale changed event */ onScaleChanged: PropTypes.func, @@ -64,6 +67,7 @@ const multiGestureCanvasPropTypes = { const multiGestureCanvasDefaultProps = { isActive: true, + areTransformationsEnabled: true, onScaleChanged: () => undefined, contentSize: undefined, contentScaling: undefined, diff --git a/src/components/MultiGestureCanvas/usePanGesture.js b/src/components/MultiGestureCanvas/usePanGesture.js index aec24cb2e99e..27067511a135 100644 --- a/src/components/MultiGestureCanvas/usePanGesture.js +++ b/src/components/MultiGestureCanvas/usePanGesture.js @@ -8,7 +8,7 @@ import * as MultiGestureCanvasUtils from './utils'; // https://docs.swmansion.com/react-native-reanimated/docs/animations/withDecay/ const PAN_DECAY_DECELARATION = 0.9915; -const usePanGesture = ({canvasSize, contentSize, zoomScale, totalScale, offsetX, offsetY, panTranslateX, panTranslateY, isSwipingInPager, stopAnimation}) => { +const usePanGesture = ({areTransformationsEnabled, canvasSize, contentSize, zoomScale, totalScale, offsetX, offsetY, panTranslateX, panTranslateY, stopAnimation}) => { // The content size after fitting it to the canvas and zooming const zoomedContentWidth = useDerivedValue(() => contentSize.width * totalScale.value, [contentSize.width]); const zoomedContentHeight = useDerivedValue(() => contentSize.height * totalScale.value, [contentSize.height]); @@ -107,8 +107,8 @@ const usePanGesture = ({canvasSize, contentSize, zoomScale, totalScale, offsetX, .manualActivation(true) .averageTouches(true) .onTouchesMove((_evt, state) => { - // We only allow panning when the content is zoomed in - if (zoomScale.value <= 1 || isSwipingInPager.value) { + // We only allow panning when the content is zoomed in and the tranformations are enabled + if (zoomScale.value <= 1 || !areTransformationsEnabled) { return; } @@ -137,8 +137,8 @@ const usePanGesture = ({canvasSize, contentSize, zoomScale, totalScale, offsetX, panTranslateX.value = 0; panTranslateY.value = 0; - // If we are swiping (in the pager), we don't want to return to boundaries - if (isSwipingInPager.value) { + // If we the MultiGestureCanvas is disabled, we don't want to return to boundaries + if (!areTransformationsEnabled) { return; } diff --git a/src/components/MultiGestureCanvas/usePinchGesture.js b/src/components/MultiGestureCanvas/usePinchGesture.js index 21c5e55e0117..63f023251aa4 100644 --- a/src/components/MultiGestureCanvas/usePinchGesture.js +++ b/src/components/MultiGestureCanvas/usePinchGesture.js @@ -5,6 +5,7 @@ import {runOnJS, useAnimatedReaction, useSharedValue, withSpring} from 'react-na import * as MultiGestureCanvasUtils from './utils'; const usePinchGesture = ({ + areTransformationsEnabled, canvasSize, zoomScale, zoomRange, @@ -13,7 +14,6 @@ const usePinchGesture = ({ pinchTranslateX: totalPinchTranslateX, pinchTranslateY: totalPinchTranslateY, pinchScale, - isSwipingInPager, stopAnimation, onScaleChanged, }) => { @@ -71,8 +71,8 @@ const usePinchGesture = ({ const pinchGesture = Gesture.Pinch() .enabled(pinchEnabled) .onTouchesDown((_evt, state) => { - // We don't want to activate pinch gesture when we are swiping in the pager - if (!isSwipingInPager.value) { + // We don't want to activate pinch gesture when transformations are disabled + if (!areTransformationsEnabled) { return; } diff --git a/src/components/MultiGestureCanvas/useTapGestures.js b/src/components/MultiGestureCanvas/useTapGestures.js index eefe8c506b33..467b005fb1a2 100644 --- a/src/components/MultiGestureCanvas/useTapGestures.js +++ b/src/components/MultiGestureCanvas/useTapGestures.js @@ -6,7 +6,21 @@ import * as MultiGestureCanvasUtils from './utils'; const DOUBLE_TAP_SCALE = 3; -const useTapGestures = ({canvasSize, contentSize, minContentScale, maxContentScale, offsetX, offsetY, pinchScale, zoomScale, reset, stopAnimation, onScaleChanged, onTap}) => { +const useTapGestures = ({ + areTransformationsEnabled, + canvasSize, + contentSize, + minContentScale, + maxContentScale, + offsetX, + offsetY, + pinchScale, + zoomScale, + reset, + stopAnimation, + onScaleChanged, + onTap, +}) => { // The content size after scaling it with minimum scale to fit the content into the canvas const scaledContentWidth = useMemo(() => contentSize.width * minContentScale, [contentSize.width, minContentScale]); const scaledContentHeight = useMemo(() => contentSize.height * minContentScale, [contentSize.height, minContentScale]); @@ -88,6 +102,13 @@ const useTapGestures = ({canvasSize, contentSize, minContentScale, maxContentSca ); const doubleTapGesture = Gesture.Tap() + .onTouchesDown((_evt, state) => { + if (areTransformationsEnabled) { + return; + } + + state.fail(); + }) .numberOfTaps(2) .maxDelay(150) .maxDistance(20) From 79bfaa3e14810b3729aa25435892bc4a6f971fa4 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Sun, 7 Jan 2024 21:52:00 +0100 Subject: [PATCH 02/33] add callbacks --- src/components/Lightbox.js | 10 ++++----- src/components/MultiGestureCanvas/index.js | 11 +++++----- .../MultiGestureCanvas/usePinchGesture.js | 2 ++ .../MultiGestureCanvas/useTapGestures.js | 22 ++++++++++++------- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index 6f1f7a0c6437..2ce3ca8eb51f 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -24,9 +24,6 @@ const cachedDimensions = new Map(); const propTypes = { ...zoomRangePropTypes, - /** Triggers whenever the zoom scale changes */ - onScaleChanged: PropTypes.func, - /** Handles errors while displaying the image */ onError: PropTypes.func, @@ -51,19 +48,20 @@ const defaultProps = { const DEFAULT_IMAGE_SIZE = 200; -function Lightbox({isAuthTokenRequired, source, onScaleChanged, onError, style, zoomRange}) { +function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { const StyleUtils = useStyleUtils(); const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext); - const {isUsedInCarousel, isSingleCarouselItem, page, activePage, onTap, isPagerSwiping} = useMemo(() => { + const {isUsedInCarousel, isSingleCarouselItem, isPagerSwiping, page, activePage, onTap, onScaleChanged} = useMemo(() => { if (attachmentCarouselPagerContext == null) { return { isUsedInCarousel: false, isSingleCarouselItem: true, + isPagerSwiping: false, page: 0, activePage: 0, onTap: () => {}, - isPagerSwiping: false, + onScaleChanged: () => {}, }; } diff --git a/src/components/MultiGestureCanvas/index.js b/src/components/MultiGestureCanvas/index.js index 049fe85cd53c..300d273c3274 100644 --- a/src/components/MultiGestureCanvas/index.js +++ b/src/components/MultiGestureCanvas/index.js @@ -65,12 +65,11 @@ function MultiGestureCanvas({canvasSize, isActive, areTransformationsEnabled, on /** * Resets the canvas to the initial state and animates back smoothly */ - const reset = MultiGestureCanvasUtils.useWorkletCallback((animated) => { - pinchScale.value = 1; - - stopAnimation(); + const reset = MultiGestureCanvasUtils.useWorkletCallback((animated, callbackProp) => { + const callback = callbackProp || (() => {}); pinchScale.value = 1; + stopAnimation(); if (animated) { offsetX.value = withSpring(0, MultiGestureCanvasUtils.SPRING_CONFIG); @@ -79,7 +78,7 @@ function MultiGestureCanvas({canvasSize, isActive, areTransformationsEnabled, on panTranslateY.value = withSpring(0, MultiGestureCanvasUtils.SPRING_CONFIG); pinchTranslateX.value = withSpring(0, MultiGestureCanvasUtils.SPRING_CONFIG); pinchTranslateY.value = withSpring(0, MultiGestureCanvasUtils.SPRING_CONFIG); - zoomScale.value = withSpring(1, MultiGestureCanvasUtils.SPRING_CONFIG); + zoomScale.value = withSpring(1, MultiGestureCanvasUtils.SPRING_CONFIG, callback); return; } @@ -90,6 +89,8 @@ function MultiGestureCanvas({canvasSize, isActive, areTransformationsEnabled, on pinchTranslateX.value = 0; pinchTranslateY.value = 0; zoomScale.value = 1; + + callback(); }); const {singleTapGesture: baseSingleTapGesture, doubleTapGesture} = useTapGestures({ diff --git a/src/components/MultiGestureCanvas/usePinchGesture.js b/src/components/MultiGestureCanvas/usePinchGesture.js index 63f023251aa4..b363f3cf1218 100644 --- a/src/components/MultiGestureCanvas/usePinchGesture.js +++ b/src/components/MultiGestureCanvas/usePinchGesture.js @@ -141,6 +141,8 @@ const usePinchGesture = ({ } const triggerScaleChangeCallback = () => { + 'worklet'; + if (onScaleChanged == null) { return; } diff --git a/src/components/MultiGestureCanvas/useTapGestures.js b/src/components/MultiGestureCanvas/useTapGestures.js index 467b005fb1a2..ebc573301927 100644 --- a/src/components/MultiGestureCanvas/useTapGestures.js +++ b/src/components/MultiGestureCanvas/useTapGestures.js @@ -29,7 +29,7 @@ const useTapGestures = ({ const doubleTapScale = useMemo(() => Math.max(DOUBLE_TAP_SCALE, maxContentScale / minContentScale), [maxContentScale, minContentScale]); const zoomToCoordinates = MultiGestureCanvasUtils.useWorkletCallback( - (focalX, focalY) => { + (focalX, focalY, callbackProp) => { 'worklet'; stopAnimation(); @@ -93,9 +93,11 @@ const useTapGestures = ({ offsetAfterZooming.y = 0; } + const callback = callbackProp || (() => {}); + offsetX.value = withSpring(offsetAfterZooming.x, MultiGestureCanvasUtils.SPRING_CONFIG); offsetY.value = withSpring(offsetAfterZooming.y, MultiGestureCanvasUtils.SPRING_CONFIG); - zoomScale.value = withSpring(doubleTapScale, MultiGestureCanvasUtils.SPRING_CONFIG); + zoomScale.value = withSpring(doubleTapScale, MultiGestureCanvasUtils.SPRING_CONFIG, callback); pinchScale.value = doubleTapScale; }, [scaledContentWidth, scaledContentHeight, canvasSize, doubleTapScale], @@ -113,16 +115,20 @@ const useTapGestures = ({ .maxDelay(150) .maxDistance(20) .onEnd((evt) => { + const triggerScaleChangedEvent = () => { + 'worklet'; + + if (onScaleChanged != null) { + runOnJS(onScaleChanged)(zoomScale.value); + } + }; + // If the content is already zoomed, we want to reset the zoom, // otherwwise we want to zoom in if (zoomScale.value > 1) { - reset(true); + reset(true, triggerScaleChangedEvent); } else { - zoomToCoordinates(evt.x, evt.y); - } - - if (onScaleChanged != null) { - runOnJS(onScaleChanged)(zoomScale.value); + zoomToCoordinates(evt.x, evt.y, triggerScaleChangedEvent); } }); From 391d0357065a067e2e24ddfc5fabdfe47a83ef65 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Sun, 7 Jan 2024 21:58:33 +0100 Subject: [PATCH 03/33] fix: typo --- src/components/Attachments/AttachmentCarousel/CarouselItem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Attachments/AttachmentCarousel/CarouselItem.js b/src/components/Attachments/AttachmentCarousel/CarouselItem.js index abe371768879..a73b1f58e7b3 100644 --- a/src/components/Attachments/AttachmentCarousel/CarouselItem.js +++ b/src/components/Attachments/AttachmentCarousel/CarouselItem.js @@ -94,7 +94,7 @@ function CarouselItem({item, onPress}) { From be3c6744e9d3c28742104045ed56470fcd084a56 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Sun, 7 Jan 2024 22:00:23 +0100 Subject: [PATCH 04/33] move prop --- src/components/Attachments/AttachmentCarousel/CarouselItem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Attachments/AttachmentCarousel/CarouselItem.js b/src/components/Attachments/AttachmentCarousel/CarouselItem.js index a73b1f58e7b3..878b4eef9539 100644 --- a/src/components/Attachments/AttachmentCarousel/CarouselItem.js +++ b/src/components/Attachments/AttachmentCarousel/CarouselItem.js @@ -94,8 +94,8 @@ function CarouselItem({item, onPress}) { From 30313f21d1a7423985c82c418020dad4baa0ee6b Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Sun, 7 Jan 2024 22:19:13 +0100 Subject: [PATCH 05/33] fix: wrong condition --- src/components/Lightbox.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index 2ce3ca8eb51f..e5c8f044485d 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -187,7 +187,7 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { Date: Sun, 7 Jan 2024 22:24:21 +0100 Subject: [PATCH 06/33] pass pagerRef --- src/components/Lightbox.js | 3 ++- src/components/MultiGestureCanvas/index.js | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index e5c8f044485d..8d3cc474167e 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -52,7 +52,7 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { const StyleUtils = useStyleUtils(); const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext); - const {isUsedInCarousel, isSingleCarouselItem, isPagerSwiping, page, activePage, onTap, onScaleChanged} = useMemo(() => { + const {isUsedInCarousel, isSingleCarouselItem, isPagerSwiping, page, activePage, onTap, onScaleChanged, pagerRef} = useMemo(() => { if (attachmentCarouselPagerContext == null) { return { isUsedInCarousel: false, @@ -193,6 +193,7 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { canvasSize={containerSize} contentSize={imageDimensions?.lightboxSize} zoomRange={zoomRange} + pagerRef={pagerRef} > Date: Sun, 7 Jan 2024 22:24:31 +0100 Subject: [PATCH 07/33] fix: arrow button visibility --- .../Attachments/AttachmentCarousel/index.native.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/Attachments/AttachmentCarousel/index.native.js b/src/components/Attachments/AttachmentCarousel/index.native.js index 64262b38b0d1..3d06ceeaa1ec 100644 --- a/src/components/Attachments/AttachmentCarousel/index.native.js +++ b/src/components/Attachments/AttachmentCarousel/index.native.js @@ -131,7 +131,12 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, initialPage={page} scrollEnabled={isZoomedOut} activeSource={activeSource} - onTap={() => setShouldShowArrows(!shouldShowArrows)} + onTap={() => { + if (!isZoomedOut) { + return; + } + setShouldShowArrows(!shouldShowArrows); + }} onPageSelected={({nativeEvent: {position: newPage}}) => updatePage(newPage)} onScaleChanged={handleScaleChange} ref={pagerRef} From 934bfd3377a385707c8d5c5810f0a5b6c27748f0 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Sun, 7 Jan 2024 22:26:09 +0100 Subject: [PATCH 08/33] pass pagerRef --- src/components/Attachments/AttachmentCarousel/Pager/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.js b/src/components/Attachments/AttachmentCarousel/Pager/index.js index 84f77dae4953..3fd6475f314c 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.js +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.js @@ -98,6 +98,7 @@ function AttachmentCarouselPager({items, activeSource, initialPage, scrollEnable isPagerSwiping: isPagerSwipingState, onTap, onScaleChanged, + pagerRef, }), [activePageState, isPagerSwipingState, itemsMeta, onScaleChanged, onTap], ); From 6be9adbe56b4113f874612167fe86c57dd5e410f Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Sun, 7 Jan 2024 22:46:37 +0100 Subject: [PATCH 09/33] fix: pager swiping --- .../AttachmentCarousel/Pager/index.js | 30 +++++++------------ src/components/Lightbox.js | 2 +- src/components/MultiGestureCanvas/index.js | 8 ++--- .../MultiGestureCanvas/usePanGesture.js | 6 ++-- .../MultiGestureCanvas/usePinchGesture.js | 4 +-- .../MultiGestureCanvas/useTapGestures.js | 4 +-- 6 files changed, 23 insertions(+), 31 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.js b/src/components/Attachments/AttachmentCarousel/Pager/index.js index 3fd6475f314c..ab7c07c1dc30 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.js +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.js @@ -3,7 +3,7 @@ import React, {useEffect, useImperativeHandle, useMemo, useRef, useState} from ' import {View} from 'react-native'; import {createNativeWrapper} from 'react-native-gesture-handler'; import PagerView from 'react-native-pager-view'; -import Animated, {runOnJS, useAnimatedReaction, useEvent, useHandler, useSharedValue} from 'react-native-reanimated'; +import Animated, {useEvent, useHandler, useSharedValue} from 'react-native-reanimated'; import _ from 'underscore'; import CarouselItem from '@components/Attachments/AttachmentCarousel/CarouselItem'; import refPropTypes from '@components/refPropTypes'; @@ -59,18 +59,10 @@ function AttachmentCarouselPager({items, activeSource, initialPage, scrollEnable const styles = useThemeStyles(); const pagerRef = useRef(null); + const isPagerSwiping = useSharedValue(false); const activePage = useSharedValue(initialPage); const [activePageState, setActivePageState] = useState(initialPage); - // Set active page initially and when initial page changes - useEffect(() => { - setActivePageState(initialPage); - activePage.value = initialPage; - }, [activePage, initialPage]); - - const itemsMeta = useMemo(() => _.map(items, (item, index) => ({source: item.source, index, isActive: index === activePageState})), [activePageState, items]); - - const isPagerSwiping = useSharedValue(false); const pageScrollHandler = usePageScrollHandler( { onPageScroll: (e) => { @@ -83,24 +75,24 @@ function AttachmentCarouselPager({items, activeSource, initialPage, scrollEnable [], ); - const [isPagerSwipingState, setPagerSwipingState] = useState(false); - useAnimatedReaction( - () => [isPagerSwiping.value], - (isSwiping) => { - runOnJS(setPagerSwipingState)(isSwiping); - }, - ); + // Set active page initially and when initial page changes + useEffect(() => { + setActivePageState(initialPage); + activePage.value = initialPage; + }, [activePage, initialPage]); + + const itemsMeta = useMemo(() => _.map(items, (item, index) => ({source: item.source, index, isActive: index === activePageState})), [activePageState, items]); const contextValue = useMemo( () => ({ itemsMeta, activePage: activePageState, - isPagerSwiping: isPagerSwipingState, + isPagerSwiping, onTap, onScaleChanged, pagerRef, }), - [activePageState, isPagerSwipingState, itemsMeta, onScaleChanged, onTap], + [activePageState, isPagerSwiping, itemsMeta, onScaleChanged, onTap], ); useImperativeHandle( diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index 8d3cc474167e..17d249113eb7 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -187,13 +187,13 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { { +const usePanGesture = ({canvasSize, contentSize, zoomScale, totalScale, offsetX, offsetY, panTranslateX, panTranslateY, stopAnimation, isPagerSwiping}) => { // The content size after fitting it to the canvas and zooming const zoomedContentWidth = useDerivedValue(() => contentSize.width * totalScale.value, [contentSize.width]); const zoomedContentHeight = useDerivedValue(() => contentSize.height * totalScale.value, [contentSize.height]); @@ -108,7 +108,7 @@ const usePanGesture = ({areTransformationsEnabled, canvasSize, contentSize, zoom .averageTouches(true) .onTouchesMove((_evt, state) => { // We only allow panning when the content is zoomed in and the tranformations are enabled - if (zoomScale.value <= 1 || !areTransformationsEnabled) { + if (zoomScale.value <= 1 || isPagerSwiping.value) { return; } @@ -138,7 +138,7 @@ const usePanGesture = ({areTransformationsEnabled, canvasSize, contentSize, zoom panTranslateY.value = 0; // If we the MultiGestureCanvas is disabled, we don't want to return to boundaries - if (!areTransformationsEnabled) { + if (isPagerSwiping.value) { return; } diff --git a/src/components/MultiGestureCanvas/usePinchGesture.js b/src/components/MultiGestureCanvas/usePinchGesture.js index b363f3cf1218..73a51dc96087 100644 --- a/src/components/MultiGestureCanvas/usePinchGesture.js +++ b/src/components/MultiGestureCanvas/usePinchGesture.js @@ -5,7 +5,6 @@ import {runOnJS, useAnimatedReaction, useSharedValue, withSpring} from 'react-na import * as MultiGestureCanvasUtils from './utils'; const usePinchGesture = ({ - areTransformationsEnabled, canvasSize, zoomScale, zoomRange, @@ -16,6 +15,7 @@ const usePinchGesture = ({ pinchScale, stopAnimation, onScaleChanged, + isPagerSwiping, }) => { // The current pinch gesture event scale const currentPinchScale = useSharedValue(1); @@ -72,7 +72,7 @@ const usePinchGesture = ({ .enabled(pinchEnabled) .onTouchesDown((_evt, state) => { // We don't want to activate pinch gesture when transformations are disabled - if (!areTransformationsEnabled) { + if (!isPagerSwiping.value) { return; } diff --git a/src/components/MultiGestureCanvas/useTapGestures.js b/src/components/MultiGestureCanvas/useTapGestures.js index ebc573301927..18b67a2b2fe0 100644 --- a/src/components/MultiGestureCanvas/useTapGestures.js +++ b/src/components/MultiGestureCanvas/useTapGestures.js @@ -7,7 +7,6 @@ import * as MultiGestureCanvasUtils from './utils'; const DOUBLE_TAP_SCALE = 3; const useTapGestures = ({ - areTransformationsEnabled, canvasSize, contentSize, minContentScale, @@ -20,6 +19,7 @@ const useTapGestures = ({ stopAnimation, onScaleChanged, onTap, + isPagerSwiping, }) => { // The content size after scaling it with minimum scale to fit the content into the canvas const scaledContentWidth = useMemo(() => contentSize.width * minContentScale, [contentSize.width, minContentScale]); @@ -105,7 +105,7 @@ const useTapGestures = ({ const doubleTapGesture = Gesture.Tap() .onTouchesDown((_evt, state) => { - if (areTransformationsEnabled) { + if (!isPagerSwiping.value) { return; } From 3376f869266fb598ee4fb8ff0d027a5e1ac72f44 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 8 Jan 2024 09:41:09 +0100 Subject: [PATCH 10/33] generalize MultiGestureCanvas props --- src/components/Lightbox.js | 4 ++-- src/components/MultiGestureCanvas/index.js | 23 +++++++++++++++---- .../MultiGestureCanvas/usePanGesture.js | 6 ++--- .../MultiGestureCanvas/usePinchGesture.js | 4 ++-- .../MultiGestureCanvas/useTapGestures.js | 4 ++-- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index 17d249113eb7..e301e94b5200 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -192,8 +192,8 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { canvasSize={containerSize} contentSize={imageDimensions?.lightboxSize} zoomRange={zoomRange} - pagerRef={pagerRef} - isPagerSwiping={isPagerSwiping} + externalGestureRef={pagerRef} + shouldDisableTransformationGestures={isPagerSwiping} > { +const usePanGesture = ({canvasSize, contentSize, zoomScale, totalScale, offsetX, offsetY, panTranslateX, panTranslateY, stopAnimation, shouldDisableTransformationGestures}) => { // The content size after fitting it to the canvas and zooming const zoomedContentWidth = useDerivedValue(() => contentSize.width * totalScale.value, [contentSize.width]); const zoomedContentHeight = useDerivedValue(() => contentSize.height * totalScale.value, [contentSize.height]); @@ -108,7 +108,7 @@ const usePanGesture = ({canvasSize, contentSize, zoomScale, totalScale, offsetX, .averageTouches(true) .onTouchesMove((_evt, state) => { // We only allow panning when the content is zoomed in and the tranformations are enabled - if (zoomScale.value <= 1 || isPagerSwiping.value) { + if (zoomScale.value <= 1 || shouldDisableTransformationGestures.value) { return; } @@ -138,7 +138,7 @@ const usePanGesture = ({canvasSize, contentSize, zoomScale, totalScale, offsetX, panTranslateY.value = 0; // If we the MultiGestureCanvas is disabled, we don't want to return to boundaries - if (isPagerSwiping.value) { + if (shouldDisableTransformationGestures.value) { return; } diff --git a/src/components/MultiGestureCanvas/usePinchGesture.js b/src/components/MultiGestureCanvas/usePinchGesture.js index 73a51dc96087..154a232f584d 100644 --- a/src/components/MultiGestureCanvas/usePinchGesture.js +++ b/src/components/MultiGestureCanvas/usePinchGesture.js @@ -15,7 +15,7 @@ const usePinchGesture = ({ pinchScale, stopAnimation, onScaleChanged, - isPagerSwiping, + shouldDisableTransformationGestures, }) => { // The current pinch gesture event scale const currentPinchScale = useSharedValue(1); @@ -72,7 +72,7 @@ const usePinchGesture = ({ .enabled(pinchEnabled) .onTouchesDown((_evt, state) => { // We don't want to activate pinch gesture when transformations are disabled - if (!isPagerSwiping.value) { + if (!shouldDisableTransformationGestures.value) { return; } diff --git a/src/components/MultiGestureCanvas/useTapGestures.js b/src/components/MultiGestureCanvas/useTapGestures.js index 18b67a2b2fe0..0a97ce379f76 100644 --- a/src/components/MultiGestureCanvas/useTapGestures.js +++ b/src/components/MultiGestureCanvas/useTapGestures.js @@ -19,7 +19,7 @@ const useTapGestures = ({ stopAnimation, onScaleChanged, onTap, - isPagerSwiping, + shouldDisableTransformationGestures, }) => { // The content size after scaling it with minimum scale to fit the content into the canvas const scaledContentWidth = useMemo(() => contentSize.width * minContentScale, [contentSize.width, minContentScale]); @@ -105,7 +105,7 @@ const useTapGestures = ({ const doubleTapGesture = Gesture.Tap() .onTouchesDown((_evt, state) => { - if (!isPagerSwiping.value) { + if (!shouldDisableTransformationGestures.value) { return; } From 7195fdf84ae74034c8d97064c9487e4034989477 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 8 Jan 2024 10:52:15 +0100 Subject: [PATCH 11/33] add more improvements to Lightbox --- src/components/ImageView/index.native.js | 2 +- src/components/Lightbox.js | 142 ++++++++++----------- src/components/MultiGestureCanvas/utils.ts | 23 +++- 3 files changed, 90 insertions(+), 77 deletions(-) diff --git a/src/components/ImageView/index.native.js b/src/components/ImageView/index.native.js index 007bc753ec0d..9189548ce1a7 100644 --- a/src/components/ImageView/index.native.js +++ b/src/components/ImageView/index.native.js @@ -26,7 +26,7 @@ const defaultProps = { function ImageView({isAuthTokenRequired, url, onScaleChanged, style, zoomRange, onError}) { return ( setCanvasSize({width: PixelRatio.roundToNearestPixel(width), height: PixelRatio.roundToNearestPixel(height)}), + [], + ); + + const [contentSize, setInternalContentSize] = useState(() => cachedImageDimensions.get(uri)); + const setContentSize = useCallback( + (newDimensions) => { + setInternalContentSize(newDimensions); + cachedImageDimensions.set(uri, newDimensions); + }, + [uri], + ); + const updateContentSize = useCallback(({nativeEvent: {width, height}}) => setContentSize({width: width * PixelRatio.get(), height: height * PixelRatio.get()}), [setContentSize]); + const contentLoaded = contentSize != null; + const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext); const {isUsedInCarousel, isSingleCarouselItem, isPagerSwiping, page, activePage, onTap, onScaleChanged, pagerRef} = useMemo(() => { if (attachmentCarouselPagerContext == null) { @@ -65,34 +90,35 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { }; } - const foundPageIndex = _.findIndex(attachmentCarouselPagerContext.itemsMeta, (item) => item.source === source); + const foundPageIndex = _.findIndex(attachmentCarouselPagerContext.itemsMeta, (item) => item.source === uri); return { ...attachmentCarouselPagerContext, isUsedInCarousel: true, isSingleCarouselItem: attachmentCarouselPagerContext.itemsMeta.length === 1, page: foundPageIndex, }; - }, [attachmentCarouselPagerContext, source]); + }, [attachmentCarouselPagerContext, uri]); const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem; - const [containerSize, setContainerSize] = useState({width: 0, height: 0}); - const isContainerLoaded = containerSize.width !== 0 && containerSize.height !== 0; - - const [imageDimensions, _setImageDimensions] = useState(() => cachedDimensions.get(source)); - const setImageDimensions = (newDimensions) => { - _setImageDimensions(newDimensions); - cachedDimensions.set(source, newDimensions); - }; - const isActivePage = page === activePage; const [isActive, setActive] = useState(isActivePage); - const [isImageLoaded, setImageLoaded] = useState(false); const isInactiveCarouselItem = hasSiblingCarouselItems && !isActive; const [isFallbackVisible, setFallbackVisible] = useState(isInactiveCarouselItem); - const [isFallbackLoaded, setFallbackLoaded] = useState(false); + const [isFallbackImageLoaded, setFallbackImageLoaded] = useState(false); + const fallbackSize = useMemo(() => { + if (!hasSiblingCarouselItems || contentSize == null || canvasSize.width === 0 || canvasSize.height === 0) { + return DEFAULT_IMAGE_DIMENSIONS; + } + + const {minScale} = MultiGestureCanvasUtils.getCanvasFitScale({canvasSize, contentSize}); + + return { + width: PixelRatio.roundToNearestPixel(contentSize.width * minScale), + height: PixelRatio.roundToNearestPixel(contentSize.height * minScale), + }; + }, [canvasSize, hasSiblingCarouselItems, contentSize]); - const isLightboxLoaded = imageDimensions?.lightboxSize != null; const isLightboxInRange = useMemo(() => { if (NUMBER_OF_CONCURRENT_LIGHTBOXES === -1) { return true; @@ -102,20 +128,17 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { const indexOutOfRange = page > activePage + indexCanvasOffset || page < activePage - indexCanvasOffset; return !indexOutOfRange; }, [activePage, page]); - const isLightboxVisible = isLightboxInRange && (isActive || isLightboxLoaded || isFallbackLoaded); + const [isLightboxImageLoaded, setLightboxImageLoaded] = useState(false); + const isLightboxVisible = isLightboxInRange && (isActive || isLightboxImageLoaded || isFallbackImageLoaded); // If the fallback image is currently visible, we want to hide the Lightbox until the fallback gets hidden, // so that we don't see two overlapping images at the same time. + // We cannot NOT render it, because we need to render the Lightbox to get the correct dimensions for the fallback image. // If there the Lightbox is not used within a carousel, we don't need to hide the Lightbox, // because it's only going to be rendered after the fallback image is hidden. const shouldHideLightbox = hasSiblingCarouselItems && isFallbackVisible; - const isLoading = isActive && (!isContainerLoaded || !isImageLoaded); - - const updateCanvasSize = useCallback( - ({nativeEvent}) => setContainerSize({width: PixelRatio.roundToNearestPixel(nativeEvent.layout.width), height: PixelRatio.roundToNearestPixel(nativeEvent.layout.height)}), - [], - ); + const isLoading = isActive && (!isCanvasLoaded || !contentLoaded); // We delay setting a page to active state by a (few) millisecond(s), // to prevent the image transformer from flashing while still rendering @@ -132,8 +155,8 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { if (isLightboxVisible) { return; } - setImageLoaded(false); - }, [isLightboxVisible]); + setContentSize(undefined); + }, [isLightboxVisible, setContentSize]); useEffect(() => { if (!hasSiblingCarouselItems) { @@ -141,47 +164,29 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { } if (isActive) { - if (isImageLoaded && isFallbackVisible) { + if (contentLoaded && isFallbackVisible) { // We delay hiding the fallback image while image transformer is still rendering setTimeout(() => { setFallbackVisible(false); - setFallbackLoaded(false); + setFallbackImageLoaded(false); }, 100); } } else { - if (isLightboxVisible && isLightboxLoaded) { + if (isLightboxVisible && isLightboxImageLoaded) { return; } // Show fallback when the image goes out of focus or when the image is loading setFallbackVisible(true); } - }, [hasSiblingCarouselItems, isActive, isImageLoaded, isFallbackVisible, isLightboxLoaded, isLightboxVisible]); - - const fallbackSize = useMemo(() => { - if (!hasSiblingCarouselItems || (imageDimensions?.lightboxSize == null && imageDimensions?.fallbackSize == null) || containerSize.width === 0 || containerSize.height === 0) { - return { - width: DEFAULT_IMAGE_SIZE, - height: DEFAULT_IMAGE_SIZE, - }; - } - - const imageSize = imageDimensions.lightboxSize || imageDimensions.fallbackSize; - - const {minScale} = getCanvasFitScale({canvasSize: containerSize, contentSize: imageSize}); - - return { - width: PixelRatio.roundToNearestPixel(imageSize.width * minScale), - height: PixelRatio.roundToNearestPixel(imageSize.height * minScale), - }; - }, [containerSize, hasSiblingCarouselItems, imageDimensions]); + }, [hasSiblingCarouselItems, isActive, isFallbackVisible, isLightboxVisible, contentLoaded, isLightboxImageLoaded]); return ( - {isContainerLoaded && ( + {isCanvasLoaded && ( <> {isLightboxVisible && ( @@ -189,23 +194,19 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { isActive={isActive} onTap={onTap} onScaleChanged={onScaleChanged} - canvasSize={containerSize} - contentSize={imageDimensions?.lightboxSize} + canvasSize={canvasSize} + contentSize={contentSize} zoomRange={zoomRange} externalGestureRef={pagerRef} shouldDisableTransformationGestures={isPagerSwiping} > setImageLoaded(true)} - onLoad={(e) => { - const width = (e.nativeEvent?.width || 0) * PixelRatio.get(); - const height = (e.nativeEvent?.height || 0) * PixelRatio.get(); - setImageDimensions({...imageDimensions, lightboxSize: {width, height}}); - }} + onLoad={updateContentSize} + onLoadEnd={() => setLightboxImageLoaded(true)} /> @@ -215,21 +216,12 @@ function Lightbox({isAuthTokenRequired, source, onError, style, zoomRange}) { {isFallbackVisible && ( setFallbackLoaded(true)} - onLoad={(e) => { - const width = (e.nativeEvent?.width || 0) * PixelRatio.get(); - const height = (e.nativeEvent?.height || 0) * PixelRatio.get(); - - if (imageDimensions?.lightboxSize != null) { - return; - } - - setImageDimensions({...imageDimensions, fallbackSize: {width, height}}); - }} + onLoad={updateContentSize} + onLoadEnd={() => setFallbackImageLoaded(true)} /> )} diff --git a/src/components/MultiGestureCanvas/utils.ts b/src/components/MultiGestureCanvas/utils.ts index 5cddd009117a..d61eb8c6efcd 100644 --- a/src/components/MultiGestureCanvas/utils.ts +++ b/src/components/MultiGestureCanvas/utils.ts @@ -43,4 +43,25 @@ const useWorkletCallback = (callback: Parameters[0], deps: P return useCallback(callback, deps); }; -export {SPRING_CONFIG, zoomScaleBounceFactors, clamp, useWorkletCallback}; +type GetCanvasFitScale = (props: { + canvasSize: { + width: number; + height: number; + }; + contentSize: { + width: number; + height: number; + }; +}) => {scaleX: number; scaleY: number; minScale: number; maxScale: number}; + +const getCanvasFitScale: GetCanvasFitScale = ({canvasSize, contentSize}) => { + const scaleX = canvasSize.width / contentSize.width; + const scaleY = canvasSize.height / contentSize.height; + + const minScale = Math.min(scaleX, scaleY); + const maxScale = Math.max(scaleX, scaleY); + + return {scaleX, scaleY, minScale, maxScale}; +}; + +export {SPRING_CONFIG, zoomScaleBounceFactors, clamp, useWorkletCallback, getCanvasFitScale}; From c5669ddbd97b2e4068157ff225b6410bf4290b0c Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 8 Jan 2024 10:55:40 +0100 Subject: [PATCH 12/33] rename prop --- src/components/Lightbox.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index c456929ee3f4..adc5ff1c3976 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -67,9 +67,9 @@ function Lightbox({isAuthTokenRequired, uri, onError, style, zoomRange}) { const [contentSize, setInternalContentSize] = useState(() => cachedImageDimensions.get(uri)); const setContentSize = useCallback( - (newDimensions) => { - setInternalContentSize(newDimensions); - cachedImageDimensions.set(uri, newDimensions); + (newContentSize) => { + setInternalContentSize(newContentSize); + cachedImageDimensions.set(uri, newContentSize); }, [uri], ); From 2cb2e3c7ea7c890b8b55a9d2b626659866d894be Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 8 Jan 2024 11:45:56 +0100 Subject: [PATCH 13/33] rename variable --- src/components/Lightbox.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index adc5ff1c3976..14db04d328cd 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -90,12 +90,12 @@ function Lightbox({isAuthTokenRequired, uri, onError, style, zoomRange}) { }; } - const foundPageIndex = _.findIndex(attachmentCarouselPagerContext.itemsMeta, (item) => item.source === uri); + const foundPage = _.findIndex(attachmentCarouselPagerContext.itemsMeta, (item) => item.source === uri); return { ...attachmentCarouselPagerContext, isUsedInCarousel: true, isSingleCarouselItem: attachmentCarouselPagerContext.itemsMeta.length === 1, - page: foundPageIndex, + page: foundPage, }; }, [attachmentCarouselPagerContext, uri]); const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem; From e4d30dd20588877b28318bcc9d4a716be8cadba6 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 8 Jan 2024 11:50:05 +0100 Subject: [PATCH 14/33] update prop types --- .../MultiGestureCanvas/propTypes.js | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/components/MultiGestureCanvas/propTypes.js b/src/components/MultiGestureCanvas/propTypes.js index 744dd07db0c8..a72d2364e0a5 100644 --- a/src/components/MultiGestureCanvas/propTypes.js +++ b/src/components/MultiGestureCanvas/propTypes.js @@ -1,4 +1,5 @@ import PropTypes from 'prop-types'; +import refPropTypes from '@components/refPropTypes'; const defaultZoomRange = { min: 1, @@ -24,35 +25,35 @@ const multiGestureCanvasPropTypes = { ...zoomRangePropTypes, /** - * Wheter the canvas is currently active (in the screen) or not. - * Disables certain gestures and functionality + * Wheter the canvas is currently active (in the screen) or not. + * Disables certain gestures and functionality */ isActive: PropTypes.bool, - /** Whether pan, pinch and double tap transformations are enabled */ - areTransformationsEnabled: PropTypes.bool, - /** Handles scale changed event */ onScaleChanged: PropTypes.func, - /** The width and height of the canvas. - * This is needed in order to properly scale the content in the canvas + /** + * The width and height of the canvas. + * This is needed in order to properly scale the content in the canvas */ canvasSize: PropTypes.shape({ width: PropTypes.number.isRequired, height: PropTypes.number.isRequired, }).isRequired, - /** The width and height of the content. - * This is needed in order to properly scale the content in the canvas + /** + * The width and height of the content. + * This is needed in order to properly scale the content in the canvas */ contentSize: PropTypes.shape({ width: PropTypes.number, height: PropTypes.number, }), - /** The scale factors (scaleX, scaleY) that are used to scale the content (width/height) to the canvas size. - * `scaledWidth` and `scaledHeight` reflect the actual size of the content after scaling. + /** + * The scale factors (scaleX, scaleY) that are used to scale the content (width/height) to the canvas size. + * `scaledWidth` and `scaledHeight` reflect the actual size of the content after scaling. */ contentScaling: PropTypes.shape({ scaleX: PropTypes.number, @@ -61,6 +62,17 @@ const multiGestureCanvasPropTypes = { scaledHeight: PropTypes.number, }), + /** + * A shared value of type boolean, that indicates disabled the transformation gestures (pinch, pan, double tap) + */ + shouldDisableTransformationGestures: PropTypes.shape({value: PropTypes.bool}), + + /** + * A ref to an external gesture handler, like a PagerView from `react-native-pager-view` + * Used to disable the pan, pinch and double tap gesture when the user is swiping between pages + */ + externalGestureRef: refPropTypes, + /** Content that should be transformed inside the canvas (images, pdf, ...) */ children: PropTypes.node.isRequired, }; From d469354122ae2d9cfa8dc442b0a6008b8ba51e4a Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 8 Jan 2024 12:01:35 +0100 Subject: [PATCH 15/33] fix: propType --- src/components/Lightbox.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/Lightbox.js b/src/components/Lightbox.js index 14db04d328cd..455afaa24b44 100644 --- a/src/components/Lightbox.js +++ b/src/components/Lightbox.js @@ -5,7 +5,6 @@ import React, {useCallback, useContext, useEffect, useMemo, useState} from 'reac import {ActivityIndicator, PixelRatio, StyleSheet, View} from 'react-native'; import useStyleUtils from '@hooks/useStyleUtils'; import AttachmentCarouselPagerContext from './Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext'; -import * as AttachmentsPropTypes from './Attachments/propTypes'; import Image from './Image'; import MultiGestureCanvas from './MultiGestureCanvas'; import {zoomRangeDefaultProps, zoomRangePropTypes} from './MultiGestureCanvas/propTypes'; @@ -33,7 +32,7 @@ const propTypes = { onError: PropTypes.func, /** URI to full-sized attachment, SVG function, or numeric static image on native platforms */ - uri: AttachmentsPropTypes.string.isRequired, + uri: PropTypes.string.isRequired, /** Whether source url requires authentication */ isAuthTokenRequired: PropTypes.bool, From 5c11456f5d0581fb888d7d1fdd489f40fce6bcc4 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 16 Jan 2024 16:54:44 +0100 Subject: [PATCH 16/33] change to pagerRef --- src/components/MultiGestureCanvas/index.js | 13 ++----------- src/components/MultiGestureCanvas/propTypes.js | 7 ++----- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/components/MultiGestureCanvas/index.js b/src/components/MultiGestureCanvas/index.js index 07bb3323fcb7..f4c04a0b9ce2 100644 --- a/src/components/MultiGestureCanvas/index.js +++ b/src/components/MultiGestureCanvas/index.js @@ -25,16 +25,7 @@ function getDeepDefaultProps({contentSize: contentSizeProp = {}, zoomRange: zoom return {contentSize, zoomRange}; } -function MultiGestureCanvas({ - canvasSize, - isActive, - onScaleChanged, - onTap, - children, - shouldDisableTransformationGestures: shouldDisableTransformationGesturesProp, - externalGestureRef, - ...props -}) { +function MultiGestureCanvas({canvasSize, isActive, onScaleChanged, onTap, children, shouldDisableTransformationGestures: shouldDisableTransformationGesturesProp, pagerRef, ...props}) { const styles = useThemeStyles(); const StyleUtils = useStyleUtils(); @@ -136,7 +127,7 @@ function MultiGestureCanvas({ stopAnimation, shouldDisableTransformationGestures, }) - .simultaneousWithExternalGesture(externalGestureRef, singleTapGesture, doubleTapGesture) + .simultaneousWithExternalGesture(pagerRef, singleTapGesture, doubleTapGesture) .withRef(panGestureRef); const pinchGesture = usePinchGesture({ diff --git a/src/components/MultiGestureCanvas/propTypes.js b/src/components/MultiGestureCanvas/propTypes.js index c249ec9518f4..2593c4447b61 100644 --- a/src/components/MultiGestureCanvas/propTypes.js +++ b/src/components/MultiGestureCanvas/propTypes.js @@ -61,11 +61,8 @@ const multiGestureCanvasPropTypes = { */ shouldDisableTransformationGestures: PropTypes.shape({value: PropTypes.bool}), - /** - * A ref to an external gesture handler, like a PagerView from `react-native-pager-view` - * Used to disable the pan, pinch and double tap gesture when the user is swiping between pages - */ - externalGestureRef: refPropTypes, + /** If there is a pager wrapping the canvas, we need to disable the pan gesture in case the pager is swiping */ + pagerRef: refPropTypes, // TODO: For TS migration: Exclude /** Content that should be transformed inside the canvas (images, pdf, ...) */ children: PropTypes.node.isRequired, From 500c5cdcf6e985dc0d63878cbf80fb360e6b2bc6 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 29 Jan 2024 16:58:02 +0100 Subject: [PATCH 17/33] fix: AttachmentView --- src/components/Attachments/AttachmentCarousel/Pager/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index 6641e0f6d687..484bdfe28daf 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -134,6 +134,7 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect style={styles.flex1} > Date: Mon, 29 Jan 2024 22:24:08 +0100 Subject: [PATCH 18/33] rename context variable --- .../Pager/AttachmentCarouselPagerContext.ts | 2 +- .../Attachments/AttachmentCarousel/Pager/index.tsx | 9 ++++++--- src/components/Lightbox/index.tsx | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts index c13af8eeb73b..5b1994d46e14 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts +++ b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts @@ -10,7 +10,7 @@ type AttachmentCarouselItemMeta = { }; type AttachmentCarouselPagerContextValue = { - itemsMeta: AttachmentCarouselItemMeta[]; + pagerItems: AttachmentCarouselItemMeta[]; activePage: number; pagerRef: ForwardedRef; isPagerScrolling: SharedValue; diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index 484bdfe28daf..6dfc434f6ed8 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -57,7 +57,10 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect activePage.value = initialPage; }, [activePage, initialPage]); - const itemsMeta = useMemo(() => items.map((item, index) => ({source: item.source, index, isActive: index === activePageState})), [activePageState, items]); + /** + * The pager uses the source index and current active state to render the pages. + */ + const pagerItems = useMemo(() => items.map((item, index) => ({source: item.source, index, isActive: index === activePageState})), [activePageState, items]); /** * This callback is passed to the MultiGestureCanvas/Lightbox through the AttachmentCarouselPagerContext. @@ -97,7 +100,7 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect const contextValue = useMemo( () => ({ - itemsMeta, + pagerItems, activePage: activePageState, isPagerScrolling, isScrollEnabled, @@ -105,7 +108,7 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect onTap: handleTap, onScaleChanged: handleScaleChange, }), - [itemsMeta, activePageState, isPagerScrolling, isScrollEnabled, handleTap, handleScaleChange], + [pagerItems, activePageState, isPagerScrolling, isScrollEnabled, handleTap, handleScaleChange], ); const animatedProps = useAnimatedProps(() => ({ diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index 638738a5c9b9..a1752062d03f 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -69,11 +69,11 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan }; } - const foundPage = attachmentCarouselPagerContext.itemsMeta.findIndex((item) => item.source === uri); + const foundPage = attachmentCarouselPagerContext.pagerItems.findIndex((item) => item.source === uri); return { ...attachmentCarouselPagerContext, isUsedInCarousel: true, - isSingleCarouselItem: attachmentCarouselPagerContext.itemsMeta.length === 1, + isSingleCarouselItem: attachmentCarouselPagerContext.pagerItems.length === 1, page: foundPage, }; }, [attachmentCarouselPagerContext, isPagerScrollingFallback, uri]); From c0b21fc1b4324cb0e96e5e6e7532c3d9590208bd Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 29 Jan 2024 22:52:45 +0100 Subject: [PATCH 19/33] improve comments --- .../Pager/AttachmentCarouselPagerContext.ts | 16 ++++++++++++-- .../AttachmentCarousel/Pager/index.tsx | 22 +++++++++++++++---- src/components/Lightbox/index.tsx | 10 +++++++-- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts index 5b1994d46e14..32f4a490edba 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts +++ b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts @@ -3,14 +3,26 @@ import {createContext} from 'react'; import type PagerView from 'react-native-pager-view'; import type {SharedValue} from 'react-native-reanimated'; -type AttachmentCarouselItemMeta = { +/** + * The pager items array is used within the pager to render and navigate between the images + */ +type AttachmentCarouselPagerItems = { + /** + * The source of the image is used to identify each attachment/page in the pager + */ source: string; + /** + * The index of the pager item determines the order of the images in the pager + */ index: number; + /** + * The active state of the pager item determines whether the image is currently transformable with pinch, pan and tap gestures + */ isActive: boolean; }; type AttachmentCarouselPagerContextValue = { - pagerItems: AttachmentCarouselItemMeta[]; + pagerItems: AttachmentCarouselPagerItems[]; activePage: number; pagerRef: ForwardedRef; isPagerScrolling: SharedValue; diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index 6dfc434f6ed8..b4c4aa0f192d 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -20,17 +20,31 @@ type AttachmentCarouselPagerHandle = { setPage: (selectedPage: number) => void; }; -type PagerItem = { - key: string; - url: string; +type Attachment = { source: string; }; type AttachmentCarouselPagerProps = { - items: PagerItem[]; + /** + * The attachments to be rendered in the pager. + */ + items: Attachment[]; + /** + * The source of the currently active attachment. + */ activeSource: string; + /** + * The index of the initial page to be rendered. + */ initialPage: number; + /** + * A callback to be called when the page is changed. + */ onPageSelected: () => void; + /** + * A callback that can be used to toggle the attachment carousel arrows, when the scale of the image changes. + * @param showArrows If set, it will show/hide the arrows. If not set, it will toggle the arrows. + */ onRequestToggleArrows: (showArrows?: boolean) => void; }; diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index a1752062d03f..cbd49835d2a6 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -43,6 +43,10 @@ type LightboxProps = { function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChangedProp, onError, style, zoomRange = DEFAULT_ZOOM_RANGE}: LightboxProps) { const StyleUtils = useStyleUtils(); + /** React hooks must be used in the render function of the component at top-level and unconditionally. + * Therefore, in order to provide a default value for "isPagerScrolling" if the "AttachmentCarouselPagerContext" is not available, + * we need to create a shared value that can be used in the render function. + * */ const isPagerScrollingFallback = useSharedValue(false); const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext); @@ -77,6 +81,8 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan page: foundPage, }; }, [attachmentCarouselPagerContext, isPagerScrollingFallback, uri]); + + /** Whether the Lightbox is used within an attachmnet carousel and there are more than one page in the carousel */ const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem; const isActive = page === activePage; @@ -179,7 +185,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan } }, [hasSiblingCarouselItems, isActive, isFallbackVisible, isLightboxImageLoaded, isLightboxVisible]); - const handleScalChange = useCallback( + const handleScaleChange = useCallback( (scale: number) => { onScaleChangedProp?.(scale); onScaleChangedContext?.(scale); @@ -204,7 +210,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan pagerRef={pagerRef} shouldDisableTransformationGestures={isPagerScrolling} onTap={onTap} - onScaleChanged={handleScalChange} + onScaleChanged={handleScaleChange} > Date: Wed, 7 Feb 2024 16:26:25 +0100 Subject: [PATCH 20/33] Update src/components/Lightbox/index.tsx Co-authored-by: Tim Golen --- src/components/Lightbox/index.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index cbd49835d2a6..c87f0d3c0622 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -43,10 +43,11 @@ type LightboxProps = { function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChangedProp, onError, style, zoomRange = DEFAULT_ZOOM_RANGE}: LightboxProps) { const StyleUtils = useStyleUtils(); - /** React hooks must be used in the render function of the component at top-level and unconditionally. + /** + * React hooks must be used in the render function of the component at top-level and unconditionally. * Therefore, in order to provide a default value for "isPagerScrolling" if the "AttachmentCarouselPagerContext" is not available, * we need to create a shared value that can be used in the render function. - * */ + */ const isPagerScrollingFallback = useSharedValue(false); const attachmentCarouselPagerContext = useContext(AttachmentCarouselPagerContext); From d9460c48cd3d4ad424b401b098f4094a5a7a0e48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 7 Feb 2024 16:27:16 +0100 Subject: [PATCH 21/33] Update src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts Co-authored-by: Tim Golen --- .../Pager/AttachmentCarouselPagerContext.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts index 32f4a490edba..f3b46ed3651c 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts +++ b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts @@ -7,13 +7,10 @@ import type {SharedValue} from 'react-native-reanimated'; * The pager items array is used within the pager to render and navigate between the images */ type AttachmentCarouselPagerItems = { - /** - * The source of the image is used to identify each attachment/page in the pager - */ + /** The source of the image is used to identify each attachment/page in the pager */ source: string; - /** - * The index of the pager item determines the order of the images in the pager - */ + + /** The index of the pager item determines the order of the images in the pager */ index: number; /** * The active state of the pager item determines whether the image is currently transformable with pinch, pan and tap gestures From 57676b477f92a255f9640df457f6248fc04435ae Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Mon, 12 Feb 2024 20:56:04 +0100 Subject: [PATCH 22/33] fix: prettier --- src/components/Lightbox/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index c87f0d3c0622..48abee216a59 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -43,7 +43,7 @@ type LightboxProps = { function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChangedProp, onError, style, zoomRange = DEFAULT_ZOOM_RANGE}: LightboxProps) { const StyleUtils = useStyleUtils(); - /** + /** * React hooks must be used in the render function of the component at top-level and unconditionally. * Therefore, in order to provide a default value for "isPagerScrolling" if the "AttachmentCarouselPagerContext" is not available, * we need to create a shared value that can be used in the render function. From 0ec7d1972d45db8555a48a74efb2b90e1ff36f03 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Mon, 12 Feb 2024 20:56:26 +0100 Subject: [PATCH 23/33] refactor: setActivePageState -> setActivePageIndex, activePageState -> activePageIndex --- .../Attachments/AttachmentCarousel/Pager/index.tsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index b4c4aa0f192d..98d4df242d02 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -57,7 +57,7 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect const isScrollEnabled = useSharedValue(true); const activePage = useSharedValue(initialPage); - const [activePageState, setActivePageState] = useState(initialPage); + const [activePageIndex, setActivePageIndex] = useState(initialPage); const pageScrollHandler = usePageScrollHandler((e) => { 'worklet'; @@ -67,14 +67,14 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect }, []); useEffect(() => { - setActivePageState(initialPage); + setActivePageIndex(initialPage); activePage.value = initialPage; }, [activePage, initialPage]); /** * The pager uses the source index and current active state to render the pages. */ - const pagerItems = useMemo(() => items.map((item, index) => ({source: item.source, index, isActive: index === activePageState})), [activePageState, items]); + const pagerItems = useMemo(() => items.map((item, index) => ({source: item.source, index, isActive: index === activePageIndex})), [activePageIndex, items]); /** * This callback is passed to the MultiGestureCanvas/Lightbox through the AttachmentCarouselPagerContext. @@ -115,14 +115,14 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect const contextValue = useMemo( () => ({ pagerItems, - activePage: activePageState, + activePage: activePageIndex, isPagerScrolling, isScrollEnabled, pagerRef, onTap: handleTap, onScaleChanged: handleScaleChange, }), - [pagerItems, activePageState, isPagerScrolling, isScrollEnabled, handleTap, handleScaleChange], + [pagerItems, activePageIndex, isPagerScrolling, isScrollEnabled, handleTap, handleScaleChange], ); const animatedProps = useAnimatedProps(() => ({ @@ -155,11 +155,11 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect item={item} isSingleItem={items.length === 1} index={index} - isFocused={index === activePageState && activeSource === item.source} + isFocused={index === activePageIndex && activeSource === item.source} /> )), - [activePageState, activeSource, items, styles.flex1], + [activePageIndex, activeSource, items, styles.flex1], ); return ( From 390fb5c7dddf99805c37c4a62c629c21322fa7f5 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Mon, 12 Feb 2024 21:26:10 +0100 Subject: [PATCH 24/33] refactor: comment formatting --- src/components/MultiGestureCanvas/index.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/MultiGestureCanvas/index.tsx b/src/components/MultiGestureCanvas/index.tsx index c64fee0285d1..1efbe1827b85 100644 --- a/src/components/MultiGestureCanvas/index.tsx +++ b/src/components/MultiGestureCanvas/index.tsx @@ -36,9 +36,7 @@ type MultiGestureCanvasProps = ChildrenProps & { /** Range of zoom that can be applied to the content by pinching or double tapping. */ zoomRange?: Partial; - /** - * A shared value of type boolean, that indicates disabled the transformation gestures (pinch, pan, double tap) - */ + /** A shared value of type boolean, that indicates disabled the transformation gestures (pinch, pan, double tap) */ shouldDisableTransformationGestures?: SharedValue; /** If there is a pager wrapping the canvas, we need to disable the pan gesture in case the pager is swiping */ From 6af0ba19b174fd5ec22a44d82025dfc6d9b70b69 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 13 Feb 2024 12:18:18 +0100 Subject: [PATCH 25/33] fix: strict comparison with null --- src/components/Lightbox/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index 48abee216a59..0c66df4724ac 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -61,7 +61,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan onScaleChanged: onScaleChangedContext, pagerRef, } = useMemo(() => { - if (attachmentCarouselPagerContext == null) { + if (attachmentCarouselPagerContext === null) { return { isUsedInCarousel: false, isSingleCarouselItem: true, From 66ec3e62f73e807ee3c67b016485f7a2ef8f4e6a Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 13 Feb 2024 12:41:13 +0100 Subject: [PATCH 26/33] refactor: rename handler --- src/components/Lightbox/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index 0c66df4724ac..b74a7e50514b 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -186,7 +186,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan } }, [hasSiblingCarouselItems, isActive, isFallbackVisible, isLightboxImageLoaded, isLightboxVisible]); - const handleScaleChange = useCallback( + const scaleChange = useCallback( (scale: number) => { onScaleChangedProp?.(scale); onScaleChangedContext?.(scale); @@ -211,7 +211,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan pagerRef={pagerRef} shouldDisableTransformationGestures={isPagerScrolling} onTap={onTap} - onScaleChanged={handleScaleChange} + onScaleChanged={scaleChange} > Date: Tue, 13 Feb 2024 12:42:15 +0100 Subject: [PATCH 27/33] fix: specify type for --- src/components/Attachments/AttachmentCarousel/Pager/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index 98d4df242d02..f1096d7c9d0c 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -30,7 +30,7 @@ type AttachmentCarouselPagerProps = { */ items: Attachment[]; /** - * The source of the currently active attachment. + * The source (URL) of the currently active attachment. */ activeSource: string; /** From 96e72c979d196919df422c7d6a8760e86a9816ca Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 13 Feb 2024 12:50:43 +0100 Subject: [PATCH 28/33] fix: comment about unused variables --- src/components/MultiGestureCanvas/usePinchGesture.ts | 1 + src/components/MultiGestureCanvas/useTapGestures.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/components/MultiGestureCanvas/usePinchGesture.ts b/src/components/MultiGestureCanvas/usePinchGesture.ts index 249830646672..74a2326748a1 100644 --- a/src/components/MultiGestureCanvas/usePinchGesture.ts +++ b/src/components/MultiGestureCanvas/usePinchGesture.ts @@ -97,6 +97,7 @@ const usePinchGesture = ({ const pinchGesture = Gesture.Pinch() .enabled(pinchEnabled) + // The first argument is not used, but must be defined // eslint-disable-next-line @typescript-eslint/naming-convention .onTouchesDown((_evt, state) => { // We don't want to activate pinch gesture when we are swiping in the pager diff --git a/src/components/MultiGestureCanvas/useTapGestures.ts b/src/components/MultiGestureCanvas/useTapGestures.ts index a5c4a707184a..a28333725d6e 100644 --- a/src/components/MultiGestureCanvas/useTapGestures.ts +++ b/src/components/MultiGestureCanvas/useTapGestures.ts @@ -120,6 +120,7 @@ const useTapGestures = ({ ); const doubleTapGesture = Gesture.Tap() + // The first argument is not used, but must be defined // eslint-disable-next-line @typescript-eslint/naming-convention .onTouchesDown((_evt, state) => { if (!shouldDisableTransformationGestures.value) { From 5b9d413b930424b49cc3acab70365c3706eb1aa4 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 13 Feb 2024 13:02:08 +0100 Subject: [PATCH 29/33] docs: added comments for new properties in context --- .../AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts index f3b46ed3651c..faf2d7bff217 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts +++ b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts @@ -19,7 +19,9 @@ type AttachmentCarouselPagerItems = { }; type AttachmentCarouselPagerContextValue = { + /** The list of items that are shown in the pager */ pagerItems: AttachmentCarouselPagerItems[]; + /** The index of the active page */ activePage: number; pagerRef: ForwardedRef; isPagerScrolling: SharedValue; From 577110a7d8318d57deceac78de7f90f7fe9f1bdc Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 13 Feb 2024 16:15:42 +0100 Subject: [PATCH 30/33] chore: added empty lines --- .../Pager/AttachmentCarouselPagerContext.ts | 2 ++ src/components/Attachments/AttachmentCarousel/Pager/index.tsx | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts index faf2d7bff217..1f4fb78f723b 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts +++ b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts @@ -12,6 +12,7 @@ type AttachmentCarouselPagerItems = { /** The index of the pager item determines the order of the images in the pager */ index: number; + /** * The active state of the pager item determines whether the image is currently transformable with pinch, pan and tap gestures */ @@ -21,6 +22,7 @@ type AttachmentCarouselPagerItems = { type AttachmentCarouselPagerContextValue = { /** The list of items that are shown in the pager */ pagerItems: AttachmentCarouselPagerItems[]; + /** The index of the active page */ activePage: number; pagerRef: ForwardedRef; diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index f1096d7c9d0c..99ac7c18fefb 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -29,18 +29,22 @@ type AttachmentCarouselPagerProps = { * The attachments to be rendered in the pager. */ items: Attachment[]; + /** * The source (URL) of the currently active attachment. */ activeSource: string; + /** * The index of the initial page to be rendered. */ initialPage: number; + /** * A callback to be called when the page is changed. */ onPageSelected: () => void; + /** * A callback that can be used to toggle the attachment carousel arrows, when the scale of the image changes. * @param showArrows If set, it will show/hide the arrows. If not set, it will toggle the arrows. From 62651a0e00f53d536890489280bc29cc629218e9 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 14 Feb 2024 14:13:37 +0100 Subject: [PATCH 31/33] fix: consistent comments --- .../Pager/AttachmentCarouselPagerContext.ts | 8 ++------ .../AttachmentCarousel/Pager/index.tsx | 20 ++++++------------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts index 1f4fb78f723b..fd9b57511cc4 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts +++ b/src/components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext.ts @@ -3,9 +3,7 @@ import {createContext} from 'react'; import type PagerView from 'react-native-pager-view'; import type {SharedValue} from 'react-native-reanimated'; -/** - * The pager items array is used within the pager to render and navigate between the images - */ +/** The pager items array is used within the pager to render and navigate between the images */ type AttachmentCarouselPagerItems = { /** The source of the image is used to identify each attachment/page in the pager */ source: string; @@ -13,9 +11,7 @@ type AttachmentCarouselPagerItems = { /** The index of the pager item determines the order of the images in the pager */ index: number; - /** - * The active state of the pager item determines whether the image is currently transformable with pinch, pan and tap gestures - */ + /** The active state of the pager item determines whether the image is currently transformable with pinch, pan and tap gestures */ isActive: boolean; }; diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index 99ac7c18fefb..7e5612238609 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -25,24 +25,16 @@ type Attachment = { }; type AttachmentCarouselPagerProps = { - /** - * The attachments to be rendered in the pager. - */ + /** The attachments to be rendered in the pager. */ items: Attachment[]; - /** - * The source (URL) of the currently active attachment. - */ + /** The source (URL) of the currently active attachment. */ activeSource: string; - /** - * The index of the initial page to be rendered. - */ + /** The index of the initial page to be rendered. */ initialPage: number; - /** - * A callback to be called when the page is changed. - */ + /** A callback to be called when the page is changed. */ onPageSelected: () => void; /** @@ -147,7 +139,7 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect [], ); - const Content = useMemo( + const carouselItems = useMemo( () => items.map((item, index) => ( - {Content} + {carouselItems} ); From d738e436593a0f8e2120dffe35847c3d0b360281 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 14 Feb 2024 14:30:36 +0100 Subject: [PATCH 32/33] fix: better comment --- src/components/Attachments/AttachmentCarousel/Pager/index.tsx | 4 +--- src/components/Lightbox/index.tsx | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index 7e5612238609..ae83ee0b00ed 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -67,9 +67,7 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect activePage.value = initialPage; }, [activePage, initialPage]); - /** - * The pager uses the source index and current active state to render the pages. - */ + /** The `pagerItems` object that passed down to the context. Later used to detect current page, whether it's a single image gallery etc. */ const pagerItems = useMemo(() => items.map((item, index) => ({source: item.source, index, isActive: index === activePageIndex})), [activePageIndex, items]); /** diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index b74a7e50514b..36cb175e3c45 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -83,7 +83,7 @@ function Lightbox({isAuthTokenRequired = false, uri, onScaleChanged: onScaleChan }; }, [attachmentCarouselPagerContext, isPagerScrollingFallback, uri]); - /** Whether the Lightbox is used within an attachmnet carousel and there are more than one page in the carousel */ + /** Whether the Lightbox is used within an attachment carousel and there are more than one page in the carousel */ const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem; const isActive = page === activePage; From cfc6174199b1c7a05c6845b30b8469a840498a49 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 14 Feb 2024 14:33:44 +0100 Subject: [PATCH 33/33] fix: remove useMemo --- .../AttachmentCarousel/Pager/index.tsx | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx index ae83ee0b00ed..8704584c3e18 100644 --- a/src/components/Attachments/AttachmentCarousel/Pager/index.tsx +++ b/src/components/Attachments/AttachmentCarousel/Pager/index.tsx @@ -137,24 +137,20 @@ function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelect [], ); - const carouselItems = useMemo( - () => - items.map((item, index) => ( - - - - )), - [activePageIndex, activeSource, items, styles.flex1], - ); + const carouselItems = items.map((item, index) => ( + + + + )); return (