From d7ced98db778c6925518dd08c35ecd0765273d8e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Sep 2020 15:02:24 -0700 Subject: [PATCH 01/34] add comment --- package-lock.json | 16 +++ package.json | 1 + src/components/InvertedFlatList/index.js | 98 +++++++++++++++++++ .../InvertedFlatList/index.native.js | 14 +++ src/page/home/MainView.js | 60 ++++-------- src/page/home/report/ReportActionItem.js | 20 +++- src/page/home/report/ReportActionsView.js | 46 ++++++--- 7 files changed, 195 insertions(+), 60 deletions(-) create mode 100644 src/components/InvertedFlatList/index.js create mode 100644 src/components/InvertedFlatList/index.native.js diff --git a/package-lock.json b/package-lock.json index d71ca160cec8..c4eae3ab69ba 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15362,6 +15362,22 @@ } } }, + "react-window-reversed": { + "version": "1.4.1", + "resolved": "https://registry.npmjs.org/react-window-reversed/-/react-window-reversed-1.4.1.tgz", + "integrity": "sha512-bVfkAdr+6psOJZp27rzoKg3xg5oVkDFumzpOE2HHVR9yt+5MkX4HGsLeA0NZ2mwLcJ82YsLy3GyNLPW7J5G4OQ==", + "requires": { + "@babel/runtime": "^7.0.0", + "memoize-one": "^3.1.1" + }, + "dependencies": { + "memoize-one": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-3.1.1.tgz", + "integrity": "sha512-YqVh744GsMlZu6xkhGslPSqSurOv6P+kLN2J3ysBZfagLcL5FdRK/0UpgLoL8hwjjEvvAVkjJZyFP+1T6p1vgA==" + } + } + }, "read-config-file": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/read-config-file/-/read-config-file-4.0.1.tgz", diff --git a/package.json b/package.json index 043471be3983..834e43b2bcf3 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "react-router-dom": "^5.2.0", "react-router-native": "^5.2.0", "react-web-config": "^1.0.0", + "react-window-reversed": "^1.4.1", "save": "^2.4.0", "underscore": "^1.10.2" }, diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js new file mode 100644 index 000000000000..a622ed827a11 --- /dev/null +++ b/src/components/InvertedFlatList/index.js @@ -0,0 +1,98 @@ +import _ from 'underscore'; +import React, {forwardRef} from 'react'; +import {View} from 'react-native'; +import {VariableSizeList} from 'react-window-reversed'; + + +/** + * This component is an alternate implementation of FlatList for web. + * FlatList with an inverted prop does not work correctly on web but + * works fine for mobile so we are using react-window here to create + * our inverted list scroller so eke out extra performance on web. + */ +class InvertedFlatList extends React.Component { + constructor(props) { + super(props); + + this.sizeMap = {}; + + this.state = { + listHeight: 0, + }; + + this.getSize = this.getSize.bind(this); + } + + componentDidMount() { + this.setState({listHeight: this.list.offsetHeight}, () => { + // This is unfortunate, but it's really difficult to tell when the initial + // set of items have rendered and until then there's no guarantee that we + // will scroll to the exact bottom of the list due to how the programmatic + // scroll in react-window works. + + // eslint-disable-next-line no-underscore-dangle + _.defer(() => this.listRef._outerRef.scrollTo({top: this.listRef._outerRef.scrollHeight}), 0); + }); + } + + shouldComponentUpdate(prevProps, prevState) { + return prevProps.data.length !== this.props.data.length + || prevState.listHeight !== this.state.listHeight; + } + + /** + * @TODO this shouldn't really be specific to report actions + * + * @param {Number} index + * @returns {Number} + */ + getSize(index) { + const {action} = this.props.data[index]; + if (action.actionName !== 'ADDCOMMENT') { + return 0; + } + + return this.sizeMap[index] || 42; + } + + render() { + return ( + this.list = el} + > + this.listRef = el} + overscanCount={5} + reversed + > + {({index, style}) => ( +
+ {this.props.renderItem({ + item: this.props.data[index], + index, + onLayout: ({nativeEvent}) => { + const prevSize = this.sizeMap[index] || 0; + if (prevSize !== nativeEvent.layout.height) { + this.sizeMap[index] = nativeEvent.layout.height; + this.listRef.resetAfterIndex(0); + } + }, + needsLayoutCalculation: style.height === 42, + })} +
+ )} + + + ); + } +} + +export default forwardRef((props, forwardedRef) => ( + // eslint-disable-next-line react/jsx-props-no-spreading + +)); diff --git a/src/components/InvertedFlatList/index.native.js b/src/components/InvertedFlatList/index.native.js new file mode 100644 index 000000000000..fa592fad5dd7 --- /dev/null +++ b/src/components/InvertedFlatList/index.native.js @@ -0,0 +1,14 @@ +import React, {forwardRef} from 'react'; +import {FlatList} from 'react-native'; + +/** + * Inverted FlatList for native is just the base FlatList + */ +export default forwardRef((props, forwardedRef) => ( + +)); diff --git a/src/page/home/MainView.js b/src/page/home/MainView.js index 8aaadf4c8afd..ce9962b449c0 100644 --- a/src/page/home/MainView.js +++ b/src/page/home/MainView.js @@ -25,51 +25,25 @@ const propTypes = { const defaultProps = { reports: {}, }; +const MainView = (props) => { + const reportIDInURL = parseInt(props.match.params.reportID, 10); -class MainView extends React.Component { - render() { - if (!_.size(this.props.reports)) { - return null; - } - - const reportIDInURL = parseInt(this.props.match.params.reportID, 10); - - // The styles for each of our reports. Basically, they are all hidden except for the one matching the - // reportID in the URL - let activeReportID; - const reportStyles = _.reduce(this.props.reports, (memo, report) => { - const isActiveReport = reportIDInURL === report.reportID; - const finalData = {...memo}; - let reportStyle; - - if (isActiveReport) { - activeReportID = report.reportID; - reportStyle = [styles.dFlex, styles.flex1]; - } else { - reportStyle = [styles.dNone]; - } - - finalData[report.reportID] = [reportStyle]; - return finalData; - }, {}); - - return ( - <> - {_.map(this.props.reports, report => ( - - - - ))} - - ); + if (!_.size(props.reports) || !reportIDInURL) { + return null; } -} + + return ( + + + + ); +}; MainView.propTypes = propTypes; MainView.defaultProps = defaultProps; diff --git a/src/page/home/report/ReportActionItem.js b/src/page/home/report/ReportActionItem.js index cf9407c77ecd..7cca864f418b 100644 --- a/src/page/home/report/ReportActionItem.js +++ b/src/page/home/report/ReportActionItem.js @@ -12,12 +12,24 @@ const propTypes = { // Should the comment have the appearance of being grouped with the previous comment? displayAsGroup: PropTypes.bool.isRequired, + + // Used to tell this component that we want it to call onLayout when it renders + needsLayoutCalculation: PropTypes.bool, + + // Called when the view layout completes + onLayout: PropTypes.func, +}; + +const defaultProps = { + needsLayoutCalculation: false, + onLayout: () => {}, }; class ReportActionItem extends React.Component { shouldComponentUpdate(nextProps) { // This component should only render if the action's sequenceNumber or displayAsGroup props change - return nextProps.displayAsGroup !== this.props.displayAsGroup + return nextProps.needsLayoutCalculation !== this.props.needsLayoutCalculation + || nextProps.displayAsGroup !== this.props.displayAsGroup || !_.isEqual(nextProps.action, this.props.action); } @@ -28,7 +40,10 @@ class ReportActionItem extends React.Component { } return ( - + {!displayAsGroup && } {displayAsGroup && } @@ -37,5 +52,6 @@ class ReportActionItem extends React.Component { } ReportActionItem.propTypes = propTypes; +ReportActionItem.defaultProps = defaultProps; export default ReportActionItem; diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 807abeff3c25..853a8414884d 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -1,5 +1,5 @@ import React from 'react'; -import {View, ScrollView, Keyboard} from 'react-native'; +import {View, Keyboard} from 'react-native'; import PropTypes from 'prop-types'; import _ from 'underscore'; import lodashGet from 'lodash.get'; @@ -12,6 +12,7 @@ import styles from '../../../style/StyleSheet'; import {withRouter} from '../../../lib/Router'; import ReportActionPropTypes from './ReportActionPropTypes'; import compose from '../../../lib/compose'; +import InvertedFlatList from '../../../components/InvertedFlatList'; const propTypes = { // These are from withRouter @@ -126,6 +127,17 @@ class ReportActionsView extends React.Component { this.recordMaxAction(); } + /** + * Converts items into array of objects for the FlatList + * + * @returns {Array} + */ + getItems() { + return _.sortBy(this.props.reportActions, 'sequenceNumber') + .map((item, index) => ({action: item, index})) + .reverse(); + } + render() { if (!_.size(this.props.reportActions)) { return ( @@ -134,24 +146,28 @@ class ReportActionsView extends React.Component { ); } - + const data = this.getItems(); return ( - { - this.actionListElement = el; - }} - onContentSizeChange={this.scrollToListBottom} - bounces={false} - contentContainerStyle={[styles.chatContentScrollView]} - > - {_.chain(this.props.reportActions).sortBy('sequenceNumber').map((item, index) => ( + this.actionListElement = el} + data={data} + renderItem={({ + item, + index, + onLayout, + needsLayoutCalculation, + }) => ( - )).value()} - + )} + bounces={false} + contentContainerStyle={[styles.chatContentScrollView]} + keyExtractor={item => `${item.action.sequenceNumber}`} + /> ); } } From dc6b35af7ebc32c6416f60c4a13a2d90cd4fc0d8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Sep 2020 21:04:53 -0700 Subject: [PATCH 02/34] remove reverse --- src/components/InvertedFlatList/index.js | 118 +++++++++++------- .../InvertedFlatList/index.native.js | 2 +- src/page/home/report/ReportActionItem.js | 4 - src/page/home/report/ReportActionsView.js | 4 +- 4 files changed, 74 insertions(+), 54 deletions(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index a622ed827a11..8a90bb888f7d 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -1,8 +1,37 @@ -import _ from 'underscore'; -import React, {forwardRef} from 'react'; +import React, {forwardRef, createContext} from 'react'; import {View} from 'react-native'; -import {VariableSizeList} from 'react-window-reversed'; +import {VariableSizeList} from 'react-window'; +const ReactWindowContext = createContext({}); +const MINIMUM_ROW_HEIGHT = 42; + +/** + * This is the inner most + */ +const innerElement = forwardRef((props, ref) => { + return ( + + {({dimensions}) => { + const innerHeight = props.style.height; + const top = dimensions.top || 0; + const height = dimensions.height || 0; + const difference = height - top - innerHeight; + + return ( +
0 ? difference : 0, + }} + /> + ); + }} + + ); +}); /** * This component is an alternate implementation of FlatList for web. @@ -24,14 +53,10 @@ class InvertedFlatList extends React.Component { } componentDidMount() { + // Set the height of the list after the component mounts + // and then scroll to the bottom. this.setState({listHeight: this.list.offsetHeight}, () => { - // This is unfortunate, but it's really difficult to tell when the initial - // set of items have rendered and until then there's no guarantee that we - // will scroll to the exact bottom of the list due to how the programmatic - // scroll in react-window works. - - // eslint-disable-next-line no-underscore-dangle - _.defer(() => this.listRef._outerRef.scrollTo({top: this.listRef._outerRef.scrollHeight}), 0); + this.listRef.scrollToItem(this.props.data.length, 'auto'); }); } @@ -41,53 +66,52 @@ class InvertedFlatList extends React.Component { } /** - * @TODO this shouldn't really be specific to report actions - * * @param {Number} index * @returns {Number} */ getSize(index) { - const {action} = this.props.data[index]; - if (action.actionName !== 'ADDCOMMENT') { - return 0; - } - - return this.sizeMap[index] || 42; + return this.sizeMap[index] || MINIMUM_ROW_HEIGHT; } render() { return ( - this.list = el} + - this.listRef = el} - overscanCount={5} - reversed + this.list = el} > - {({index, style}) => ( -
- {this.props.renderItem({ - item: this.props.data[index], - index, - onLayout: ({nativeEvent}) => { - const prevSize = this.sizeMap[index] || 0; - if (prevSize !== nativeEvent.layout.height) { - this.sizeMap[index] = nativeEvent.layout.height; - this.listRef.resetAfterIndex(0); - } - }, - needsLayoutCalculation: style.height === 42, - })} -
- )} - - + this.listRef = el} + overscanCount={1} + innerRef={el => this.innerRef = el} + outerRef={el => this.outerRef = el} + innerElementType={innerElement} + > + {({index, style}) => ( +
+ {this.props.renderItem({ + item: this.props.data[index], + index, + onLayout: ({nativeEvent}) => { + const prevSize = this.sizeMap[index] || 0; + if (prevSize !== nativeEvent.layout.height) { + this.sizeMap[index] = nativeEvent.layout.height; + this.listRef.resetAfterIndex(0); + } + }, + needsLayoutCalculation: style.height === MINIMUM_ROW_HEIGHT, + })} +
+ )} + + + ); } } diff --git a/src/components/InvertedFlatList/index.native.js b/src/components/InvertedFlatList/index.native.js index fa592fad5dd7..e84d5cb5dc3f 100644 --- a/src/components/InvertedFlatList/index.native.js +++ b/src/components/InvertedFlatList/index.native.js @@ -7,7 +7,7 @@ import {FlatList} from 'react-native'; export default forwardRef((props, forwardedRef) => ( diff --git a/src/page/home/report/ReportActionItem.js b/src/page/home/report/ReportActionItem.js index 7cca864f418b..b0eaa3d86e27 100644 --- a/src/page/home/report/ReportActionItem.js +++ b/src/page/home/report/ReportActionItem.js @@ -35,10 +35,6 @@ class ReportActionItem extends React.Component { render() { const {action, displayAsGroup} = this.props; - if (action.actionName !== 'ADDCOMMENT') { - return null; - } - return ( ({action: item, index})) - .reverse(); + .filter(action => action.actionName === 'ADDCOMMENT') + .map((item, index) => ({action: item, index})); } render() { From d9fc537230618aa33fcd185e329766565a507cdd Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Sep 2020 21:45:16 -0700 Subject: [PATCH 03/34] remove unneeded things --- src/components/InvertedFlatList/index.js | 68 ++++++++++++++--------- src/page/home/report/ReportActionsView.js | 11 ---- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index 8a90bb888f7d..856155259c93 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -6,32 +6,35 @@ const ReactWindowContext = createContext({}); const MINIMUM_ROW_HEIGHT = 42; /** - * This is the inner most + * This is the innermost element and we are passing it as a custom + * component so that we can overwrite some styles and simulate + * an inverse FlatList with items starting from the bottom of the + * scroll position. react-window has no "reverse" feature so we've + * built something similar around the existing API. */ -const innerElement = forwardRef((props, ref) => { - return ( - - {({dimensions}) => { - const innerHeight = props.style.height; - const top = dimensions.top || 0; - const height = dimensions.height || 0; - const difference = height - top - innerHeight; +const innerElement = forwardRef((props, ref) => ( + + {({dimensions}) => { + const innerHeight = props.style.height; + const top = dimensions.top || 0; + const height = dimensions.height || 0; + const difference = height - top - innerHeight; - return ( -
0 ? difference : 0, - }} - /> - ); - }} - - ); -}); + return ( +
0 ? difference : 0, + }} + /> + ); + }} + +)); /** * This component is an alternate implementation of FlatList for web. @@ -56,7 +59,11 @@ class InvertedFlatList extends React.Component { // Set the height of the list after the component mounts // and then scroll to the bottom. this.setState({listHeight: this.list.offsetHeight}, () => { - this.listRef.scrollToItem(this.props.data.length, 'auto'); + this.scrollToEnd(); + }); + + this.props.forwardedRef({ + scrollToEnd: () => this.scrollToEnd(), }); } @@ -73,6 +80,13 @@ class InvertedFlatList extends React.Component { return this.sizeMap[index] || MINIMUM_ROW_HEIGHT; } + /** + * Scroll to end implementation for web. + */ + scrollToEnd() { + this.listRef.scrollToItem(this.props.data.length - 1, 'end'); + } + render() { return ( diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 130e21d4b652..20d9b282828c 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -73,12 +73,6 @@ class ReportActionsView extends React.Component { // eslint-disable-next-line isConsecutiveActionMadeByPreviousActor(actionIndex) { const reportActions = lodashGet(this.props, 'reportActions', {}); - - // This is the created action and the very first action so it cannot be a consecutive comment. - if (actionIndex === 0) { - return false; - } - const previousAction = reportActions[actionIndex - 1]; const currentAction = reportActions[actionIndex]; @@ -88,11 +82,6 @@ class ReportActionsView extends React.Component { return false; } - // Only comments that follow other comments are consecutive - if (previousAction.actionName !== 'ADDCOMMENT' || currentAction.actionName !== 'ADDCOMMENT') { - return false; - } - // Comments are only grouped if they happen within 5 minutes of each other if (currentAction.timestamp - previousAction.timestamp > 300) { return false; From 6c4d266e4fd0aa918698eb143e9cd88e633cd982 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 23 Sep 2020 21:50:43 -0700 Subject: [PATCH 04/34] fix starting point of flex items --- package-lock.json | 22 ++++++++++------------ package.json | 2 +- src/style/StyleSheet.js | 2 +- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index c4eae3ab69ba..5f0c77f71f9c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11284,6 +11284,11 @@ "integrity": "sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g=", "dev": true }, + "memoize-one": { + "version": "5.1.1", + "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-5.1.1.tgz", + "integrity": "sha512-HKeeBpWvqiVJD57ZUAsJNm71eHTykffzcLZVYWiVfQeI1rJtuEaS7hQiEpWfVVk18donPwJEcFKIkCmPJNOhHA==" + }, "memory-fs": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/memory-fs/-/memory-fs-0.4.1.tgz", @@ -15362,20 +15367,13 @@ } } }, - "react-window-reversed": { - "version": "1.4.1", - "resolved": "https://registry.npmjs.org/react-window-reversed/-/react-window-reversed-1.4.1.tgz", - "integrity": "sha512-bVfkAdr+6psOJZp27rzoKg3xg5oVkDFumzpOE2HHVR9yt+5MkX4HGsLeA0NZ2mwLcJ82YsLy3GyNLPW7J5G4OQ==", + "react-window": { + "version": "1.8.5", + "resolved": "https://registry.npmjs.org/react-window/-/react-window-1.8.5.tgz", + "integrity": "sha512-HeTwlNa37AFa8MDZFZOKcNEkuF2YflA0hpGPiTT9vR7OawEt+GZbfM6wqkBahD3D3pUjIabQYzsnY/BSJbgq6Q==", "requires": { "@babel/runtime": "^7.0.0", - "memoize-one": "^3.1.1" - }, - "dependencies": { - "memoize-one": { - "version": "3.1.1", - "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-3.1.1.tgz", - "integrity": "sha512-YqVh744GsMlZu6xkhGslPSqSurOv6P+kLN2J3ysBZfagLcL5FdRK/0UpgLoL8hwjjEvvAVkjJZyFP+1T6p1vgA==" - } + "memoize-one": ">=3.1.1 <6" } }, "read-config-file": { diff --git a/package.json b/package.json index 834e43b2bcf3..08b73e56ccb9 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "react-router-dom": "^5.2.0", "react-router-native": "^5.2.0", "react-web-config": "^1.0.0", - "react-window-reversed": "^1.4.1", + "react-window": "^1.8.5", "save": "^2.4.0", "underscore": "^1.10.2" }, diff --git a/src/style/StyleSheet.js b/src/style/StyleSheet.js index b2c2c22111ed..a2037ec5802b 100644 --- a/src/style/StyleSheet.js +++ b/src/style/StyleSheet.js @@ -452,7 +452,7 @@ const styles = { chatContentScrollView: { flexGrow: 1, - justifyContent: 'flex-end', + justifyContent: 'flex-start', paddingVertical: 16, }, From 941a8ebcb92d5ad4bb71bd5f28954e46b8fdbec9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 01:09:51 -0700 Subject: [PATCH 05/34] Fix up comments --- src/components/InvertedFlatList/index.js | 67 ++++++++++++------- .../InvertedFlatList/index.native.js | 1 + 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index 856155259c93..3b954cbe0cb6 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -1,16 +1,17 @@ -import React, {forwardRef, createContext} from 'react'; +import React, {forwardRef, createContext, Component} from 'react'; import {View} from 'react-native'; import {VariableSizeList} from 'react-window'; +import styles from '../../style/StyleSheet'; const ReactWindowContext = createContext({}); -const MINIMUM_ROW_HEIGHT = 42; +const DEFAULT_ROW_HEIGHT = 42; /** * This is the innermost element and we are passing it as a custom * component so that we can overwrite some styles and simulate * an inverse FlatList with items starting from the bottom of the - * scroll position. react-window has no "reverse" feature so we've - * built something similar around the existing API. + * scroll position by adding additional margin. react-window has no + * inverted feature so this works around the existing API. */ const innerElement = forwardRef((props, ref) => ( @@ -37,22 +38,22 @@ const innerElement = forwardRef((props, ref) => ( )); /** - * This component is an alternate implementation of FlatList for web. - * FlatList with an inverted prop does not work correctly on web but - * works fine for mobile so we are using react-window here to create - * our inverted list scroller so eke out extra performance on web. + * This component is a recreation of FlatList for web. + * FlatList when used with an "inverted" prop does not work + * correctly on web but works fine for mobile so we are + * using react-window here to create our inverted list + * scroller to improve performance on web. */ -class InvertedFlatList extends React.Component { +class InvertedFlatList extends Component { constructor(props) { super(props); - this.sizeMap = {}; - - this.state = { - listHeight: 0, - }; - this.getSize = this.getSize.bind(this); + + // Stores each item's computed height after it renders + // once and is reference for the life of the component + this.sizeMap = {}; + this.state = {listHeight: 0}; } componentDidMount() { @@ -62,26 +63,37 @@ class InvertedFlatList extends React.Component { this.scrollToEnd(); }); + // This allows us to call this.listRef.scrollToEnd() from + // the parent component where this will be used. this.props.forwardedRef({ scrollToEnd: () => this.scrollToEnd(), }); } shouldComponentUpdate(prevProps, prevState) { + // We only need to update when the data length changes + // or the list height changes (since we are calculating the + // height on the first render pass) return prevProps.data.length !== this.props.data.length || prevState.listHeight !== this.state.listHeight; } /** + * Returns a previously recorded size or the default. + * + * The default is not a minimum, but the initial height + * to pass to react-window so that we can calculate and + * cache the actual height. + * * @param {Number} index * @returns {Number} */ getSize(index) { - return this.sizeMap[index] || MINIMUM_ROW_HEIGHT; + return this.sizeMap[index] || DEFAULT_ROW_HEIGHT; } /** - * Scroll to end implementation for web. + * ScrollToEnd implementation. Similar to FlatListInstance.scrollToEnd() */ scrollToEnd() { this.listRef.scrollToItem(this.props.data.length - 1, 'end'); @@ -90,10 +102,13 @@ class InvertedFlatList extends React.Component { render() { return ( this.list = el} > {({index, style}) => ( + // Do not modify or remove these styles they are + // required by react-window to function correctly
{this.props.renderItem({ item: this.props.data[index], index, onLayout: ({nativeEvent}) => { + const computedHeight = nativeEvent.layout.height; + if (computedHeight === DEFAULT_ROW_HEIGHT) { + throw new Error('InvertedFlatList rendered row height equal to the constant default height.'); + } const prevSize = this.sizeMap[index] || 0; - if (prevSize !== nativeEvent.layout.height) { - this.sizeMap[index] = nativeEvent.layout.height; + if (prevSize !== computedHeight) { + this.sizeMap[index] = computedHeight; this.listRef.resetAfterIndex(0); } }, - // Minimum row height is a magic number. In the event that we + // Default row height is a magic number. In the event that we // have a row that is the exact same size we will get caught in - // an infinite loop. Do not set row heights to this! - needsLayoutCalculation: style.height === MINIMUM_ROW_HEIGHT, + // an infinite loop. Avoid setting row heights to this. + needsLayoutCalculation: style.height === DEFAULT_ROW_HEIGHT, })}
)} diff --git a/src/components/InvertedFlatList/index.native.js b/src/components/InvertedFlatList/index.native.js index e84d5cb5dc3f..b256f581b37f 100644 --- a/src/components/InvertedFlatList/index.native.js +++ b/src/components/InvertedFlatList/index.native.js @@ -6,6 +6,7 @@ import {FlatList} from 'react-native'; */ export default forwardRef((props, forwardedRef) => ( Date: Thu, 24 Sep 2020 01:10:24 -0700 Subject: [PATCH 06/34] space --- src/components/InvertedFlatList/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index 3b954cbe0cb6..dcf3d0aacf02 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -123,7 +123,7 @@ class InvertedFlatList extends Component { innerElementType={innerElement} > {({index, style}) => ( - // Do not modify or remove these styles they are + // Do not modify or remove these styles they are // required by react-window to function correctly
{this.props.renderItem({ From be1191425041cbc065916fb84046406ffbf2a4e8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 01:22:27 -0700 Subject: [PATCH 07/34] more comments --- src/components/InvertedFlatList/index.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index dcf3d0aacf02..5026f0cacfc8 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -102,6 +102,7 @@ class InvertedFlatList extends Component { render() { return ( {({index, style}) => ( + // Do not modify or remove these styles they are // required by react-window to function correctly
@@ -134,10 +136,13 @@ class InvertedFlatList extends Component { if (computedHeight === DEFAULT_ROW_HEIGHT) { throw new Error('InvertedFlatList rendered row height equal to the constant default height.'); } + + // Check out previous size against the computedHeight returned when + // our renderItem layouts. If there's any difference then we reset const prevSize = this.sizeMap[index] || 0; if (prevSize !== computedHeight) { this.sizeMap[index] = computedHeight; - this.listRef.resetAfterIndex(0); + this.listRef.resetAfterIndex(index); } }, From f4b578df5f55cc43c5b57b679d7c226565dc9737 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 10:52:17 -0700 Subject: [PATCH 08/34] use a boolean flag for needs layout. fix scroll to bottom --- src/components/InvertedFlatList/index.js | 36 ++++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index 5026f0cacfc8..a819d8cf46de 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -4,7 +4,7 @@ import {VariableSizeList} from 'react-window'; import styles from '../../style/StyleSheet'; const ReactWindowContext = createContext({}); -const DEFAULT_ROW_HEIGHT = 42; +const INITIAL_ROW_HEIGHT = 42; /** * This is the innermost element and we are passing it as a custom @@ -20,7 +20,6 @@ const innerElement = forwardRef((props, ref) => ( const top = dimensions.top || 0; const height = dimensions.height || 0; const difference = height - top - innerHeight; - return (
{ + this.outerRef.scrollTo({top: this.outerRef.scrollHeight}); + setTimeout(() => { + this.outerRef.scrollTo({top: this.outerRef.scrollHeight}); + }, 300); + }, 300); } render() { @@ -133,23 +141,15 @@ class InvertedFlatList extends Component { index, onLayout: ({nativeEvent}) => { const computedHeight = nativeEvent.layout.height; - if (computedHeight === DEFAULT_ROW_HEIGHT) { - throw new Error('InvertedFlatList rendered row height equal to the constant default height.'); + if (this.didLayout[index]) { + return; } - // Check out previous size against the computedHeight returned when - // our renderItem layouts. If there's any difference then we reset - const prevSize = this.sizeMap[index] || 0; - if (prevSize !== computedHeight) { - this.sizeMap[index] = computedHeight; - this.listRef.resetAfterIndex(index); - } + this.sizeMap[index] = computedHeight; + this.listRef.resetAfterIndex(index); + this.didLayout[index] = true; }, - - // Default row height is a magic number. In the event that we - // have a row that is the exact same size we will get caught in - // an infinite loop. Avoid setting row heights to this. - needsLayoutCalculation: style.height === DEFAULT_ROW_HEIGHT, + needsLayoutCalculation: !this.didLayout[index], })}
)} From d9f5efe9ed5d7a246ad0a4c09261146d40e40ef5 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 11:16:11 -0700 Subject: [PATCH 09/34] Improve comments and clean up --- src/page/home/MainView.js | 1 - src/page/home/report/ReportActionItem.js | 26 +++++++++++++++++------- src/style/StyleSheet.js | 8 ++++++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/page/home/MainView.js b/src/page/home/MainView.js index ce9962b449c0..6260b66b267a 100644 --- a/src/page/home/MainView.js +++ b/src/page/home/MainView.js @@ -34,7 +34,6 @@ const MainView = (props) => { return ( {!displayAsGroup && } diff --git a/src/style/StyleSheet.js b/src/style/StyleSheet.js index a2037ec5802b..8f4f00bd6a6e 100644 --- a/src/style/StyleSheet.js +++ b/src/style/StyleSheet.js @@ -118,6 +118,14 @@ const styles = { display: 'none', }, + opacity1: { + opacity: 1, + }, + + opacity0: { + opacity: 0, + }, + textP: { color: colors.text, fontSize: 15, From 179645371fb8530678e4265a3007cc1778f4d542 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 14:23:22 -0700 Subject: [PATCH 10/34] Render all reports at once --- src/components/InvertedFlatList/index.js | 22 ++++---- src/page/home/MainView.js | 65 +++++++++++++++++------ src/page/home/report/ReportActionItem.js | 10 ++-- src/page/home/report/ReportActionsView.js | 39 +++++++------- src/page/home/report/ReportView.js | 6 ++- 5 files changed, 88 insertions(+), 54 deletions(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index a819d8cf46de..eacdf738cac6 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -1,4 +1,5 @@ import React, {forwardRef, createContext, Component} from 'react'; +import _ from 'underscore'; import {View} from 'react-native'; import {VariableSizeList} from 'react-window'; import styles from '../../style/StyleSheet'; @@ -53,15 +54,12 @@ class InvertedFlatList extends Component { // once and is reference for the life of the component this.sizeMap = {}; this.didLayout = {}; - this.state = {listHeight: 0}; } componentDidMount() { // Set the height of the list after the component mounts // and then scroll to the bottom. - this.setState({listHeight: this.list.offsetHeight}, () => { - this.scrollToEnd(); - }); + this.scrollToEnd(); // This allows us to call this.listRef.scrollToEnd() from // the parent component where this will be used. @@ -70,12 +68,12 @@ class InvertedFlatList extends Component { }); } - shouldComponentUpdate(prevProps, prevState) { - // We only need to update when the data length changes - // or the list height changes (since we are calculating the - // height on the first render pass) - return prevProps.data.length !== this.props.data.length - || prevState.listHeight !== this.state.listHeight; + componentDidUpdate(prevProps) { + if (!prevProps.isInView && this.props.isInView) { + this.setState({didRender: false}, () => { + this.scrollToEnd(); + }); + } } /** @@ -117,11 +115,11 @@ class InvertedFlatList extends Component { value={{dimensions: this.list ? this.list.getBoundingClientRect() : {}}} > this.list = el} > { - const reportIDInURL = parseInt(props.match.params.reportID, 10); - if (!_.size(props.reports) || !reportIDInURL) { - return null; - } +class MainView extends Component { + render() { + const reportIDInURL = parseInt(this.props.match.params.reportID, 10); - return ( - - - - ); -}; + if (!_.size(this.props.reports) || !reportIDInURL) { + return null; + } + + // The styles for each of our reports. Basically, they are all hidden except for the one matching the + // reportID in the URL + let activeReportID; + const reportStyles = _.reduce(this.props.reports, (memo, report) => { + const isActiveReport = reportIDInURL === report.reportID; + const finalData = {...memo}; + let reportStyle; + + if (isActiveReport) { + activeReportID = report.reportID; + reportStyle = [styles.dFlex, styles.flex1]; + } else { + reportStyle = [styles.dNone]; + } + + finalData[report.reportID] = [reportStyle]; + return finalData; + }, {}); + + return ( + this.mainView = el} + > + {_.map(this.props.reports, report => ( + + + + ))} + + ); + } +} MainView.propTypes = propTypes; MainView.defaultProps = defaultProps; diff --git a/src/page/home/report/ReportActionItem.js b/src/page/home/report/ReportActionItem.js index 90744facbded..87bb5a46fda0 100644 --- a/src/page/home/report/ReportActionItem.js +++ b/src/page/home/report/ReportActionItem.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {Component} from 'react'; import {View} from 'react-native'; import PropTypes from 'prop-types'; import ReportActionItemSingle from './ReportActionItemSingle'; @@ -27,7 +27,7 @@ const defaultProps = { onLayout: () => {}, }; -class ReportActionItem extends React.Component { +class ReportActionItem extends Component { shouldComponentUpdate(nextProps) { // Allow this component to update if the needsLayoutCalculation prop changes // so we can make the component visible. @@ -40,13 +40,13 @@ class ReportActionItem extends React.Component { return true; } - // If the action's sequenceNumber changes then we'll update it - return nextProps.action.sequenceNumber !== this.props.action.sequenceNumber; + // If the item sequenceNumber changes then we'll update it + return this.props.action.sequenceNumber !== nextProps.action.sequenceNumber; } render() { const {action, displayAsGroup} = this.props; - const viewStyle = this.props.needsLayoutCalculation ? [styles.opacity0] : [styles.opacity1]; + const viewStyle = this.props.needsLayoutCalculation ? styles.opacity0 : styles.opacity1; return ( + + + ); + } + + // If we only have the created action then no one has left a comment + if (_.size(this.props.reportActions) === 1) { return ( Be the first person to comment! @@ -140,6 +142,7 @@ class ReportActionsView extends React.Component { this.actionListElement = el} data={data} + height={this.props.height} renderItem={({ item, index, @@ -156,6 +159,7 @@ class ReportActionsView extends React.Component { bounces={false} contentContainerStyle={[styles.chatContentScrollView]} keyExtractor={item => `${item.action.sequenceNumber}`} + isInView={this.props.isActiveReport} /> ); } @@ -164,11 +168,8 @@ class ReportActionsView extends React.Component { ReportActionsView.propTypes = propTypes; ReportActionsView.defaultProps = defaultProps; -export default compose( - withRouter, - withIon({ - reportActions: { - key: ({reportID}) => `${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, - }, - }), -)(ReportActionsView); +export default withIon({ + reportActions: { + key: ({reportID}) => `${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, + }, +})(ReportActionsView); diff --git a/src/page/home/report/ReportView.js b/src/page/home/report/ReportView.js index 642afd1df829..b389c7360442 100644 --- a/src/page/home/report/ReportView.js +++ b/src/page/home/report/ReportView.js @@ -24,7 +24,11 @@ class ReportView extends React.PureComponent { const shouldShowComposeForm = this.props.isActiveReport; return ( - + {shouldShowComposeForm && ( Date: Thu, 24 Sep 2020 16:11:20 -0700 Subject: [PATCH 11/34] mush better --- src/components/InvertedFlatList/index.js | 164 ------------------ .../InvertedFlatList/index.native.js | 15 -- src/page/home/report/ReportActionsView.js | 3 +- 3 files changed, 2 insertions(+), 180 deletions(-) delete mode 100644 src/components/InvertedFlatList/index.js delete mode 100644 src/components/InvertedFlatList/index.native.js diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js deleted file mode 100644 index eacdf738cac6..000000000000 --- a/src/components/InvertedFlatList/index.js +++ /dev/null @@ -1,164 +0,0 @@ -import React, {forwardRef, createContext, Component} from 'react'; -import _ from 'underscore'; -import {View} from 'react-native'; -import {VariableSizeList} from 'react-window'; -import styles from '../../style/StyleSheet'; - -const ReactWindowContext = createContext({}); -const INITIAL_ROW_HEIGHT = 42; - -/** - * This is the innermost element and we are passing it as a custom - * component so that we can overwrite some styles and simulate - * an inverse FlatList with items starting from the bottom of the - * scroll position by adding additional margin. react-window has no - * inverted feature so this works around the existing API. - */ -const innerElement = forwardRef((props, ref) => ( - - {({dimensions}) => { - const innerHeight = props.style.height; - const top = dimensions.top || 0; - const height = dimensions.height || 0; - const difference = height - top - innerHeight; - return ( -
0 ? difference : 0, - }} - /> - ); - }} - -)); - -/** - * This component is a recreation of FlatList for web. - * FlatList when used with an "inverted" prop does not work - * correctly on web but works fine for mobile so we are - * using react-window here to create our inverted list - * scroller to improve performance on web. - */ -class InvertedFlatList extends Component { - constructor(props) { - super(props); - - this.getSize = this.getSize.bind(this); - - // Stores each item's computed height after it renders - // once and is reference for the life of the component - this.sizeMap = {}; - this.didLayout = {}; - } - - componentDidMount() { - // Set the height of the list after the component mounts - // and then scroll to the bottom. - this.scrollToEnd(); - - // This allows us to call this.listRef.scrollToEnd() from - // the parent component where this will be used. - this.props.forwardedRef({ - scrollToEnd: () => this.scrollToEnd(), - }); - } - - componentDidUpdate(prevProps) { - if (!prevProps.isInView && this.props.isInView) { - this.setState({didRender: false}, () => { - this.scrollToEnd(); - }); - } - } - - /** - * Returns a previously recorded size or the default. - * - * The default is not a minimum, but the initial height - * to pass to react-window so that we can calculate and - * cache the actual height. - * - * @param {Number} index - * @returns {Number} - */ - getSize(index) { - return this.sizeMap[index] || INITIAL_ROW_HEIGHT; - } - - /** - * ScrollToEnd implementation. Similar to FlatListInstance.scrollToEnd() - */ - scrollToEnd() { - // This bit of inception is to fix accurate scrolling to end. - // Just one scroll will get us to the bottom but new items will - // render offsetting the scroll position so we need to scroll once more. - setTimeout(() => { - this.outerRef.scrollTo({top: this.outerRef.scrollHeight}); - setTimeout(() => { - this.outerRef.scrollTo({top: this.outerRef.scrollHeight}); - }, 300); - }, 300); - } - - render() { - return ( - - this.list = el} - > - this.listRef = el} - overscanCount={1} - innerRef={el => this.innerRef = el} - outerRef={el => this.outerRef = el} - innerElementType={innerElement} - > - {({index, style}) => ( - - // Do not modify or remove these styles they are - // required by react-window to function correctly -
- {this.props.renderItem({ - item: this.props.data[index], - index, - onLayout: ({nativeEvent}) => { - const computedHeight = nativeEvent.layout.height; - if (this.didLayout[index]) { - return; - } - - this.sizeMap[index] = computedHeight; - this.listRef.resetAfterIndex(index); - this.didLayout[index] = true; - }, - needsLayoutCalculation: !this.didLayout[index], - })} -
- )} - - - - ); - } -} - -export default forwardRef((props, forwardedRef) => ( - // eslint-disable-next-line react/jsx-props-no-spreading - -)); diff --git a/src/components/InvertedFlatList/index.native.js b/src/components/InvertedFlatList/index.native.js deleted file mode 100644 index b256f581b37f..000000000000 --- a/src/components/InvertedFlatList/index.native.js +++ /dev/null @@ -1,15 +0,0 @@ -import React, {forwardRef} from 'react'; -import {FlatList} from 'react-native'; - -/** - * Inverted FlatList for native is just the base FlatList - */ -export default forwardRef((props, forwardedRef) => ( - -)); diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index ce8003c98f1b..14cf507379b5 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -116,7 +116,8 @@ class ReportActionsView extends React.Component { getItems() { return _.sortBy(this.props.reportActions, 'sequenceNumber') .filter(action => action.actionName === 'ADDCOMMENT') - .map((item, index) => ({action: item, index})); + .map((item, index) => ({action: item, index})) + .reverse(); } render() { From 2c81b346d8f9b14a3904cae437110d6834fc410a Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 17:01:38 -0700 Subject: [PATCH 12/34] get FlatList to work hehe --- src/components/InvertedFlatList.js | 79 +++++++++++++++++++++++ src/page/home/MainView.js | 8 +-- src/page/home/report/ReportActionsView.js | 7 +- src/page/home/report/ReportView.js | 1 - 4 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 src/components/InvertedFlatList.js diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js new file mode 100644 index 000000000000..2604da9d63a1 --- /dev/null +++ b/src/components/InvertedFlatList.js @@ -0,0 +1,79 @@ +import React, {forwardRef, Component} from 'react'; +import _ from 'underscore'; +import lodashGet from 'lodash.get'; +import PropTypes from 'prop-types'; +import {FlatList} from 'react-native'; + +const INITIAL_ROW_HEIGHT = 42; + +const propTypes = { + // Same as FlatList can be any array of anything + data: PropTypes.arrayOf(PropTypes.any), + + // Ref to the underlying FlatList component + innerRef: PropTypes.func.isRequired, + + // Similar to FlatList renderItem however also + // passed an onLayout and needsLayoutCalculation + // property that must be implemented by the component + // being rendered + renderItem: PropTypes.func.isRequired, +}; + +const defaultProps = { + data: [], +}; + +class InvertedFlatList extends Component { + constructor(props) { + super(props); + + // Stores each item's computed height after it renders + // once and is reference for the life of the component + this.sizeMap = {}; + } + + render() { + return ( + { + const size = this.sizeMap[index] || {}; + return { + length: size.length || INITIAL_ROW_HEIGHT, + offset: size.offset || (INITIAL_ROW_HEIGHT * index), + index + }; + }} + renderItem={({item, index}) => this.props.renderItem({ + item, + index, + onLayout: ({nativeEvent}) => { + const computedHeight = nativeEvent.layout.height; + if (this.sizeMap[index]) { + return; + } + const prevHeight = lodashGet(this.sizeMap, [index - 1, 'height'], 0); + const prevOffset = lodashGet(this.sizeMap, [index - 1, 'offset'], 0); + this.sizeMap[index] = { + length: computedHeight, + offset: prevOffset + prevHeight, + }; + }, + needsLayoutCalculation: _.isUndefined(this.sizeMap[index]), + })} + /> + ); + } +} + +InvertedFlatList.propTypes = propTypes; +InvertedFlatList.defaultProps = defaultProps; + +export default forwardRef((props, ref) => ( + // eslint-disable-next-line react/jsx-props-no-spreading + +)); diff --git a/src/page/home/MainView.js b/src/page/home/MainView.js index 54e6aaa434f8..f4b4792ff9e0 100644 --- a/src/page/home/MainView.js +++ b/src/page/home/MainView.js @@ -54,10 +54,7 @@ class MainView extends Component { }, {}); return ( - this.mainView = el} - > + <> {_.map(this.props.reports, report => ( ))} - + ); } } diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 14cf507379b5..2403763d4c13 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -16,10 +16,13 @@ const propTypes = { // The ID of the report actions will be created for reportID: PropTypes.number.isRequired, + // Is this report currently in view? + isActiveReport: PropTypes.bool.isRequired, + /* Ion Props */ // Array of report actions for this report - reportActions: PropTypes.PropTypes.objectOf(PropTypes.shape(ReportActionPropTypes)), + reportActions: PropTypes.objectOf(PropTypes.shape(ReportActionPropTypes)), }; const defaultProps = { @@ -143,7 +146,6 @@ class ReportActionsView extends React.Component { this.actionListElement = el} data={data} - height={this.props.height} renderItem={({ item, index, @@ -160,7 +162,6 @@ class ReportActionsView extends React.Component { bounces={false} contentContainerStyle={[styles.chatContentScrollView]} keyExtractor={item => `${item.action.sequenceNumber}`} - isInView={this.props.isActiveReport} /> ); } diff --git a/src/page/home/report/ReportView.js b/src/page/home/report/ReportView.js index b389c7360442..4032f706c71d 100644 --- a/src/page/home/report/ReportView.js +++ b/src/page/home/report/ReportView.js @@ -27,7 +27,6 @@ class ReportView extends React.PureComponent { {shouldShowComposeForm && ( From e97b820d07e2b251aa784d97f6c7f737faa1716b Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 17:02:01 -0700 Subject: [PATCH 13/34] remove react-window --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 08b73e56ccb9..043471be3983 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,6 @@ "react-router-dom": "^5.2.0", "react-router-native": "^5.2.0", "react-web-config": "^1.0.0", - "react-window": "^1.8.5", "save": "^2.4.0", "underscore": "^1.10.2" }, From 96bc39256ad7390854e438bdb90138070008fcd7 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 17:02:32 -0700 Subject: [PATCH 14/34] update package json --- package-lock.json | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/package-lock.json b/package-lock.json index 5f0c77f71f9c..d71ca160cec8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11284,11 +11284,6 @@ "integrity": "sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g=", "dev": true }, - "memoize-one": { - "version": "5.1.1", - "resolved": "https://registry.npmjs.org/memoize-one/-/memoize-one-5.1.1.tgz", - "integrity": "sha512-HKeeBpWvqiVJD57ZUAsJNm71eHTykffzcLZVYWiVfQeI1rJtuEaS7hQiEpWfVVk18donPwJEcFKIkCmPJNOhHA==" - }, "memory-fs": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/memory-fs/-/memory-fs-0.4.1.tgz", @@ -15367,15 +15362,6 @@ } } }, - "react-window": { - "version": "1.8.5", - "resolved": "https://registry.npmjs.org/react-window/-/react-window-1.8.5.tgz", - "integrity": "sha512-HeTwlNa37AFa8MDZFZOKcNEkuF2YflA0hpGPiTT9vR7OawEt+GZbfM6wqkBahD3D3pUjIabQYzsnY/BSJbgq6Q==", - "requires": { - "@babel/runtime": "^7.0.0", - "memoize-one": ">=3.1.1 <6" - } - }, "read-config-file": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/read-config-file/-/read-config-file-4.0.1.tgz", From 90f62733dce803e0855c15d0c06f3f37a7389434 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 17:08:51 -0700 Subject: [PATCH 15/34] fix up comment --- src/components/InvertedFlatList.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index 2604da9d63a1..3c47da483963 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -28,15 +28,17 @@ class InvertedFlatList extends Component { constructor(props) { super(props); - // Stores each item's computed height after it renders - // once and is reference for the life of the component + // Stores each item's computed height and offset after it + // renders once and is referenced for the life of the component. + // This is essential to getting FlatList inverted to work on web + // and also enables more predictable scrolling on mobile. this.sizeMap = {}; } render() { return ( Date: Thu, 24 Sep 2020 17:30:42 -0700 Subject: [PATCH 16/34] move renderItem to method --- src/page/home/report/ReportActionsView.js | 50 ++++++++++++++++------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index b7bb501a385d..37dcce371654 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -33,9 +33,9 @@ class ReportActionsView extends React.Component { constructor(props) { super(props); + this.renderItem = this.renderItem.bind(this); this.scrollToListBottom = this.scrollToListBottom.bind(this); this.recordMaxAction = this.recordMaxAction.bind(this); - this.sortedReportActions = this.updateSortedReportActions(); } @@ -123,6 +123,36 @@ class ReportActionsView extends React.Component { this.recordMaxAction(); } + /** + * Do not move this or make it an anonymous function it is a method + * so it will not be recreated each time we render an item + * + * See: https://reactnative.dev/docs/optimizing-flatlist-configuration#avoid-anonymous-function-on-renderitem + * + * @param {Object} args + * @param {Object} args.item + * @param {Number} args.index + * @param {Function} args.onLayout + * @param {Boolean} args.needsLayoutCalculation + * + * @returns {React.Component} + */ + renderItem({ + item, + index, + onLayout, + needsLayoutCalculation + }) { + return ( + + ); + } + render() { // Comments have not loaded at all yet show a loading spinner if (!_.size(this.props.reportActions)) { @@ -147,22 +177,14 @@ class ReportActionsView extends React.Component { this.actionListElement = el} data={this.sortedReportActions} - renderItem={({ - item, - index, - onLayout, - needsLayoutCalculation, - }) => ( - - )} + renderItem={this.renderItem} bounces={false} contentContainerStyle={[styles.chatContentScrollView]} keyExtractor={item => `${item.action.sequenceNumber}`} + maxToRenderPerBatch={20} + updateCellsBatchingPeriod={25} + initialNumToRender={1} + windowSize={10} /> ); } From 9f7171c1f8430de443f294845d3123accf406652 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 17:37:45 -0700 Subject: [PATCH 17/34] start with 50 1 is too few --- src/page/home/report/ReportActionsView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 37dcce371654..6ee70b08b88e 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -183,7 +183,7 @@ class ReportActionsView extends React.Component { keyExtractor={item => `${item.action.sequenceNumber}`} maxToRenderPerBatch={20} updateCellsBatchingPeriod={25} - initialNumToRender={1} + initialNumToRender={50} windowSize={10} /> ); From 5b3677e7fea32e36e89dfb983f50bf47cc086023 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 17:43:31 -0700 Subject: [PATCH 18/34] use Scroll to index as end is actualy the top of the list in this case --- src/page/home/report/ReportActionsView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 6ee70b08b88e..91ce2cdb2e74 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -118,7 +118,7 @@ class ReportActionsView extends React.Component { */ scrollToListBottom() { if (this.actionListElement) { - this.actionListElement.scrollToEnd({animated: false}); + this.actionListElement.scrollToIndex({animated: false, index: 0}); } this.recordMaxAction(); } From 6b99ee8475eae9b1e5c5ec08e7047d408d60275c Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 18:24:04 -0700 Subject: [PATCH 19/34] fix the grouping issues --- src/page/home/report/ReportActionsView.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 91ce2cdb2e74..8d1a374f587a 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -80,7 +80,7 @@ class ReportActionsView extends React.Component { * @return {Boolean} */ isConsecutiveActionMadeByPreviousActor(actionIndex) { - const previousAction = this.sortedReportActions[actionIndex - 1]; + const previousAction = this.sortedReportActions[actionIndex + 1]; const currentAction = this.sortedReportActions[actionIndex]; // It's OK for there to be no previous action, and in that case, false will be returned @@ -90,11 +90,11 @@ class ReportActionsView extends React.Component { } // Comments are only grouped if they happen within 5 minutes of each other - if (currentAction.timestamp - previousAction.timestamp > 300) { + if (currentAction.action.timestamp - previousAction.action.timestamp > 300) { return false; } - return currentAction.actorEmail === previousAction.actorEmail; + return currentAction.action.actorEmail === previousAction.action.actorEmail; } /** From 617da5058ede61177c63a5d387079d6b10c029a9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 19:04:49 -0700 Subject: [PATCH 20/34] make more epic --- src/components/InvertedFlatList.js | 44 ++++++++++++++---------- src/page/home/report/ReportActionItem.js | 30 ++-------------- 2 files changed, 27 insertions(+), 47 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index 3c47da483963..fd40817d14b0 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -1,8 +1,7 @@ import React, {forwardRef, Component} from 'react'; -import _ from 'underscore'; import lodashGet from 'lodash.get'; import PropTypes from 'prop-types'; -import {FlatList} from 'react-native'; +import {FlatList, View} from 'react-native'; const INITIAL_ROW_HEIGHT = 42; @@ -35,6 +34,25 @@ class InvertedFlatList extends Component { this.sizeMap = {}; } + /** + * Measure an item and return the cache the offset and length. + * + * @param {React.NativeSyntheticEvent} nativeEvent + * @param {Number} index + */ + measureItemLayout(nativeEvent, index) { + const computedHeight = nativeEvent.layout.height; + if (this.sizeMap[index]) { + return; + } + const prevHeight = lodashGet(this.sizeMap, [index - 1, 'height'], 0); + const prevOffset = lodashGet(this.sizeMap, [index - 1, 'offset'], 0); + this.sizeMap[index] = { + length: computedHeight, + offset: prevOffset + prevHeight, + }; + } + render() { return ( this.props.renderItem({ - item, - index, - onLayout: ({nativeEvent}) => { - const computedHeight = nativeEvent.layout.height; - if (this.sizeMap[index]) { - return; - } - const prevHeight = lodashGet(this.sizeMap, [index - 1, 'height'], 0); - const prevOffset = lodashGet(this.sizeMap, [index - 1, 'offset'], 0); - this.sizeMap[index] = { - length: computedHeight, - offset: prevOffset + prevHeight, - }; - }, - needsLayoutCalculation: _.isUndefined(this.sizeMap[index]), - })} + renderItem={({item, index}) => ( + this.measureItemLayout(nativeEvent, index)}> + {this.props.renderItem({item, index})} + + )} /> ); } diff --git a/src/page/home/report/ReportActionItem.js b/src/page/home/report/ReportActionItem.js index 87bb5a46fda0..a95fc291acc6 100644 --- a/src/page/home/report/ReportActionItem.js +++ b/src/page/home/report/ReportActionItem.js @@ -1,10 +1,8 @@ import React, {Component} from 'react'; -import {View} from 'react-native'; import PropTypes from 'prop-types'; import ReportActionItemSingle from './ReportActionItemSingle'; import ReportActionPropTypes from './ReportActionPropTypes'; import ReportActionItemGrouped from './ReportActionItemGrouped'; -import styles from '../../../style/StyleSheet'; const propTypes = { // All the data of the action item @@ -12,29 +10,10 @@ const propTypes = { // Should the comment have the appearance of being grouped with the previous comment? displayAsGroup: PropTypes.bool.isRequired, - - // This is a performance feature used to tell this component that we want it - // to call onLayout when it renders. We need this so that we can calculate the size of - // this component and pass it to our InvertedFlatList implementation. - needsLayoutCalculation: PropTypes.bool, - - // Called when the view layout completes - onLayout: PropTypes.func, -}; - -const defaultProps = { - needsLayoutCalculation: false, - onLayout: () => {}, }; class ReportActionItem extends Component { shouldComponentUpdate(nextProps) { - // Allow this component to update if the needsLayoutCalculation prop changes - // so we can make the component visible. - if (nextProps.needsLayoutCalculation !== this.props.needsLayoutCalculation) { - return true; - } - // If the grouping changes then we want to update the UI if (nextProps.displayAsGroup !== this.props.displayAsGroup) { return true; @@ -46,20 +25,15 @@ class ReportActionItem extends Component { render() { const {action, displayAsGroup} = this.props; - const viewStyle = this.props.needsLayoutCalculation ? styles.opacity0 : styles.opacity1; return ( - + <> {!displayAsGroup && } {displayAsGroup && } - + ); } } ReportActionItem.propTypes = propTypes; -ReportActionItem.defaultProps = defaultProps; export default ReportActionItem; From 11928e9fbecf76fb159ea508429ab0b132a23a5c Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 19:07:08 -0700 Subject: [PATCH 21/34] remove opacity stuff --- src/style/StyleSheet.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/style/StyleSheet.js b/src/style/StyleSheet.js index 8f4f00bd6a6e..a2037ec5802b 100644 --- a/src/style/StyleSheet.js +++ b/src/style/StyleSheet.js @@ -118,14 +118,6 @@ const styles = { display: 'none', }, - opacity1: { - opacity: 1, - }, - - opacity0: { - opacity: 0, - }, - textP: { color: colors.text, fontSize: 15, From eccd61cf38c70e631cfe66a84176636dad3e8055 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 19:11:54 -0700 Subject: [PATCH 22/34] bind more methods for performance or something --- src/components/InvertedFlatList.js | 55 +++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index fd40817d14b0..e2da7710167a 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -27,6 +27,9 @@ class InvertedFlatList extends Component { constructor(props) { super(props); + this.renderItem = this.renderItem.bind(this); + this.getItemLayout = this.getItemLayout.bind(this); + // Stores each item's computed height and offset after it // renders once and is referenced for the life of the component. // This is essential to getting FlatList inverted to work on web @@ -53,6 +56,43 @@ class InvertedFlatList extends Component { }; } + /** + * Render item method wraps the real itm to render in a + * View component so we can attach an onLayout handler and + * measure it when it renders. + * + * @param {Object} params + * @param {Object} params.item + * @param {Number} params.index + * + * @return {React.Component} + */ + renderItem({item, index}) { + return ( + this.measureItemLayout(nativeEvent, index)}> + {this.props.renderItem({item, index})} + + ); + } + + /** + * Return default or previously cached height for + * a renderItem row + * + * @param {*} data + * @param {Number} index + * + * @return {Object} + */ + getItemLayout(data, index) { + const size = this.sizeMap[index] || {}; + return { + length: size.length || INITIAL_ROW_HEIGHT, + offset: size.offset || (INITIAL_ROW_HEIGHT * index), + index + }; + } + render() { return ( { - const size = this.sizeMap[index] || {}; - return { - length: size.length || INITIAL_ROW_HEIGHT, - offset: size.offset || (INITIAL_ROW_HEIGHT * index), - index - }; - }} - renderItem={({item, index}) => ( - this.measureItemLayout(nativeEvent, index)}> - {this.props.renderItem({item, index})} - - )} + renderItem={this.renderItem} + getItemLayout={this.getItemLayout} /> ); } From b43de0c3b774b7bb8b4d2630b990e5054ecd0d85 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 19:22:08 -0700 Subject: [PATCH 23/34] offset 0 is the secret --- src/components/InvertedFlatList.js | 53 ++++++++++++++---------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index e2da7710167a..c6178e253aff 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -1,5 +1,4 @@ import React, {forwardRef, Component} from 'react'; -import lodashGet from 'lodash.get'; import PropTypes from 'prop-types'; import {FlatList, View} from 'react-native'; @@ -12,10 +11,8 @@ const propTypes = { // Ref to the underlying FlatList component innerRef: PropTypes.func.isRequired, - // Similar to FlatList renderItem however also - // passed an onLayout and needsLayoutCalculation - // property that must be implemented by the component - // being rendered + // Same as FlatList although we wrap it in a measuring helper + // before passing to the actual FlatList component renderItem: PropTypes.func.isRequired, }; @@ -31,14 +28,32 @@ class InvertedFlatList extends Component { this.getItemLayout = this.getItemLayout.bind(this); // Stores each item's computed height and offset after it - // renders once and is referenced for the life of the component. + // renders once and is then referenced for the life of this component. // This is essential to getting FlatList inverted to work on web - // and also enables more predictable scrolling on mobile. + // and also enables more predictable scrolling on native platforms. this.sizeMap = {}; } /** - * Measure an item and return the cache the offset and length. + * Return default or previously cached height for + * a renderItem row + * + * @param {*} data + * @param {Number} index + * + * @return {Object} + */ + getItemLayout(data, index) { + const size = this.sizeMap[index] || {}; + return { + length: size.length || INITIAL_ROW_HEIGHT, + offset: size.offset || (INITIAL_ROW_HEIGHT * index), + index + }; + } + + /** + * Measure item and cache the returned offset and length * * @param {React.NativeSyntheticEvent} nativeEvent * @param {Number} index @@ -48,11 +63,9 @@ class InvertedFlatList extends Component { if (this.sizeMap[index]) { return; } - const prevHeight = lodashGet(this.sizeMap, [index - 1, 'height'], 0); - const prevOffset = lodashGet(this.sizeMap, [index - 1, 'offset'], 0); this.sizeMap[index] = { length: computedHeight, - offset: prevOffset + prevHeight, + offset: 0, }; } @@ -75,24 +88,6 @@ class InvertedFlatList extends Component { ); } - /** - * Return default or previously cached height for - * a renderItem row - * - * @param {*} data - * @param {Number} index - * - * @return {Object} - */ - getItemLayout(data, index) { - const size = this.sizeMap[index] || {}; - return { - length: size.length || INITIAL_ROW_HEIGHT, - offset: size.offset || (INITIAL_ROW_HEIGHT * index), - index - }; - } - render() { return ( Date: Thu, 24 Sep 2020 19:26:38 -0700 Subject: [PATCH 24/34] Fix up some comments --- src/components/InvertedFlatList.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index c6178e253aff..68d7b0a72b86 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -27,8 +27,8 @@ class InvertedFlatList extends Component { this.renderItem = this.renderItem.bind(this); this.getItemLayout = this.getItemLayout.bind(this); - // Stores each item's computed height and offset after it - // renders once and is then referenced for the life of this component. + // Stores each item's computed height after it renders + // once and is then referenced for the life of this component. // This is essential to getting FlatList inverted to work on web // and also enables more predictable scrolling on native platforms. this.sizeMap = {}; @@ -47,13 +47,13 @@ class InvertedFlatList extends Component { const size = this.sizeMap[index] || {}; return { length: size.length || INITIAL_ROW_HEIGHT, - offset: size.offset || (INITIAL_ROW_HEIGHT * index), + offset: INITIAL_ROW_HEIGHT * index, index }; } /** - * Measure item and cache the returned offset and length + * Measure item and cache the returned length (a.k.a. height) * * @param {React.NativeSyntheticEvent} nativeEvent * @param {Number} index @@ -65,7 +65,6 @@ class InvertedFlatList extends Component { } this.sizeMap[index] = { length: computedHeight, - offset: 0, }; } From 6897b95327bcbaca1e4a1e5e053f1de130c9f605 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 19:28:31 -0700 Subject: [PATCH 25/34] fix typo --- src/components/InvertedFlatList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index 68d7b0a72b86..c1ee812c1a51 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -69,7 +69,7 @@ class InvertedFlatList extends Component { } /** - * Render item method wraps the real itm to render in a + * Render item method wraps the prop renderItem to render in a * View component so we can attach an onLayout handler and * measure it when it renders. * From 9b34893047048145e38d839711a5007db51377d5 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 21:25:50 -0700 Subject: [PATCH 26/34] Lighten up report action items --- src/components/InvertedFlatList.js | 16 +++++++--- src/page/home/report/ReportActionItem.js | 13 +++------ .../home/report/ReportActionItemGrouped.js | 4 +-- .../home/report/ReportActionItemMessage.js | 10 ++++--- .../home/report/ReportActionItemSingle.js | 29 +++++++------------ src/page/home/report/ReportActionsView.js | 1 + src/style/StyleSheet.js | 10 +------ 7 files changed, 36 insertions(+), 47 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index c1ee812c1a51..59559d8a04d3 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -2,8 +2,6 @@ import React, {forwardRef, Component} from 'react'; import PropTypes from 'prop-types'; import {FlatList, View} from 'react-native'; -const INITIAL_ROW_HEIGHT = 42; - const propTypes = { // Same as FlatList can be any array of anything data: PropTypes.arrayOf(PropTypes.any), @@ -14,6 +12,11 @@ const propTypes = { // Same as FlatList although we wrap it in a measuring helper // before passing to the actual FlatList component renderItem: PropTypes.func.isRequired, + + // This must be set to the minimum size of one of the + // renderItem rows. Web will have issues with FlatList + // if this is inaccurate. + initialRowHeight: PropTypes.number.isRequired, }; const defaultProps = { @@ -34,6 +37,11 @@ class InvertedFlatList extends Component { this.sizeMap = {}; } + shouldComponentUpdate(prevProps) { + // The FlatList itself should only re-render if items are added + return prevProps.data.length !== this.props.data.length; + } + /** * Return default or previously cached height for * a renderItem row @@ -46,8 +54,8 @@ class InvertedFlatList extends Component { getItemLayout(data, index) { const size = this.sizeMap[index] || {}; return { - length: size.length || INITIAL_ROW_HEIGHT, - offset: INITIAL_ROW_HEIGHT * index, + length: size.length || this.props.initialRowHeight, + offset: this.props.initialRowHeight * index, index }; } diff --git a/src/page/home/report/ReportActionItem.js b/src/page/home/report/ReportActionItem.js index a95fc291acc6..a81e2e703280 100644 --- a/src/page/home/report/ReportActionItem.js +++ b/src/page/home/report/ReportActionItem.js @@ -15,20 +15,15 @@ const propTypes = { class ReportActionItem extends Component { shouldComponentUpdate(nextProps) { // If the grouping changes then we want to update the UI - if (nextProps.displayAsGroup !== this.props.displayAsGroup) { - return true; - } - - // If the item sequenceNumber changes then we'll update it - return this.props.action.sequenceNumber !== nextProps.action.sequenceNumber; + return nextProps.displayAsGroup !== this.props.displayAsGroup; } render() { - const {action, displayAsGroup} = this.props; return ( <> - {!displayAsGroup && } - {displayAsGroup && } + {!this.props.displayAsGroup + ? + : } ); } diff --git a/src/page/home/report/ReportActionItemGrouped.js b/src/page/home/report/ReportActionItemGrouped.js index 3ed21a4de5f7..10601f7aa761 100644 --- a/src/page/home/report/ReportActionItemGrouped.js +++ b/src/page/home/report/ReportActionItemGrouped.js @@ -16,9 +16,7 @@ class ReportActionItemGrouped extends React.PureComponent { return ( - - - + ); diff --git a/src/page/home/report/ReportActionItemMessage.js b/src/page/home/report/ReportActionItemMessage.js index 906596e880ff..c185b4fca784 100644 --- a/src/page/home/report/ReportActionItemMessage.js +++ b/src/page/home/report/ReportActionItemMessage.js @@ -1,6 +1,8 @@ import React from 'react'; +import {View} from 'react-native'; import PropTypes from 'prop-types'; import _ from 'underscore'; +import styles from '../../../style/StyleSheet'; import ReportActionItemFragment from './ReportActionItemFragment'; import ReportActionPropTypes from './ReportActionPropTypes'; @@ -10,15 +12,15 @@ const propTypes = { }; const ReportActionItemMessage = ({action}) => ( - <> - {_.map(_.compact(action.message), fragment => ( + + {_.map(_.compact(action.message), (fragment, index) => ( ))} - + ); ReportActionItemMessage.propTypes = propTypes; diff --git a/src/page/home/report/ReportActionItemSingle.js b/src/page/home/report/ReportActionItemSingle.js index abe3beeb690d..8d265780cd9e 100644 --- a/src/page/home/report/ReportActionItemSingle.js +++ b/src/page/home/report/ReportActionItemSingle.js @@ -22,28 +22,21 @@ class ReportActionItemSingle extends React.PureComponent { : action.avatar; return ( - - - - - + - {action.person.map(fragment => ( - - - + {_.map(action.person, (fragment, index) => ( + ))} - - - - - - + + ); diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 8d1a374f587a..adcbbed0e62e 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -181,6 +181,7 @@ class ReportActionsView extends React.Component { bounces={false} contentContainerStyle={[styles.chatContentScrollView]} keyExtractor={item => `${item.action.sequenceNumber}`} + initialRowHeight={32} maxToRenderPerBatch={20} updateCellsBatchingPeriod={25} initialNumToRender={50} diff --git a/src/style/StyleSheet.js b/src/style/StyleSheet.js index a2037ec5802b..f789548f013c 100644 --- a/src/style/StyleSheet.js +++ b/src/style/StyleSheet.js @@ -161,11 +161,9 @@ const styles = { }, // Actions - actionAvatarWrapper: { - width: 40, - }, actionAvatar: { borderRadius: 20, + marginRight: 8, height: 40, width: 40, }, @@ -473,12 +471,6 @@ const styles = { paddingRight: 20, }, - chatItemLeft: { - display: 'flex', - flexShrink: 0, - marginRight: 8, - }, - chatItemRightGrouped: { flexGrow: 1, flexShrink: 1, From 94cedaca78d6bc2de3b5000c1b62988d49ccc88a Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 22:49:35 -0700 Subject: [PATCH 27/34] move perf things into the InvertedFlatList --- src/components/InvertedFlatList.js | 6 ++++++ src/page/home/report/ReportActionsView.js | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index 59559d8a04d3..42720a5af1af 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -104,6 +104,12 @@ class InvertedFlatList extends Component { inverted renderItem={this.renderItem} getItemLayout={this.getItemLayout} + removeClippedSubviews + initialNumToRender={10} + maxToRenderPerBatch={10} + updateCellsBatchingPeriod={50} + windowSize={20} + bounces={false} /> ); } diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index adcbbed0e62e..909d9c5ba99f 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -178,14 +178,9 @@ class ReportActionsView extends React.Component { ref={el => this.actionListElement = el} data={this.sortedReportActions} renderItem={this.renderItem} - bounces={false} contentContainerStyle={[styles.chatContentScrollView]} keyExtractor={item => `${item.action.sequenceNumber}`} initialRowHeight={32} - maxToRenderPerBatch={20} - updateCellsBatchingPeriod={25} - initialNumToRender={50} - windowSize={10} /> ); } From 195e46ceb08597e0d3c577456dc494b2bfbe410e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 22:54:51 -0700 Subject: [PATCH 28/34] Actually, nothing bad will happen if we have no reports --- src/page/home/MainView.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/page/home/MainView.js b/src/page/home/MainView.js index f4b4792ff9e0..0893326eba5b 100644 --- a/src/page/home/MainView.js +++ b/src/page/home/MainView.js @@ -28,17 +28,11 @@ const defaultProps = { class MainView extends Component { render() { - const reportIDInURL = parseInt(this.props.match.params.reportID, 10); - - if (!_.size(this.props.reports) || !reportIDInURL) { - return null; - } - // The styles for each of our reports. Basically, they are all hidden except for the one matching the // reportID in the URL let activeReportID; const reportStyles = _.reduce(this.props.reports, (memo, report) => { - const isActiveReport = reportIDInURL === report.reportID; + const isActiveReport = parseInt(this.props.match.params.reportID, 10) === report.reportID; const finalData = {...memo}; let reportStyle; From f379508c61c8d73aadc69d02138f2c4f028abc9b Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 24 Sep 2020 23:00:18 -0700 Subject: [PATCH 29/34] remove loading spinner --- src/page/home/report/ReportActionsView.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index 909d9c5ba99f..de5da23bc3fc 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -1,5 +1,5 @@ import React from 'react'; -import {View, Keyboard, ActivityIndicator} from 'react-native'; +import {View, Keyboard} from 'react-native'; import PropTypes from 'prop-types'; import _ from 'underscore'; import lodashGet from 'lodash.get'; @@ -8,7 +8,7 @@ import withIon from '../../../components/withIon'; import {fetchActions, updateLastReadActionID} from '../../../lib/actions/Report'; import IONKEYS from '../../../IONKEYS'; import ReportActionItem from './ReportActionItem'; -import styles, {colors} from '../../../style/StyleSheet'; +import styles from '../../../style/StyleSheet'; import ReportActionPropTypes from './ReportActionPropTypes'; import InvertedFlatList from '../../../components/InvertedFlatList'; @@ -154,13 +154,9 @@ class ReportActionsView extends React.Component { } render() { - // Comments have not loaded at all yet show a loading spinner + // Comments have not loaded at all yet do nothing if (!_.size(this.props.reportActions)) { - return ( - - - - ); + return null; } // If we only have the created action then no one has left a comment From 54783f557c386cff65b5b3b5c2235558fbcea2d8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 25 Sep 2020 13:24:57 -0700 Subject: [PATCH 30/34] Remove performance steps. Improve measuring logic for offset --- src/components/InvertedFlatList.js | 44 +++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList.js index 42720a5af1af..94b699e27c3c 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import React, {forwardRef, Component} from 'react'; import PropTypes from 'prop-types'; import {FlatList, View} from 'react-native'; @@ -52,10 +53,32 @@ class InvertedFlatList extends Component { * @return {Object} */ getItemLayout(data, index) { - const size = this.sizeMap[index] || {}; + const size = this.sizeMap[index]; + + if (size) { + return { + length: size.length, + offset: size.offset, + index, + }; + } + + // If we don't have a size yet means we haven't measured this + // item yet. However, we can still calculate the offset by looking + // at the last size we have recorded (if any) + const lastMeasuredIndex = _.last(_.keys(this.sizeMap)) || 0; + const lastMeasuredItem = this.sizeMap[lastMeasuredIndex]; + return { - length: size.length || this.props.initialRowHeight, - offset: this.props.initialRowHeight * index, + // We haven't measured this so we must return the minimum row height + length: this.props.initialRowHeight, + + // Offset will either be based on the lastMeasuredItem or the index + + // initialRowHeight since we can only assume that all previous items + // have not yet been measured + offset: _.isUndefined(lastMeasuredItem) + ? this.props.initialRowHeight * index + : lastMeasuredItem.offset + this.props.initialRowHeight, index }; } @@ -68,11 +91,22 @@ class InvertedFlatList extends Component { */ measureItemLayout(nativeEvent, index) { const computedHeight = nativeEvent.layout.height; + + // We've already measured this item so we don't need to + // measure it again. if (this.sizeMap[index]) { return; } + + const previousItem = this.sizeMap[index - 1] || {}; + + // If there is no previousItem this can mean we haven't yet measured + // the previous item or that we are at index 0 and there is no previousItem + const previousLength = previousItem.length || 0; + const previousOffset = previousItem.offset || 0; this.sizeMap[index] = { length: computedHeight, + offset: previousLength + previousOffset, }; } @@ -105,10 +139,6 @@ class InvertedFlatList extends Component { renderItem={this.renderItem} getItemLayout={this.getItemLayout} removeClippedSubviews - initialNumToRender={10} - maxToRenderPerBatch={10} - updateCellsBatchingPeriod={50} - windowSize={20} bounces={false} /> ); From 63f13e3863227dbad7d8e7eac3a6bbe58c9ef846 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 25 Sep 2020 14:50:56 -0700 Subject: [PATCH 31/34] Add hack to make scroll direction work on web --- .../BaseInvertedFlatList.js} | 15 +++--- src/components/InvertedFlatList/index.js | 51 +++++++++++++++++++ .../InvertedFlatList/index.native.js | 3 ++ src/lib/CollectionUtils.js | 18 +++++++ src/page/home/report/ReportActionsView.js | 20 ++++++-- 5 files changed, 94 insertions(+), 13 deletions(-) rename src/components/{InvertedFlatList.js => InvertedFlatList/BaseInvertedFlatList.js} (91%) create mode 100644 src/components/InvertedFlatList/index.js create mode 100644 src/components/InvertedFlatList/index.native.js create mode 100644 src/lib/CollectionUtils.js diff --git a/src/components/InvertedFlatList.js b/src/components/InvertedFlatList/BaseInvertedFlatList.js similarity index 91% rename from src/components/InvertedFlatList.js rename to src/components/InvertedFlatList/BaseInvertedFlatList.js index 94b699e27c3c..e753ff4e9bd9 100644 --- a/src/components/InvertedFlatList.js +++ b/src/components/InvertedFlatList/BaseInvertedFlatList.js @@ -2,14 +2,12 @@ import _ from 'underscore'; import React, {forwardRef, Component} from 'react'; import PropTypes from 'prop-types'; import {FlatList, View} from 'react-native'; +import {lastItem} from '../../lib/CollectionUtils'; const propTypes = { // Same as FlatList can be any array of anything data: PropTypes.arrayOf(PropTypes.any), - // Ref to the underlying FlatList component - innerRef: PropTypes.func.isRequired, - // Same as FlatList although we wrap it in a measuring helper // before passing to the actual FlatList component renderItem: PropTypes.func.isRequired, @@ -24,7 +22,7 @@ const defaultProps = { data: [], }; -class InvertedFlatList extends Component { +class BaseInvertedFlatList extends Component { constructor(props) { super(props); @@ -66,8 +64,7 @@ class InvertedFlatList extends Component { // If we don't have a size yet means we haven't measured this // item yet. However, we can still calculate the offset by looking // at the last size we have recorded (if any) - const lastMeasuredIndex = _.last(_.keys(this.sizeMap)) || 0; - const lastMeasuredItem = this.sizeMap[lastMeasuredIndex]; + const lastMeasuredItem = lastItem(this.sizeMap); return { // We haven't measured this so we must return the minimum row height @@ -145,10 +142,10 @@ class InvertedFlatList extends Component { } } -InvertedFlatList.propTypes = propTypes; -InvertedFlatList.defaultProps = defaultProps; +BaseInvertedFlatList.propTypes = propTypes; +BaseInvertedFlatList.defaultProps = defaultProps; export default forwardRef((props, ref) => ( // eslint-disable-next-line react/jsx-props-no-spreading - + )); diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js new file mode 100644 index 000000000000..8b5eb4b1612d --- /dev/null +++ b/src/components/InvertedFlatList/index.js @@ -0,0 +1,51 @@ +import React, {useEffect, useRef, useCallback, forwardRef} from "react"; +import BaseInvertedFlatList from './BaseInvertedFlatList'; + +// This is copied from https://codesandbox.io/s/react-native-dsyse +// It's a HACK alert since FlatList has inverted scrolling on web +const InvertedFlatList = (props) => { + const ref = useRef(null); + + const invertedWheelEvent = useCallback(e => { + ref.current.getScrollableNode().scrollTop -= e.deltaY; + e.preventDefault(); + }, []); + + useEffect(() => { + props.forwardedRef(ref); + }, []); + + useEffect(() => { + const currentRef = ref.current; + if (currentRef != null) { + currentRef + .getScrollableNode() + .addEventListener("wheel", invertedWheelEvent); + + currentRef.setNativeProps({ + style: { + transform: "translate3d(0,0,0) scaleY(-1)" + } + }); + } + + return () => { + if (currentRef != null) { + currentRef + .getScrollableNode() + .removeEventListener("wheel", invertedWheelEvent); + } + }; + }, [ref, invertedWheelEvent]); + + return ( + + ); +}; + +export default forwardRef((props, ref) => ( + +)); diff --git a/src/components/InvertedFlatList/index.native.js b/src/components/InvertedFlatList/index.native.js new file mode 100644 index 000000000000..c4e2f7c079e9 --- /dev/null +++ b/src/components/InvertedFlatList/index.native.js @@ -0,0 +1,3 @@ +import BaseInvertedFlatList from './BaseInvertedFlatList'; +BaseInvertedFlatList.displayName = 'InvertedFlatList'; +export default BaseInvertedFlatList; diff --git a/src/lib/CollectionUtils.js b/src/lib/CollectionUtils.js new file mode 100644 index 000000000000..4e5f285fff49 --- /dev/null +++ b/src/lib/CollectionUtils.js @@ -0,0 +1,18 @@ +import _ from 'underscore'; + +/** + * Return the highest item in a numberd collection + * + * e.g. {1: '1', 2: '2', 3: '3'} -> '3' + * + * @param {Object} object + * @return {*} + */ +export function lastItem(object = {}) { + const lastKey = _.last(_.keys(object)) || 0; + return object[lastKey]; +} + +export default { + lastItem, +}; diff --git a/src/page/home/report/ReportActionsView.js b/src/page/home/report/ReportActionsView.js index de5da23bc3fc..18b2ffead18d 100644 --- a/src/page/home/report/ReportActionsView.js +++ b/src/page/home/report/ReportActionsView.js @@ -11,6 +11,7 @@ import ReportActionItem from './ReportActionItem'; import styles from '../../../style/StyleSheet'; import ReportActionPropTypes from './ReportActionPropTypes'; import InvertedFlatList from '../../../components/InvertedFlatList'; +import {lastItem} from '../../../lib/CollectionUtils'; const propTypes = { // The ID of the report actions will be created for @@ -45,10 +46,18 @@ class ReportActionsView extends React.Component { } componentDidUpdate(prevProps) { - // When the number of actions change, wait three seconds, then record the max action - // This will make the unread indicator go away if you receive comments in the same chat you're looking at - if (this.props.isActiveReport && _.size(prevProps.reportActions) !== _.size(this.props.reportActions)) { - setTimeout(this.recordMaxAction, 3000); + if (_.size(prevProps.reportActions) !== _.size(this.props.reportActions)) { + // If a new comment is added and it's from the current user scroll to the bottom otherwise + // leave the user positioned where they are now in the list. + if (lastItem(this.props.reportActions).actorEmail === this.props.session.email) { + this.scrollToListBottom(); + } + + // When the number of actions change, wait three seconds, then record the max action + // This will make the unread indicator go away if you receive comments in the same chat you're looking at + if (this.props.isActiveReport) { + setTimeout(this.recordMaxAction, 3000); + } } } @@ -189,4 +198,7 @@ export default withIon({ reportActions: { key: ({reportID}) => `${IONKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, }, + session: { + key: IONKEYS.SESSION, + }, })(ReportActionsView); From 7c6130161b3cd31f25e9a0cdf5443a9162734b13 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 25 Sep 2020 15:05:57 -0700 Subject: [PATCH 32/34] Fix style --- src/components/InvertedFlatList/index.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/components/InvertedFlatList/index.js b/src/components/InvertedFlatList/index.js index 8b5eb4b1612d..baa394645e27 100644 --- a/src/components/InvertedFlatList/index.js +++ b/src/components/InvertedFlatList/index.js @@ -1,4 +1,9 @@ -import React, {useEffect, useRef, useCallback, forwardRef} from "react"; +import React, { + useEffect, + useRef, + useCallback, + forwardRef +} from 'react'; import BaseInvertedFlatList from './BaseInvertedFlatList'; // This is copied from https://codesandbox.io/s/react-native-dsyse @@ -6,7 +11,7 @@ import BaseInvertedFlatList from './BaseInvertedFlatList'; const InvertedFlatList = (props) => { const ref = useRef(null); - const invertedWheelEvent = useCallback(e => { + const invertedWheelEvent = useCallback((e) => { ref.current.getScrollableNode().scrollTop -= e.deltaY; e.preventDefault(); }, []); @@ -20,12 +25,12 @@ const InvertedFlatList = (props) => { if (currentRef != null) { currentRef .getScrollableNode() - .addEventListener("wheel", invertedWheelEvent); + .addEventListener('wheel', invertedWheelEvent); currentRef.setNativeProps({ style: { - transform: "translate3d(0,0,0) scaleY(-1)" - } + transform: 'translate3d(0,0,0) scaleY(-1)' + }, }); } @@ -33,13 +38,14 @@ const InvertedFlatList = (props) => { if (currentRef != null) { currentRef .getScrollableNode() - .removeEventListener("wheel", invertedWheelEvent); + .removeEventListener('wheel', invertedWheelEvent); } }; }, [ref, invertedWheelEvent]); return ( @@ -47,5 +53,6 @@ const InvertedFlatList = (props) => { }; export default forwardRef((props, ref) => ( + // eslint-disable-next-line react/jsx-props-no-spreading )); From 4305201a9b39bc8c8cbecd53085aa5f116bb9279 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 25 Sep 2020 15:18:03 -0700 Subject: [PATCH 33/34] new lines yum --- src/components/InvertedFlatList/index.native.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/InvertedFlatList/index.native.js b/src/components/InvertedFlatList/index.native.js index c4e2f7c079e9..a98fa587721b 100644 --- a/src/components/InvertedFlatList/index.native.js +++ b/src/components/InvertedFlatList/index.native.js @@ -1,3 +1,4 @@ import BaseInvertedFlatList from './BaseInvertedFlatList'; + BaseInvertedFlatList.displayName = 'InvertedFlatList'; export default BaseInvertedFlatList; From 924831ab8511794361e78f619bfa60863db53981 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 25 Sep 2020 13:18:14 -1000 Subject: [PATCH 34/34] Update src/lib/CollectionUtils.js Co-authored-by: Andrew Gable --- src/lib/CollectionUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/CollectionUtils.js b/src/lib/CollectionUtils.js index 4e5f285fff49..b4501d1a5dee 100644 --- a/src/lib/CollectionUtils.js +++ b/src/lib/CollectionUtils.js @@ -1,7 +1,7 @@ import _ from 'underscore'; /** - * Return the highest item in a numberd collection + * Return the highest item in a numbered collection * * e.g. {1: '1', 2: '2', 3: '3'} -> '3' *