Skip to content

[CP Staging] Fix next steps loading state #62768

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
merged 1 commit into from
May 26, 2025
+17 −3
Merged

[CP Staging] Fix next steps loading state #62768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,25 @@ type MoneyReportHeaderProps = {
// eslint-disable-next-line react/no-unused-prop-types
transactionThreadReportID: string | undefined;

/** whether we are loading report data in openReport command */
isLoadingInitialReportActions?: boolean;

/** Whether back button should be displayed in header */
shouldDisplayBackButton?: boolean;

/** Method to trigger when pressing close button of the header */
onBackButtonPress: () => void;
};

function MoneyReportHeader({policy, report: moneyRequestReport, transactionThreadReportID, reportActions, shouldDisplayBackButton = false, onBackButtonPress}: MoneyReportHeaderProps) {
function MoneyReportHeader({
policy,
report: moneyRequestReport,
transactionThreadReportID,
reportActions,
isLoadingInitialReportActions,
shouldDisplayBackButton = false,
onBackButtonPress,
}: MoneyReportHeaderProps) {
// We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to use a correct layout for the hold expense modal https://github.com/Expensify/App/pull/47990#issuecomment-2362382026
// eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth
const {shouldUseNarrowLayout, isSmallScreenWidth, isMediumScreenWidth} = useResponsiveLayout();
Expand Down Expand Up @@ -892,7 +903,8 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
/>
</View>
)}
{shouldShowNextStep && (optimisticNextStep?.message?.length ? <MoneyReportHeaderStatusBar nextStep={optimisticNextStep} /> : <MoneyReportHeaderStatusBarSkeleton />)}
{shouldShowNextStep && !!optimisticNextStep?.message?.length && <MoneyReportHeaderStatusBar nextStep={optimisticNextStep} />}
{shouldShowNextStep && !optimisticNextStep?.message?.length && !!isLoadingInitialReportActions && <MoneyReportHeaderStatusBarSkeleton />}
Copy link
Contributor

@DylanDylann DylanDylann May 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about this approach?

Suggested change
{shouldShowNextStep && !optimisticNextStep?.message?.length && !!isLoadingInitialReportActions && <MoneyReportHeaderStatusBarSkeleton />}
const [nextStep, nextStepResult] = useOnyx(`${ONYXKEYS.COLLECTION.NEXT_STEP}${moneyRequestReport?.reportID}`, {canBeMissing: true});
..........................
{shouldShowNextStep && !optimisticNextStep?.message?.length && isLoadingOnyxValue(nextStepResult) && <MoneyReportHeaderStatusBarSkeleton />}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your solution, I am not sure why we use isLoadingInitialReportActions as a loading flag for nextStep data

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using isLoadingInitialReportActions because that value is set to "loading" when openReport command is running, openReport command is also responsible for downloading the nextSteps, so I use is as a loading indicator for nextSteps

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately isLoadingOnyxValue approach doesn't work, because when nextSteps are not present in onyx then the isLoadingOnyxValue returns false and it doesn't know that it's being downloaded by openReport

{!!statusBarProps && (
<MoneyRequestHeaderStatusBar
icon={statusBarProps.icon}
Expand Down
3 changes: 2 additions & 1 deletion src/components/MoneyRequestReportView/MoneyRequestReportView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe
policy={policy}
reportActions={reportActions}
transactionThreadReportID={transactionThreadReportID}
isLoadingInitialReportActions={isLoadingInitialReportActions}
shouldDisplayBackButton
onBackButtonPress={() => {
if (!backToRoute) {
Expand All @@ -177,7 +178,7 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe
}}
/>
),
[activeWorkspaceID, backToRoute, isTransactionThreadView, parentReportAction, policy, report, reportActions, transactionThreadReportID],
[activeWorkspaceID, backToRoute, isLoadingInitialReportActions, isTransactionThreadView, parentReportAction, policy, report, reportActions, transactionThreadReportID],
);

if (!!(isLoadingInitialReportActions && reportActions.length === 0 && !isOffline) || shouldWaitForData) {
Expand Down
1 change: 1 addition & 0 deletions src/pages/home/ReportScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) {
report={report}
policy={policy}
transactionThreadReportID={transactionThreadReportID}
isLoadingInitialReportActions={reportMetadata.isLoadingInitialReportActions}
reportActions={reportActions}
onBackButtonPress={onBackButtonPress}
/>
Expand Down
Loading