Skip to content

Commit

Permalink
Merge pull request #32124 from margelo/perunt/chat-loader-flickering
Browse files Browse the repository at this point in the history
  • Loading branch information
roryabraham authored Dec 6, 2023
2 parents 6f1839e + cc68278 commit 5681090
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 7 deletions.
18 changes: 14 additions & 4 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,14 +179,14 @@ function isTransactionThread(parentReportAction: OnyxEntry<ReportAction>): boole
* This gives us a stable order even in the case of multiple reportActions created on the same millisecond
*
*/
function getSortedReportActions(reportActions: ReportAction[] | null, shouldSortInDescendingOrder = false): ReportAction[] {
function getSortedReportActions(reportActions: ReportAction[] | null, shouldSortInDescendingOrder = false, shouldMarkTheFirstItemAsNewest = false): ReportAction[] {
if (!Array.isArray(reportActions)) {
throw new Error(`ReportActionsUtils.getSortedReportActions requires an array, received ${typeof reportActions}`);
}

const invertedMultiplier = shouldSortInDescendingOrder ? -1 : 1;

return reportActions?.filter(Boolean).sort((first, second) => {
const sortedActions = reportActions?.filter(Boolean).sort((first, second) => {
// First sort by timestamp
if (first.created !== second.created) {
return (first.created < second.created ? -1 : 1) * invertedMultiplier;
Expand All @@ -206,6 +206,16 @@ function getSortedReportActions(reportActions: ReportAction[] | null, shouldSort
// will be consistent across all users and devices
return (first.reportActionID < second.reportActionID ? -1 : 1) * invertedMultiplier;
});

// If shouldMarkTheFirstItemAsNewest is true, label the first reportAction as isNewestReportAction
if (shouldMarkTheFirstItemAsNewest && sortedActions?.length > 0) {
sortedActions[0] = {
...sortedActions[0],
isNewestReportAction: true,
};
}

return sortedActions;
}

/**
Expand Down Expand Up @@ -467,12 +477,12 @@ function filterOutDeprecatedReportActions(reportActions: ReportActions | null):
* to ensure they will always be displayed in the same order (in case multiple actions have the same timestamp).
* This is all handled with getSortedReportActions() which is used by several other methods to keep the code DRY.
*/
function getSortedReportActionsForDisplay(reportActions: ReportActions | null): ReportAction[] {
function getSortedReportActionsForDisplay(reportActions: ReportActions | null, shouldMarkTheFirstItemAsNewest = false): ReportAction[] {
const filteredReportActions = Object.entries(reportActions ?? {})
.filter(([key, reportAction]) => shouldReportActionBeVisible(reportAction, key))
.map((entry) => entry[1]);
const baseURLAdjustedReportActions = filteredReportActions.map((reportAction) => replaceBaseURL(reportAction));
return getSortedReportActions(baseURLAdjustedReportActions, true);
return getSortedReportActions(baseURLAdjustedReportActions, true, shouldMarkTheFirstItemAsNewest);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/ReportScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ export default compose(
reportActions: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getReportID(route)}`,
canEvict: false,
selector: ReportActionsUtils.getSortedReportActionsForDisplay,
selector: (reportActions) => ReportActionsUtils.getSortedReportActionsForDisplay(reportActions, true),
},
report: {
key: ({route}) => `${ONYXKEYS.COLLECTION.REPORT}${getReportID(route)}`,
Expand Down
5 changes: 3 additions & 2 deletions src/pages/home/report/ReportActionsView.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ function ReportActionsView(props) {

const isFocused = useIsFocused();
const reportID = props.report.reportID;
const hasNewestReportAction = lodashGet(props.reportActions[0], 'isNewestReportAction');

/**
* @returns {Boolean}
Expand Down Expand Up @@ -200,7 +201,7 @@ function ReportActionsView(props) {
const loadNewerChats = useMemo(
() =>
_.throttle(({distanceFromStart}) => {
if (props.isLoadingNewerReportActions || props.isLoadingInitialReportActions) {
if (props.isLoadingNewerReportActions || props.isLoadingInitialReportActions || hasNewestReportAction) {
return;
}

Expand All @@ -222,7 +223,7 @@ function ReportActionsView(props) {
const newestReportAction = _.first(props.reportActions);
Report.getNewerActions(reportID, newestReportAction.reportActionID);
}, 500),
[props.isLoadingNewerReportActions, props.isLoadingInitialReportActions, props.reportActions, reportID],
[props.isLoadingNewerReportActions, props.isLoadingInitialReportActions, props.reportActions, reportID, hasNewestReportAction],
);

/**
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/ReportAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ type ReportActionBase = {
isAttachment?: boolean;
childRecentReceiptTransactionIDs?: Record<string, string>;
reportID?: string;

/** We manually add this field while sorting to detect the end of the list */
isNewestReportAction?: boolean;
};

type ReportAction = ReportActionBase & OriginalMessage;
Expand Down
61 changes: 61 additions & 0 deletions tests/unit/ReportActionsUtilsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,71 @@ describe('ReportActionsUtils', () => {
message: [{html: 'I have changed the task'}],
},
];

const result = ReportActionsUtils.getSortedReportActionsForDisplay(input);
input.pop();
expect(result).toStrictEqual(input);
});

describe('getSortedReportActionsForDisplay with marked the first reportAction', () => {
it('should filter out non-whitelisted actions', () => {
const input = [
{
created: '2022-11-13 22:27:01.825',
reportActionID: '8401445780099176',
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
message: [{html: 'Hello world'}],
},
{
created: '2022-11-12 22:27:01.825',
reportActionID: '6401435781022176',
actionName: CONST.REPORT.ACTIONS.TYPE.CREATED,
message: [{html: 'Hello world'}],
},
{
created: '2022-11-11 22:27:01.825',
reportActionID: '2962390724708756',
actionName: CONST.REPORT.ACTIONS.TYPE.IOU,
message: [{html: 'Hello world'}],
},
{
created: '2022-11-10 22:27:01.825',
reportActionID: '1609646094152486',
actionName: CONST.REPORT.ACTIONS.TYPE.RENAMED,
message: [{html: 'Hello world'}],
},
{
created: '2022-11-09 22:27:01.825',
reportActionID: '8049485084562457',
actionName: CONST.REPORT.ACTIONS.TYPE.POLICYCHANGELOG.UPDATE_FIELD,
message: [{html: 'updated the Approval Mode from "Submit and Approve" to "Submit and Close"'}],
},
{
created: '2022-11-08 22:27:06.825',
reportActionID: '1661970171066216',
actionName: CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENTQUEUED,
message: [{html: 'Waiting for the bank account'}],
},
{
created: '2022-11-06 22:27:08.825',
reportActionID: '1661970171066220',
actionName: CONST.REPORT.ACTIONS.TYPE.TASKEDITED,
message: [{html: 'I have changed the task'}],
},
];

const resultWithoutNewestFlag = ReportActionsUtils.getSortedReportActionsForDisplay(input);
const resultWithNewestFlag = ReportActionsUtils.getSortedReportActionsForDisplay(input, true);
input.pop();
// Mark the newest report action as the newest report action
resultWithoutNewestFlag[0] = {
...resultWithoutNewestFlag[0],
isNewestReportAction: true,
};
expect(resultWithoutNewestFlag).toStrictEqual(resultWithNewestFlag);
});
});

it('should filter out closed actions', () => {
const input = [
{
Expand Down

0 comments on commit 5681090

Please sign in to comment.