Skip to content

perf: recalculate getOrderedReportIDs only on web and desktop #46276

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

Merged
dangrous merged 2 commits into from
Jul 31, 2024
Merged

perf: recalculate getOrderedReportIDs only on web and desktop #46276

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3fe2bd0
perf: recalculate getorderedreportid only on web and desktop
hurali97 Jul 26, 2024
12554f0
fix: allow calling getOrderedReportIDs if orderedReportIDs are empty
hurali97 Jul 29, 2024
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
16 changes: 14 additions & 2 deletions src/hooks/useReportIDs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {Message} from '@src/types/onyx/ReportAction';
import useActiveWorkspace from './useActiveWorkspace';
import useCurrentReportID from './useCurrentReportID';
import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails';
import useResponsiveLayout from './useResponsiveLayout';

type PolicySelector = Pick<OnyxTypes.Policy, 'type' | 'name' | 'avatarURL' | 'employeeList'>;
type ReportActionsSelector = Array<Pick<OnyxTypes.ReportAction, 'reportActionID' | 'actionName' | 'errors' | 'message' | 'originalMessage'>>;
Expand Down Expand Up @@ -86,6 +87,7 @@ function ReportIDsContextProvider({
const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT);
const [betas] = useOnyx(ONYXKEYS.BETAS);

const {shouldUseNarrowLayout} = useResponsiveLayout();
const {accountID} = useCurrentUserPersonalDetails();
const currentReportIDValue = useCurrentReportID();
const derivedCurrentReportID = currentReportIDForTests ?? currentReportIDValue?.currentReportID;
Expand Down Expand Up @@ -118,7 +120,17 @@ function ReportIDsContextProvider({
// we first generate the list as if there was no current report, then we check if
// the current report is missing from the list, which should very rarely happen. In this
// case we re-generate the list a 2nd time with the current report included.
if (derivedCurrentReportID && derivedCurrentReportID !== '-1' && orderedReportIDs.indexOf(derivedCurrentReportID) === -1) {

// We also execute the following logic if `shouldUseNarrowLayout` is false because this is
// requirement for web and desktop. Consider a case, where we have report with expenses and we click on
// any expense, a new LHN item is added in the list and is visible on web and desktop. But on mobile, we
// just navigate to the screen with expense details, so there seems no point to execute this logic on mobile.
if (
(!shouldUseNarrowLayout || orderedReportIDs.length === 0) &&
Comment thread
dangrous marked this conversation as resolved.
derivedCurrentReportID &&
derivedCurrentReportID !== '-1' &&
orderedReportIDs.indexOf(derivedCurrentReportID) === -1
) {
return {orderedReportIDs: getOrderedReportIDs(derivedCurrentReportID), currentReportID: derivedCurrentReportID ?? '-1', policyMemberAccountIDs};
}

Expand All @@ -127,7 +139,7 @@ function ReportIDsContextProvider({
currentReportID: derivedCurrentReportID ?? '-1',
policyMemberAccountIDs,
};
}, [getOrderedReportIDs, orderedReportIDs, derivedCurrentReportID, policyMemberAccountIDs]);
}, [getOrderedReportIDs, orderedReportIDs, derivedCurrentReportID, policyMemberAccountIDs, shouldUseNarrowLayout]);

return <ReportIDsContext.Provider value={contextValue}>{children}</ReportIDsContext.Provider>;
}
Expand Down