From 99a972b7f4f5fca7460371a9f181afcc1f9db9eb Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 4 May 2021 07:43:29 -1000 Subject: [PATCH 1/5] move to report actions view --- src/pages/home/report/ReportActionsView.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 2661a20c400d..e6642a5a04cc 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -106,7 +106,6 @@ class ReportActionsView extends React.Component { this.keyboardEvent = Keyboard.addListener('keyboardDidShow', this.scrollToListBottom); this.recordMaxAction(); fetchActions(this.props.reportID); - Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); } shouldComponentUpdate(nextProps, nextState) { @@ -380,6 +379,9 @@ class ReportActionsView extends React.Component { contentContainerStyle={[styles.chatContentScrollView]} keyExtractor={item => `${item.action.sequenceNumber}`} initialRowHeight={32} + onLayout={() => { + Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); + }} onEndReached={this.loadMoreChats} onEndReachedThreshold={0.75} ListFooterComponent={this.state.isLoadingMoreChats From 6ee24bd27488263be809f1bb41078ba9156781c3 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 4 May 2021 08:37:47 -1000 Subject: [PATCH 2/5] Use the debounce time in CONST --- src/CONST.js | 1 + src/libs/actions/Timing.js | 5 +++-- src/pages/home/report/ReportActionItem.js | 5 ++++- src/pages/home/report/ReportActionsView.js | 18 +++++++++++++++--- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/CONST.js b/src/CONST.js index daefbe0ec6be..5b42ea456501 100644 --- a/src/CONST.js +++ b/src/CONST.js @@ -64,6 +64,7 @@ const CONST = { HOMEPAGE_REPORTS_LOADED: 'homepage_reports_loaded', SWITCH_REPORT: 'switch_report', COLD: 'cold', + REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME: 1000, }, MESSAGES: { // eslint-disable-next-line max-len diff --git a/src/libs/actions/Timing.js b/src/libs/actions/Timing.js index 4d1eae31a36f..9dd385750d3e 100644 --- a/src/libs/actions/Timing.js +++ b/src/libs/actions/Timing.js @@ -17,10 +17,11 @@ function start(eventName) { * * @param {String} eventName - event name used as timestamp key * @param {String} [secondaryName] - optional secondary event name, passed to grafana + * @param {Number} [offset] - optional param to offset the time */ -function end(eventName, secondaryName = '') { +function end(eventName, secondaryName = '', offset = 0) { if (eventName in timestampData) { - const eventTime = Date.now() - timestampData[eventName]; + const eventTime = Date.now() - timestampData[eventName] - offset; const grafanaEventName = secondaryName ? `expensify.cash.${eventName}.${secondaryName}` : `expensify.cash.${eventName}`; diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 82878a2c384b..5c93426948f9 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -38,6 +38,9 @@ const propTypes = { // Should we display the new indicator on top of the comment? shouldDisplayNewIndicator: PropTypes.bool.isRequired, + + // Runs when the view layouts + onLayout: PropTypes.func.isRequired, }; const defaultProps = { @@ -122,7 +125,7 @@ class ReportActionItem extends Component { {this.props.shouldDisplayNewIndicator && ( )} - + {!this.props.displayAsGroup ? ( diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index e6642a5a04cc..2a9f8a08de80 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -85,6 +85,10 @@ class ReportActionsView extends React.Component { this.onVisibilityChange = this.onVisibilityChange.bind(this); this.loadMoreChats = this.loadMoreChats.bind(this); this.startRecordMaxActionTimer = this.startRecordMaxActionTimer.bind(this); + this.recordTimeToMeasureItemLayout = _.debounce( + this.recordTimeToMeasureItemLayout.bind(this), + CONST.TIMING.REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME, + ); this.sortedReportActions = []; this.timers = []; @@ -306,6 +310,16 @@ class ReportActionsView extends React.Component { this.recordMaxAction(); } + /** + * Runs each time a ReportActionItem is laid out. This method is debounced so we wait until the component has + * finished laying out items before recording the chat as switched. + */ + recordTimeToMeasureItemLayout() { + // We are offsetting the time measurement here so that we can subtract our debounce time from the initial time + // and get the actual time it took to load the report + Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD, CONST.TIMING.REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME); + } + /** * This function overrides the CellRendererComponent (defaults to a plain View), giving each ReportActionItem a * higher z-index than the one below it. This prevents issues where the ReportActionContextMenu overlapping between @@ -351,6 +365,7 @@ class ReportActionsView extends React.Component { isMostRecentIOUReportAction={item.action.sequenceNumber === this.mostRecentIOUReportSequenceNumber} iouReportID={this.props.report.iouReportID} hasOutstandingIOU={this.props.report.hasOutstandingIOU} + onLayout={this.recordTimeToMeasureItemLayout} /> ); } @@ -379,9 +394,6 @@ class ReportActionsView extends React.Component { contentContainerStyle={[styles.chatContentScrollView]} keyExtractor={item => `${item.action.sequenceNumber}`} initialRowHeight={32} - onLayout={() => { - Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD); - }} onEndReached={this.loadMoreChats} onEndReachedThreshold={0.75} ListFooterComponent={this.state.isLoadingMoreChats From ba525f0f97260cd046ff7638f8935f24c6e6fcd6 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 4 May 2021 09:15:25 -1000 Subject: [PATCH 3/5] improve comment --- src/pages/home/report/ReportActionItem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 5c93426948f9..ea67e52c5771 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -39,7 +39,7 @@ const propTypes = { // Should we display the new indicator on top of the comment? shouldDisplayNewIndicator: PropTypes.bool.isRequired, - // Runs when the view layouts + // Runs when the view enclosing the chat message lays out indicating it has rendered onLayout: PropTypes.func.isRequired, }; From 778400c0d7da8b5381bcd5759748a8171d4ae24f Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 May 2021 10:52:48 -1000 Subject: [PATCH 4/5] Cancel the debounce to avoid firing it after switching --- src/pages/home/report/ReportActionsView.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 0cab5dc1954c..dae0bb5e4f02 100644 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -192,6 +192,10 @@ class ReportActionsView extends React.Component { this.keyboardEvent.remove(); } + // We must cancel the debounce function so that we do not call the function when switching to a new chat before + // the previous one has finished loading completely. + this.recordTimeToMeasureItemLayout.cancel(); + AppState.removeEventListener('change', this.onVisibilityChange); _.each(this.timers, timer => clearTimeout(timer)); From 72d2a94a0319ee343fc36a3e1f65f2a2ed5ebdf9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 7 May 2021 09:52:46 -1000 Subject: [PATCH 5/5] increase debounce time slightl --- src/CONST.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CONST.js b/src/CONST.js index 5b42ea456501..60d444e65fe5 100644 --- a/src/CONST.js +++ b/src/CONST.js @@ -64,7 +64,7 @@ const CONST = { HOMEPAGE_REPORTS_LOADED: 'homepage_reports_loaded', SWITCH_REPORT: 'switch_report', COLD: 'cold', - REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME: 1000, + REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME: 1500, }, MESSAGES: { // eslint-disable-next-line max-len