fixed up some code per comments, added debounce to image size

Expensify · Jan 11, 2023 · bc8fc1a · bc8fc1a
1 parent 17fe0fc
commit bc8fc1a
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 22 deletions.
diff --git a/src/components/AttachmentCarousel/CarouselActions/index.js b/src/components/AttachmentCarousel/CarouselActions/index.js
@@ -10,7 +10,7 @@ const propTypes = {
     onCycleThroughAttachments: PropTypes.func.isRequired,
 
     /** Styles to be assigned to Carousel */
-    styles: PropTypes.shape({}).isRequired,
+    styles: PropTypes.arrayOf(PropTypes.shape({})).isRequired,
 
     /** Children to render */
     children: PropTypes.oneOfType([

diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js
@@ -52,7 +52,7 @@ class AttachmentCarousel extends React.Component {
         this.canUseTouchScreen = canUseTouchScreen();
         this.makeStateWithReports = this.makeStateWithReports.bind(this);
         this.cycleThroughAttachments = this.cycleThroughAttachments.bind(this);
-        this.onShowArrow = this.onShowArrow.bind(this);
+        this.toggleArrowsVisibility = this.toggleArrowsVisibility.bind(this);
 
         this.state = {shouldShowArrow: this.canUseTouchScreen};
     }
@@ -62,22 +62,14 @@ class AttachmentCarousel extends React.Component {
     }
 
     componentDidUpdate(prevProps) {
-        const prevReportSize = _.size(prevProps.reportActions);
-        const currReportSize = _.size(this.props.reportActions);
-        if (prevReportSize === currReportSize) {
+        const previousReportActionsCount = _.size(prevProps.reportActions);
+        const currentReportActionsCount = _.size(this.props.reportActions);
+        if (previousReportActionsCount === currentReportActionsCount) {
             return;
         }
         this.makeStateWithReports();
     }
 
-    /**
-     * Toggles the visibility of the arrows
-     * @param {Boolean} shouldShowArrow
-     */
-    onShowArrow(shouldShowArrow) {
-        this.setState({shouldShowArrow});
-    }
-
     /**
      * Helps to navigate between next/previous attachments
      * @param {Object} attachmentItem
@@ -94,6 +86,14 @@ class AttachmentCarousel extends React.Component {
         };
     }
 
+    /**
+     * Toggles the visibility of the arrows
+     * @param {Boolean} shouldShowArrow
+     */
+    toggleArrowsVisibility(shouldShowArrow) {
+        this.setState({shouldShowArrow});
+    }
+
     /**
      * This is called when there are new reports to set the state
      */
@@ -185,8 +185,8 @@ class AttachmentCarousel extends React.Component {
         return (
             <View
                 style={[styles.attachmentModalArrowsContainer]}
-                onMouseEnter={() => this.onShowArrow(true)}
-                onMouseLeave={() => this.onShowArrow(false)}
+                onMouseEnter={() => this.toggleArrowsVisibility(true)}
+                onMouseLeave={() => this.toggleArrowsVisibility(false)}
             >
                 {this.state.shouldShowArrow && (
                     <>
@@ -220,10 +220,10 @@ class AttachmentCarousel extends React.Component {
                     styles={[styles.attachmentModalArrowsContainer]}
                     canSwipeLeft={!this.state.isBackDisabled}
                     canSwipeRight={!this.state.isForwardDisabled}
-                    onPress={() => this.canUseTouchScreen && this.onShowArrow(!this.state.shouldShowArrow)}
+                    onPress={() => this.canUseTouchScreen && this.toggleArrowsVisibility(!this.state.shouldShowArrow)}
                     onCycleThroughAttachments={this.cycleThroughAttachments}
                 >
-                    <AttachmentView onPress={() => this.onShowArrow(!this.state.shouldShowArrow)} sourceURL={authSourceURL} file={this.state.file} />
+                    <AttachmentView onPress={() => this.toggleArrowsVisibility(!this.state.shouldShowArrow)} sourceURL={authSourceURL} file={this.state.file} />
                 </CarouselActions>
 
             </View>

diff --git a/src/components/ImageView/index.js b/src/components/ImageView/index.js
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import {
     View, Image, Pressable,
 } from 'react-native';
+import _ from 'underscore';
 import styles from '../../styles/styles';
 import * as StyleUtils from '../../styles/StyleUtils';
 import * as DeviceCapabilities from '../../libs/DeviceCapabilities';
@@ -20,6 +21,7 @@ class ImageView extends PureComponent {
         super(props);
         this.scrollableRef = null;
         this.canUseTouchScreen = DeviceCapabilities.canUseTouchScreen();
+        this.debouncedCalculateImageSize = _.debounce(this.calculateImageSize, 220);
         this.onContainerLayoutChanged = this.onContainerLayoutChanged.bind(this);
         this.onContainerPressIn = this.onContainerPressIn.bind(this);
         this.onContainerPress = this.onContainerPress.bind(this);
@@ -46,18 +48,21 @@ class ImageView extends PureComponent {
     }
 
     componentDidMount() {
+        this.debouncedCalculateImageSize();
         if (this.canUseTouchScreen) {
             return;
         }
+
         document.addEventListener('mousemove', this.trackMovement);
         document.addEventListener('mouseup', this.trackPointerPosition);
     }
 
-    componentDidUpdate() {
-        // Resize the image when cycled through
-        Image.getSize(this.props.url, (width, height) => {
-            this.setImageRegion(width, height);
-        });
+    componentDidUpdate(prevProps) {
+        if (prevProps.url === this.props.url) {
+            return;
+        }
+        this.debouncedCalculateImageSize.cancel();
+        this.debouncedCalculateImageSize();
     }
 
     componentWillUnmount() {
@@ -182,6 +187,18 @@ class ImageView extends PureComponent {
         return {offsetX, offsetY};
     }
 
+    /**
+     * Resize the image when cycled through
+     */
+    calculateImageSize() {
+        if (!this.props.url) {
+            return;
+        }
+        Image.getSize(this.props.url, (width, height) => {
+            this.setImageRegion(width, height);
+        });
+    }
+
     /**
      * @param {SyntheticEvent} e
      */