Update the transaction report to show skeleton part 2 #66165

rinej · 2025-07-15T14:02:21Z

Explanation of Change

Fixed Issues

$ #63253
PROPOSAL: #63253 (comment)

Tests

Open Reports > Expense Reports
Open an expense report with multiple transactions on it
Open one transaction thread
Ensure the transaction details are loaded immediately (since all the transaction data should be available)
Use the arrows in the header to cycle through the expense views
The transaction details should be visible immediately even if the report actions are loading

Tests flow after regression detection:
Original issue: #66109

Go to staging.expensify.com
Open FAB > Create expense > Manual.
Submit two expenses to any user.
Open expense report.
Open any expense row.
Click More.
Hold and Delete option are present in More menu.

Original issue: #66110

Open workspace chat.
Submit a manual expense.
Open the just created report.
Scroll to the top of the report.
Note that a big space with no content can be seen above receipt thumbnail.
User shouldn´t be able to scroll up above the receipt thumbnail on expense report.

Offline tests

QA Steps

Same as tests.

PR Author Checklist

Screenshots/Videos

Android: Native

Transaction-android.mp4

Android: mWeb Chrome

Transaction-androidWeb.mp4

iOS: Native

ios-Transaction.mp4

iOS: mWeb Safari

Transaction-iosWeb.mp4

MacOS: Chrome / Safari

Transaction-Web.mp4

MacOS: Desktop

Transaction-Desktop.mp4

…-skeleton

This reverts commit cf4cd41.

This reverts commit d3d18de.

…-skeleton # Conflicts: # src/pages/home/ReportScreen.tsx

allgandalf

nit pics

allgandalf · 2025-07-18T06:56:19Z

src/pages/home/report/ReportActionsView.tsx

+    /** If this report is a transaction thread report */
+    isReportTransactionThread?: boolean;


Suggested change

/** If this report is a transaction thread report */

isReportTransactionThread?: boolean;

/** If the report is a transaction thread report */

isReportTransactionThread?: boolean;

allgandalf · 2025-07-18T06:58:33Z

src/pages/home/ReportScreen.tsx

-                policyAvatar: reportOnyx.policyAvatar,
-            },
-        [reportOnyx, reportNameValuePairsOnyx, permissions],
+            // This is required to get the transaction data from the parent report, to render the optimistic transaction thread


can you elaborate more on what this means , better for future context

allgandalf · 2025-07-18T06:59:45Z

src/pages/home/ReportScreen.tsx

    const reportIDFromRoute = getNonEmptyStringOnyxID(route.params?.reportID);
    const reportActionIDFromRoute = route?.params?.reportActionID;
+    const parentReportIDFromRoute = route?.params?.parentReportID;
+    const parentReportActionIDFromRoute = route?.params?.parentReportActionID;


can you instead do:

const {reportActionIDFromRoute, parentReportIDFromRoute, parentReportActionIDFromRoute} = route?.params;

allgandalf · 2025-07-18T07:01:22Z

src/components/MoneyRequestHeader.tsx

-        },
-    };
+        }),
+        [translate, parentReportAction, transaction, reportID, report],


can you please fix this warning

allgandalf · 2025-07-18T07:13:04Z

src/components/MoneyRequestHeader.tsx

+    const getStatusIcon: (src: IconAsset) => ReactNode = useCallback(
+        (src) => (
+            <Icon
+                src={src}
+                height={variables.iconSizeSmall}
+                width={variables.iconSizeSmall}
+                fill={theme.icon}
+            />
+        ),
+        [theme.icon],
    );


does the following way to write this not work?:

const getStatusIcon = useCallback((src: IconAsset) => ( <Icon src={src} height={variables.iconSizeSmall} width={variables.iconSizeSmall} fill={theme.icon} /> ), [theme.icon]);

…created action

…-skeleton

melvin-bot · 2025-07-22T10:57:17Z

@allgandalf Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

pac-guerreiro · 2025-07-22T14:21:33Z

@allgandalf all feedback addressed and PR is ready for review! YOU SHALL PASS this PR 😎

github-actions · 2025-07-22T17:40:37Z

🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here.

allgandalf

codewise LGTM, testing now

rinej · 2025-08-05T13:54:02Z

@allgandalf Thanks for review and finding the issue. I added the fix for it. Please have a look now :)

fixed-bottom-scrolling.mov

mountiny · 2025-08-05T20:35:50Z

src/ROUTES.ts

+            if (parentReportID) {
+                queryParams.push(`parentReportID=${parentReportID}`);
+            }
+
+            if (parentReportActionID) {
+                queryParams.push(`parentReportActionID=${parentReportActionID}`);
+            }


