From 2d555f7cfa42306dec49e3c1b3cfe1f743f8d173 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 22 Mar 2023 20:37:26 +0200 Subject: [PATCH 01/13] Use virtualized list to pre-render images offscreen --- src/components/AttachmentCarousel/index.js | 78 +++++++++++++++++++--- 1 file changed, 67 insertions(+), 11 deletions(-) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index c2caebcc07a2..f11b52cb0ff3 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -1,5 +1,5 @@ import React from 'react'; -import {View} from 'react-native'; +import {View, VirtualizedList} from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; @@ -39,6 +39,10 @@ const defaultProps = { onNavigate: () => {}, }; +function CellRendererComponent(props) { + return ; +} + class AttachmentCarousel extends React.Component { constructor(props) { super(props); @@ -47,11 +51,16 @@ class AttachmentCarousel extends React.Component { this.cycleThroughAttachments = this.cycleThroughAttachments.bind(this); this.state = { + page: 0, + attachments: [], source: this.props.source, shouldShowArrow: this.canUseTouchScreen, isForwardDisabled: true, isBackDisabled: true, + layout: {}, }; + + this.scrollRef = React.createRef(); } componentDidMount() { @@ -75,7 +84,6 @@ class AttachmentCarousel extends React.Component { getAttachment(attachmentItem) { const source = _.get(attachmentItem, 'source', ''); const file = _.get(attachmentItem, 'file', {name: ''}); - this.props.onNavigate({source: addEncryptedAuthTokenToURL(source), file}); return { source, @@ -95,7 +103,7 @@ class AttachmentCarousel extends React.Component { * This is called when there are new reports to set the state */ makeStateWithReports() { - let page; + let page = this.state.page; const actions = ReportActionsUtils.getSortedReportActions(_.values(this.props.reportActions), true); /** @@ -124,11 +132,9 @@ class AttachmentCarousel extends React.Component { } }); - const {file} = this.getAttachment(attachments[page]); this.setState({ page, attachments, - file, isForwardDisabled: page === 0, isBackDisabled: page === attachments.length - 1, }); @@ -146,6 +152,8 @@ class AttachmentCarousel extends React.Component { this.setState(({attachments, page}) => { const nextIndex = page - deltaSlide; const {source, file} = this.getAttachment(attachments[nextIndex]); + this.props.onNavigate({source: addEncryptedAuthTokenToURL(source), file}); + this.scrollRef.current.scrollToIndex({index: nextIndex, animated: false}); return { page: nextIndex, source, @@ -156,11 +164,37 @@ class AttachmentCarousel extends React.Component { }); } + onMainLayout = ({nativeEvent}) => { + this.setState({layout: nativeEvent.layout}); + } + + getItemLayout = (data, index) => { + const width = this.state.layout.width; + return ({ + length: width, + offset: width * index, + index, + }); + } + + renderItem = ({item}) => { + console.log('item: ', item); + + return ( + this.toggleArrowsVisibility(!this.state.shouldShowArrow)} + source={item.source} + file={item.file} + /> + ); + } + render() { const isPageSet = Number.isInteger(this.state.page); - const authSource = addEncryptedAuthTokenToURL(this.state.source); + return ( this.toggleArrowsVisibility(true)} onMouseLeave={() => this.toggleArrowsVisibility(false)} @@ -204,11 +238,21 @@ class AttachmentCarousel extends React.Component { onPress={() => this.canUseTouchScreen && this.toggleArrowsVisibility(!this.state.shouldShowArrow)} onCycleThroughAttachments={this.cycleThroughAttachments} > - this.toggleArrowsVisibility(!this.state.shouldShowArrow)} - source={authSource} - key={authSource} - file={this.state.file} + item.source} + getItemCount={() => this.state.attachments.length} + getItem={(data, i) => this.getAttachment(data[i])} /> @@ -216,6 +260,18 @@ class AttachmentCarousel extends React.Component { } } +function CarouselItem(props) { + const authSource = addEncryptedAuthTokenToURL(props.source); + + return ( + + ); +} + AttachmentCarousel.propTypes = propTypes; AttachmentCarousel.defaultProps = defaultProps; From 61debcca878afeec7fd2595b93dbf27df5c9e1b2 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 4 Apr 2023 17:32:34 +0300 Subject: [PATCH 02/13] AttachmentCarousel: Use window layout dimensions --- src/components/AttachmentCarousel/index.js | 35 +++++++++------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index f11b52cb0ff3..53bc8ae899cf 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -18,6 +18,7 @@ import reportActionPropTypes from '../../pages/home/report/reportActionPropTypes import tryResolveUrlFromApiRoot from '../../libs/tryResolveUrlFromApiRoot'; import Tooltip from '../Tooltip'; import withLocalize, {withLocalizePropTypes} from '../withLocalize'; +import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions'; import compose from '../../libs/compose'; const propTypes = { @@ -31,6 +32,7 @@ const propTypes = { reportActions: PropTypes.objectOf(PropTypes.shape(reportActionPropTypes)), ...withLocalizePropTypes, + ...windowDimensionsPropTypes, }; const defaultProps = { @@ -57,7 +59,6 @@ class AttachmentCarousel extends React.Component { shouldShowArrow: this.canUseTouchScreen, isForwardDisabled: true, isBackDisabled: true, - layout: {}, }; this.scrollRef = React.createRef(); @@ -164,12 +165,8 @@ class AttachmentCarousel extends React.Component { }); } - onMainLayout = ({nativeEvent}) => { - this.setState({layout: nativeEvent.layout}); - } - getItemLayout = (data, index) => { - const width = this.state.layout.width; + const width = this.props.windowWidth; return ({ length: width, offset: width * index, @@ -177,24 +174,19 @@ class AttachmentCarousel extends React.Component { }); } - renderItem = ({item}) => { - console.log('item: ', item); - - return ( - this.toggleArrowsVisibility(!this.state.shouldShowArrow)} - source={item.source} - file={item.file} - /> - ); - } + renderItem = ({item}) => ( + this.toggleArrowsVisibility(!this.state.shouldShowArrow)} + source={item.source} + file={item.file} + /> + ) render() { const isPageSet = Number.isInteger(this.state.page); return ( this.toggleArrowsVisibility(true)} onMouseLeave={() => this.toggleArrowsVisibility(false)} @@ -240,13 +232,13 @@ class AttachmentCarousel extends React.Component { > Date: Wed, 5 Apr 2023 11:35:22 +0300 Subject: [PATCH 03/13] AttachmentCarousel: Make virtual list work in mobile --- src/components/AttachmentCarousel/index.js | 120 ++++++++++----------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index 53bc8ae899cf..422bc737c227 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -1,5 +1,5 @@ import React from 'react'; -import {View, VirtualizedList} from 'react-native'; +import {View, FlatList} from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; @@ -41,10 +41,6 @@ const defaultProps = { onNavigate: () => {}, }; -function CellRendererComponent(props) { - return ; -} - class AttachmentCarousel extends React.Component { constructor(props) { super(props); @@ -61,11 +57,12 @@ class AttachmentCarousel extends React.Component { isBackDisabled: true, }; - this.scrollRef = React.createRef(); - } + this.state = { + ...this.state, + ...this.makeStateWithReports(), + }; - componentDidMount() { - this.makeStateWithReports(); + this.scrollRef = React.createRef(); } componentDidUpdate(prevProps) { @@ -74,7 +71,9 @@ class AttachmentCarousel extends React.Component { if (previousReportActionsCount === currentReportActionsCount) { return; } - this.makeStateWithReports(); + + const nextState = this.makeStateWithReports(); + this.setState(nextState); } /** @@ -104,7 +103,7 @@ class AttachmentCarousel extends React.Component { * This is called when there are new reports to set the state */ makeStateWithReports() { - let page = this.state.page; + let page = 0; const actions = ReportActionsUtils.getSortedReportActions(_.values(this.props.reportActions), true); /** @@ -133,12 +132,12 @@ class AttachmentCarousel extends React.Component { } }); - this.setState({ + return { page, attachments, isForwardDisabled: page === 0, isBackDisabled: page === attachments.length - 1, - }); + }; } /** @@ -166,7 +165,7 @@ class AttachmentCarousel extends React.Component { } getItemLayout = (data, index) => { - const width = this.props.windowWidth; + const width = this.width || this.props.windowWidth; return ({ length: width, offset: width * index, @@ -174,24 +173,33 @@ class AttachmentCarousel extends React.Component { }); } - renderItem = ({item}) => ( - this.toggleArrowsVisibility(!this.state.shouldShowArrow)} - source={item.source} - file={item.file} - /> + renderItem = ({item}) => { + const authSource = addEncryptedAuthTokenToURL(item.source); + + return ( + this.toggleArrowsVisibility(!this.state.shouldShowArrow)} + source={authSource} + file={item.file} + /> + ); + } + + renderCell = props => ( + // eslint-disable-next-line react/jsx-props-no-spreading + ) - render() { - const isPageSet = Number.isInteger(this.state.page); + keyExtractor = item => item.source + render() { return ( this.toggleArrowsVisibility(true)} onMouseLeave={() => this.toggleArrowsVisibility(false)} > - {(isPageSet && this.state.shouldShowArrow) && ( + {this.state.shouldShowArrow && ( <> {!this.state.isBackDisabled && ( @@ -223,47 +231,39 @@ class AttachmentCarousel extends React.Component { )} )} - this.canUseTouchScreen && this.toggleArrowsVisibility(!this.state.shouldShowArrow)} - onCycleThroughAttachments={this.cycleThroughAttachments} - > - item.source} - getItemCount={() => this.state.attachments.length} - getItem={(data, i) => this.getAttachment(data[i])} - /> - + ); } } -function CarouselItem(props) { - const authSource = addEncryptedAuthTokenToURL(props.source); - - return ( - - ); -} - AttachmentCarousel.propTypes = propTypes; AttachmentCarousel.defaultProps = defaultProps; From 724642426a5096b60ad80d2df15d0b492494b977 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 5 Apr 2023 16:07:39 +0300 Subject: [PATCH 04/13] AttachmentCarousel: Fix flicker navigating back on mobile --- src/components/AttachmentCarousel/index.js | 4 ++-- src/components/ImageView/index.native.js | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index 422bc737c227..1707205298a9 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -153,7 +153,7 @@ class AttachmentCarousel extends React.Component { const nextIndex = page - deltaSlide; const {source, file} = this.getAttachment(attachments[nextIndex]); this.props.onNavigate({source: addEncryptedAuthTokenToURL(source), file}); - this.scrollRef.current.scrollToIndex({index: nextIndex, animated: false}); + this.scrollRef.current.scrollToIndex({index: nextIndex, animated: true}); return { page: nextIndex, source, @@ -248,7 +248,7 @@ class AttachmentCarousel extends React.Component { ref={this.scrollRef} initialScrollIndex={this.state.page} initialNumToRender={3} - windowSize={15} + windowSize={7} maxToRenderPerBatch={3} updateCellsBatchingPeriod={250} data={this.state.attachments} diff --git a/src/components/ImageView/index.native.js b/src/components/ImageView/index.native.js index 11732715b941..d2f808e56aab 100644 --- a/src/components/ImageView/index.native.js +++ b/src/components/ImageView/index.native.js @@ -37,10 +37,11 @@ class ImageView extends PureComponent { super(props); this.state = { - isLoading: true, + isLoaded: false, imageWidth: 0, imageHeight: 0, interactionPromise: undefined, + containerHeight: props.windowHeight, }; // Use the default double click interval from the ImageZoom library @@ -121,7 +122,7 @@ class ImageView extends PureComponent { const maxDimensionsScale = 11; imageWidth = Math.min(imageWidth, (containerWidth * maxDimensionsScale)); imageHeight = Math.min(imageHeight, (containerHeight * maxDimensionsScale)); - this.setState({imageHeight, imageWidth, isLoading: false}); + this.setState({imageHeight, imageWidth, isLoaded: true}); }); } @@ -145,18 +146,14 @@ class ImageView extends PureComponent { } imageLoadingStart() { - if (this.state.isLoading) { - return; - } this.resetImageZoom(); - this.setState({imageHeight: 0, imageWidth: 0, isLoading: true}); } render() { // Default windowHeight accounts for the modal header height const windowHeight = this.props.windowHeight - variables.contentHeaderHeight; const hasImageDimensions = this.state.imageWidth !== 0 && this.state.imageHeight !== 0; - const shouldShowLoadingIndicator = this.state.isLoading || !hasImageDimensions; + const shouldShowLoadingIndicator = !this.state.isLoaded || !hasImageDimensions; // Zoom view should be loaded only after measuring actual image dimensions, otherwise it causes blurred images on Android return ( From 478c497ea573e7b406840aa13186b2c0a5b0a605 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 5 Apr 2023 20:16:46 +0300 Subject: [PATCH 05/13] AttachmentCarousel: fix navigation --- src/components/AttachmentCarousel/index.js | 62 +++++++++++++--------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index 1707205298a9..07bba9ba10bf 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -6,7 +6,6 @@ import _ from 'underscore'; import * as Expensicons from '../Icon/Expensicons'; import styles from '../../styles/styles'; import themeColors from '../../styles/themes/default'; -import CarouselActions from './CarouselActions'; import Button from '../Button'; import * as ReportActionsUtils from '../../libs/ReportActionsUtils'; import AttachmentView from '../AttachmentView'; @@ -46,6 +45,11 @@ class AttachmentCarousel extends React.Component { super(props); this.canUseTouchScreen = DeviceCapabilities.canUseTouchScreen(); + this.viewabilityConfig = { + // To facilitate paging through the attachments, we want to consider an item "viewable" when it is + // more than 90% visible. When that happens we update the page index in the state. + itemVisiblePercentThreshold: 95, + }; this.cycleThroughAttachments = this.cycleThroughAttachments.bind(this); this.state = { @@ -53,8 +57,6 @@ class AttachmentCarousel extends React.Component { attachments: [], source: this.props.source, shouldShowArrow: this.canUseTouchScreen, - isForwardDisabled: true, - isBackDisabled: true, }; this.state = { @@ -135,8 +137,6 @@ class AttachmentCarousel extends React.Component { return { page, attachments, - isForwardDisabled: page === 0, - isBackDisabled: page === attachments.length - 1, }; } @@ -145,23 +145,14 @@ class AttachmentCarousel extends React.Component { * @param {Number} deltaSlide */ cycleThroughAttachments(deltaSlide) { - if ((deltaSlide > 0 && this.state.isForwardDisabled) || (deltaSlide < 0 && this.state.isBackDisabled)) { + const nextIndex = this.state.page - deltaSlide; + const nextItem = this.state.attachments[nextIndex]; + + if (!nextItem) { return; } - this.setState(({attachments, page}) => { - const nextIndex = page - deltaSlide; - const {source, file} = this.getAttachment(attachments[nextIndex]); - this.props.onNavigate({source: addEncryptedAuthTokenToURL(source), file}); - this.scrollRef.current.scrollToIndex({index: nextIndex, animated: true}); - return { - page: nextIndex, - source, - file, - isBackDisabled: nextIndex === attachments.length - 1, - isForwardDisabled: nextIndex === 0, - }; - }); + this.scrollRef.current.scrollToIndex({index: nextIndex, animated: true}); } getItemLayout = (data, index) => { @@ -192,7 +183,28 @@ class AttachmentCarousel extends React.Component { keyExtractor = item => item.source + /** + * Updates the page state when the user navigates between attachments + * @param {Array<{item: *, index: Number}>} viewableItems + */ + updatePage = ({viewableItems}) => { + // Since we can have only one item in view at a time, we can use the first item in the array + // to get the index of the current page + const entry = _.first(viewableItems); + if (!entry) { + return; + } + + const page = entry.index; + const {source, file} = this.getAttachment(entry.item); + this.props.onNavigate({source: addEncryptedAuthTokenToURL(source), file}); + this.setState({page, source}); + } + render() { + const isForwardDisabled = this.state.page === 0; + const isBackDisabled = this.state.page === _.size(this.state.attachments) - 1; + return ( {this.state.shouldShowArrow && ( <> - {!this.state.isBackDisabled && ( + {!isBackDisabled && (