Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[No QA] Use onLayout to record chat switching time #4140

Merged
merged 2 commits into from
Jul 20, 2021
Merged

[No QA] Use onLayout to record chat switching time #4140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
22fe53b
use onLayout instead of debouncing report action items
marcaaron Jul 19, 2021
9afedd1
Fix timing for chat switching by using onLayout
marcaaron Jul 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/libs/actions/Timing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ 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 = '', offset = 0) {
function end(eventName, secondaryName = '') {
if (eventName in timestampData) {
const eventTime = Date.now() - timestampData[eventName] - offset;
const eventTime = Date.now() - timestampData[eventName];
const grafanaEventName = secondaryName
? `expensify.cash.${eventName}.${secondaryName}`
: `expensify.cash.${eventName}`;
Expand Down
4 changes: 0 additions & 4 deletions src/pages/home/report/ReportActionItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ const propTypes = {
/** Draft message - if this is set the comment is in 'edit' mode */
draftMessage: PropTypes.string,

/** Runs when the view enclosing the chat message lays out indicating it has rendered */
onLayout: PropTypes.func.isRequired,

...withLocalizePropTypes,
...windowDimensionsPropTypes,
};
Expand Down Expand Up @@ -289,7 +286,6 @@ class ReportActionItem extends Component {
|| this.state.isPopoverVisible
|| this.props.draftMessage,
)}
onLayout={this.props.onLayout}
>
{!this.props.displayAsGroup
? (
Expand Down
26 changes: 10 additions & 16 deletions src/pages/home/report/ReportActionsView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,11 @@ class ReportActionsView extends React.Component {
this.renderCell = this.renderCell.bind(this);
this.scrollToListBottom = this.scrollToListBottom.bind(this);
this.onVisibilityChange = this.onVisibilityChange.bind(this);
this.recordTimeToMeasureItemLayout = this.recordTimeToMeasureItemLayout.bind(this);
this.loadMoreChats = this.loadMoreChats.bind(this);
this.sortedReportActions = [];

// We are debouncing this call with a specific delay so that when all items in the list layout we can measure
// the total time it took to complete.
this.recordTimeToMeasureItemLayout = _.debounce(
this.recordTimeToMeasureItemLayout.bind(this),
CONST.TIMING.REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME,
);
this.didLayout = false;

this.state = {
isLoadingMoreChats: false,
Expand Down Expand Up @@ -199,10 +195,6 @@ 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);

unsubscribeFromReportChannel(this.props.reportID);
Expand Down Expand Up @@ -319,13 +311,15 @@ class ReportActionsView extends React.Component {
}

/**
* 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.
* Runs when the FlatList finishes laying out
*/
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);
if (this.didLayout) {
return;
}

this.didLayout = true;
Timing.end(CONST.TIMING.SWITCH_REPORT, CONST.TIMING.COLD);
}

/**
Expand Down Expand Up @@ -374,7 +368,6 @@ class ReportActionsView extends React.Component {
isMostRecentIOUReportAction={item.action.sequenceNumber === this.mostRecentIOUReportSequenceNumber}
hasOutstandingIOU={this.props.report.hasOutstandingIOU}
index={index}
onLayout={this.recordTimeToMeasureItemLayout}
/>
);
}
Expand Down Expand Up @@ -415,6 +408,7 @@ class ReportActionsView extends React.Component {
? <ActivityIndicator size="small" color={themeColors.spinner} />
: null}
keyboardShouldPersistTaps="handled"
onLayout={this.recordTimeToMeasureItemLayout}
/>
);
}
Expand Down