@WojtekBoman @adamgrzybowski @pac-guerreiro @rinej I think we will need to find a way to handle this differently. Check out the routing philosophy https://github.com/Expensify/App/blob/main/contributingGuides/philosophies/ROUTING.md#--should-not-use-query-parameters

We should not use the query params, and I don't think this is an exception that should apply, as it's quite a common URL.

@rinej can you please summarise why these params are needed in the route? Let's find a different solution for this than the query params.

It won't be a beautiful solution, but according to this rule from the routing philosophy:

- SHOULD NOT use optional parameters If there are optional parameters, use two separate route definitions instead to be explicit.

we should have 4 different routes based on the available params.
Please remember to correctly extend the screen type

It's an example of the screen component type that is shared between two routes https://github.com/Expensify/App/blob/main/src/pages/iou/HoldReasonPage.tsx

type HoldReasonPageProps = | PlatformStackScreenProps<MoneyRequestNavigatorParamList, typeof SCREENS.MONEY_REQUEST.HOLD> | PlatformStackScreenProps<SearchReportParamList, typeof SCREENS.SEARCH.TRANSACTION_HOLD_REASON_RHP>;

As I analyse it the the parentReportID and parentReportActionID were added to enable immediate display of transaction details even while report actions are still loading, so we have the necessary parent context to load related data upfront via useOnyx (in ReportScreen.tsx).
Without these parameters, we have have to wait through a loading sequence (transaction thread → parent report → parent action → transaction details), so we see the loading screen.

I think it was the fundamental idea of @pac-guerreiro to resolve the initial issue.

@WojtekBoman even if we create separate routes for each available parameter, it still wouldn’t be ideal according to the rules, since we’re supposed to avoid including ANY new parameters there - is that correct?"

Which rule are you referring to - avoid including ANY new parameters there? I'd like to make sure we're on the same page :) I see this rule can be broken:

- SHOULD use only the minimal necessary information to render the page Examples: r/:threadReportID GOOD - threadReportID is all that is needed and all the rest of the page can be derived from that r/:parentReportID/:threadReportID BAD - the parentReportID is not necessary so it just adds cruft to the URL

On the other hand, if it makes loading faster, I think we can assume it's necessary cc: @mountiny

Sure, I will investigate and try to find another way for it

Maybe cache it in sessionStorage or something like that? We wouldn't need to take care of cleaning this information.

bump @rinej on comments ^

I tried another solution that didn’t require passing those parameters, but it ended up breaking the other report screens.

I’m now testing an approach that uses sessionStorage. The idea is to move the parameters from the navigation route into session storage.

Thanks, lets make sure it works as expected on all platforms

…-skeleton

rinej · 2025-08-11T17:19:02Z

@allgandalf @mountiny
I updated the code to store and retrieve the parameters from session storage instead of using navigation parameters.

I left the rest of the code untouched as possible to preserve existing features, such as automatically scrolling to the bottom by default.
Here’s how it looks on the web:

web-transaciton-v2.mov

mountiny · 2025-08-11T18:28:00Z

@rinej love it, thank you! Are we clearing the sessions storage upon signing out?

github-actions · 2025-08-11T18:28:48Z

🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here.

github-actions · 2025-08-11T18:49:58Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
Built from App PR #66165.

Android 🤖	iOS 🍎
❌ FAILED ❌	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/66165/index.html
The QR code can't be generated, because the Android build failed

Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/66165/NewExpensify.dmg	https://66165.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

allgandalf · 2025-08-12T04:53:52Z

Bug: Transaction table goes away after we navigate to a expesne:

Screen.Recording.2025-08-12.at.10.31.42.AM.mov

allgandalf · 2025-08-12T04:54:36Z

Follow up bug: The user is not navigated back to the workspace chat when clicking on header link afterwards:

Screen.Recording.2025-08-12.at.10.23.56.AM.mov

allgandalf · 2025-08-12T04:55:41Z

Follow up Bug: Each workspace chat i click after opening the expense has an expense at the top of it

Screen.Recording.2025-08-12.at.10.24.54.AM.mov

allgandalf · 2025-08-12T04:57:03Z

Bug: No unread message indicator

Screen.Recording.2025-08-12.at.10.26.30.AM.mov

allgandalf

please check the ^

…-skeleton # Conflicts: # src/pages/home/report/ReportActionsList.tsx

…-skeleton # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportView.tsx

pac-guerreiro · 2025-08-26T13:38:30Z

@rinej we can close this one 😄

rinej and others added 9 commits July 15, 2025 15:16

Reverted the reverted changes from main

0e864fc

revert the InvertedList in the ReportActionsList

cf4cd41

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

b07d7a1

…-skeleton

fix: missing money request actions in more menu

372f735

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

6d512ec

…-skeleton

Revert "revert the InvertedList in the ReportActionsList"

c361df7

This reverts commit cf4cd41.

Revert "revert using FlatList for transaction thread"

e13f0dc

This reverts commit d3d18de.

docs: add explanation for FlatList usage

2aac82d

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

7cc61fb

…-skeleton # Conflicts: # src/pages/home/ReportScreen.tsx

mountiny assigned rinej and pac-guerreiro Jul 17, 2025

allgandalf suggested changes Jul 18, 2025

View reviewed changes

pac-guerreiro and others added 8 commits July 21, 2025 12:13

fix: prevent image flickering

a1834cc

fix: prevent whole report actions list flickering

1104c7f

fix: prevent initial transaction thread rendering without optimistic …

d77d565

…created action

chore: resolve console warning

ed9eb5d

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

e3a8943

…-skeleton

fix: building optimistic created action for all reports

8769161

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

3c1c406

…-skeleton

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

abbb848

…-skeleton

rinej marked this pull request as ready for review July 22, 2025 10:57

rinej requested a review from a team as a code owner July 22, 2025 10:57

melvin-bot bot requested review from allgandalf and removed request for a team July 22, 2025 10:57

chore: apply suggestions

c9ba34b

This comment has been minimized.

Sign in to view

allgandalf reviewed Jul 23, 2025

View reviewed changes

fix: adjust bottom scroll calculation

9662e73

allgandalf approved these changes Aug 5, 2025

View reviewed changes

mountiny requested changes Aug 5, 2025

View reviewed changes

rinej mentioned this pull request Aug 7, 2025

Update the transaction thread report loading to show skeleton for report actions only if transaction data available #63253

Closed

16 tasks

mountiny mentioned this pull request Aug 11, 2025

[$1500] Show the full receipt when viewing an expense in the right hand pane in New Expensify #64025

Closed

16 tasks

rinej added 4 commits August 11, 2025 16:03

move parent params to the session storage instead of route params

67c80ea

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

7132a27

…-skeleton

remove double dependencies

8f37916

Lint fixes

84b3559

allgandalf suggested changes Aug 12, 2025

View reviewed changes

add auto session params clear

9368a43

mountiny mentioned this pull request Aug 13, 2025

Add the ability to cycle through reports #66608

Merged

51 tasks

rinej mentioned this pull request Aug 14, 2025

Open transaction thread on top #68528

Closed

51 tasks

pac-guerreiro added 2 commits August 20, 2025 21:00

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

fb415de

…-skeleton # Conflicts: # src/pages/home/report/ReportActionsList.tsx

Merge branch 'main' into part-2-update-the-transaction-report-to-show…

33c9ee6

…-skeleton # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportView.tsx

This was referenced Aug 21, 2025

[63253] (1/2) Open transaction thread on top #68932

Merged

[63253] (2/2) Render optimistic transaction thread #69073

Closed

mountiny mentioned this pull request Aug 25, 2025

Better RHP view #67533

Closed

51 tasks

allgandalf closed this Aug 28, 2025

pac-guerreiro mentioned this pull request Sep 11, 2025

[63253] (1/2) Open transaction thread on top #70250

Merged

49 tasks

		/** If this report is a transaction thread report */
		isReportTransactionThread?: boolean;

Update the transaction report to show skeleton part 2 #66165

Update the transaction report to show skeleton part 2 #66165

Uh oh!

Conversation

rinej commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

allgandalf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melvin-bot bot commented Jul 22, 2025

Uh oh!

pac-guerreiro commented Jul 22, 2025

Uh oh!

github-actions bot commented Jul 22, 2025

Uh oh!

This comment has been minimized.

allgandalf left a comment

Choose a reason for hiding this comment

Uh oh!

rinej commented Aug 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WojtekBoman Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rinej commented Aug 11, 2025

Uh oh!

mountiny commented Aug 11, 2025

Uh oh!

github-actions bot commented Aug 11, 2025

Uh oh!

github-actions bot commented Aug 11, 2025

Uh oh!

allgandalf commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allgandalf commented Aug 12, 2025

Uh oh!

allgandalf commented Aug 12, 2025

Uh oh!

allgandalf commented Aug 12, 2025

Uh oh!

allgandalf left a comment

Choose a reason for hiding this comment

rinej commented Jul 15, 2025 •

edited

Loading

WojtekBoman Aug 6, 2025 •

edited

Loading

allgandalf commented Aug 12, 2025 •

edited

Loading