Update the transaction thread report loading to show skeleton by rinej · Pull Request #65744 · Expensify/App

rinej · 2025-07-09T13:41:52Z

Explanation of Change

The PR was copied, because I need the write permission to modify it
Original PR -> #64211

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

Transactions-Web.mp4

Offline tests

QA Steps

Same as tests.

PR Author Checklist

Screenshots/Videos

Android: Native

Transaction-android.mp4

Android: mWeb Chrome

Transactions-mWebAndroid.mp4

iOS: Native

Transactions-iOS.mp4

iOS: mWeb Safari

Transaction-mWebiOS.mp4

MacOS: Chrome / Safari

Transactions-Web.mp4

MacOS: Desktop

Transaction-Desktop.mov

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx # src/components/MoneyRequestReportView/MoneyRequestReportTransactionsNavigation.tsx # src/pages/home/ReportScreen.tsx # src/pages/home/report/ReportActionsView.tsx

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available # Conflicts: # src/pages/home/ReportScreen.tsx # src/pages/home/report/ReportActionsList.tsx # src/pages/home/report/ReportActionsView.tsx

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

melvin-bot · 2025-07-09T13:52:27Z

@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]

github-actions · 2025-07-09T14:11:55Z

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

github-actions · 2025-07-10T14:06:50Z

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

mountiny · 2025-07-10T14:07:50Z

src/pages/home/ReportScreen.tsx

    const [reportMetadata = defaultReportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportIDFromRoute}`, {canBeMissing: true, allowStaleData: true});
-    const [policies = getEmptyObject<NonNullable<OnyxCollection<OnyxTypes.Policy>>>()] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {allowStaleData: true, canBeMissing: false});
-    const [parentReportAction] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${getNonEmptyStringOnyxID(reportOnyx?.parentReportID)}`, {
+    const [policies = getEmptyObject<NonNullable<OnyxCollection<OnyxTypes.Policy>>>()] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {allowStaleData: true, initialValue: {}, canBeMissing: false});


I think @VickyStash is in process of removing the initialValue

Yeah, it looks like something went wrong during the conflicts resolution. No need in initialValue: {}

yes thats right, I will remove it

mountiny · 2025-07-10T14:09:37Z

src/pages/home/report/ReportActionsView.tsx

-            ),
-        [reportActions, isOffline, canPerformWriteAction, reportTransactionIDs],
+            isReportTransactionThread
+                ? reportActions


I think we still want the same logic as this for report transaction thread though:

(isOffline || isDeletedParentAction(reportAction) || reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || reportAction.errors) && shouldReportActionBeVisible(reportAction, reportAction.reportActionID, canPerformWriteAction) &&

Why is it removed?

I wasn't in the initial implementation, so had to double check, but I tested and I agree that we do need that filter there. I will revert it. (I think it was initially added for some render optimization, but didn't notice any impact)

mountiny · 2025-07-10T14:20:35Z

src/components/MoneyRequestHeader.tsx

+    const handleDeleteModalConfirm = useCallback(() => {
+        setIsDeleteModalVisible(false);
+        if (!parentReportAction || !transaction) {
+            throw new Error('Data missing');


How will this look like in prod?

mountiny · 2025-07-10T14:22:17Z

src/pages/home/report/ReportActionsList.tsx

    // Parse Fullstory attributes on initial render
    useLayoutEffect(parseFSAttributes, []);

+    const ListComponent = isTransactionThread(parentReportAction) ? FlatList : InvertedFlatList;


I do not understand why we bringing this change here and it was not needed before? Feels quite significant change

It was added to fix the content shifting issue in when switching between transaction reports

Without that change:

Shifting.mp4

With that change:

NoShifting.mp4

I agree that it is the significant change (even though it is only for TransactionThread).

I will revisit it, maybe I will be able to find some better solution to fix it

When I compared it with main, the behaviour is similar as on first video, so that shifting is already on main (so the changes on this branch didn't caused it)

Maybe we don't need it or it is not in the scope of this ticket?

@mountiny Let me know your thoughts, I will revert that logic then.

@rinej I am not sure about this change right now, could we revert it please?

Sure, understand we don't wont to add to much changes in one PR, I reverted it ✅
If by any change we would like that logic, we can create separate issue for it

mountiny · 2025-07-10T14:27:32Z

Screen.Recording.2025-07-10.at.16.24.39.mp4

Also noting the receipt flickering, but NAB here

github-actions · 2025-07-10T14:33:29Z

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

Android 🤖	iOS 🍎
⏩ SKIPPED ⏩	❌ FAILED ❌
The build for Android was skipped	The QR code can't be generated, because the iOS build failed

Desktop 💻	Web 🕸️
⏩ SKIPPED ⏩	https://65744.pr-testing.expensify.com
The build for Desktop was skipped

👀 View the workflow run that generated this build 👀

mountiny · 2025-07-11T12:37:38Z

@allgandalf can you please retest again? thanks

…-to-show-skeleton

allgandalf · 2025-07-12T18:59:03Z

I will but jest is failing can we fix that please

…-to-show-skeleton

rinej · 2025-07-14T08:47:51Z

I'm on it

rinej · 2025-07-14T10:18:16Z

I found some flaky test in ReportUtilsTest.ts, so it is not related to the changes in the PR.

In the ReportUtilsTest.ts

            const report: Report = {
                ...createRandomReport(1),
                type: CONST.REPORT.TYPE.CHAT,
                participants: {
                    ...buildParticipantsFromAccountIDs([currentUserAccountID, 1234]),
                    [currentUserAccountID]: {
                        notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
                    },
                },
            };

            await Onyx.set(ONYXKEYS.SESSION, {email: currentUserEmail, accountID: currentUserAccountID});

            expect(canLeaveChat(report, undefined)).toBe(false);
        });

We are creating random report and test the canLeaveChat function.

It works well for all of report types, exept when
chatType = policyExpenseChat

When we have that type the logic inside the canLeaveChat returns true and cause the test to fails.

    if (isPolicyExpenseChat(report) && !report?.isOwnPolicyExpenseChat && !isPolicyAdminPolicyUtils(policy)) {
        return true;
    }

The tests where added 5 days ago. The fix is quite easy, just make sure that when we create the random report for tests, do not include policyExpenseChat as the chatType.

@mountiny @allgandalf
Should I fix this test in this PR? Or do in another issue, as it is not related?

mountiny · 2025-07-14T10:44:28Z

@rinej all good its handled here #66023

mountiny · 2025-07-14T11:05:20Z

src/pages/home/ReportScreen.tsx

+            parentReportIDFromRoute && parentReportActionIDFromRoute
+                ? ({
+                      ...reportOnyx,
+                      reportID: reportIDFromRoute,
+                      parentReportID: parentReportIDFromRoute,
+                      parentReportActionID: parentReportActionIDFromRoute,
+                      policyID: parentReport?.policyID,
+                      type: CONST.REPORT.TYPE.CHAT,


Can you please leave more comments around this to explain why we do it?

src/components/MoneyRequestHeader.tsx

mountiny · 2025-07-14T11:08:17Z

src/pages/home/report/ReportActionsList.tsx

    // Parse Fullstory attributes on initial render
    useLayoutEffect(parseFSAttributes, []);

+    const ListComponent = isTransactionThread(parentReportAction) ? FlatList : InvertedFlatList;


@rinej I am not sure about this change right now, could we revert it please?

pac-guerreiro · 2025-07-14T12:02:09Z

@mountiny, regarding this comment, I made this change so that the list would remain focused at the top. The InvertedList component is made in a way that focus the list on the bottom.

mountiny

Thank you, @pac-guerreiro @rinej can you please proactively check for new deploy blockers that might be related to this as with the memozation I would imagine there might be some issues popping up

OSBotify · 2025-07-14T15:25:55Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2025-07-15T03:22:35Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.81-0 🚀

platform	result
🖥 desktop 🖥	success ✅
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

mountiny · 2025-07-15T07:39:59Z

I had to revert this because of https://expensify.slack.com/archives/C05LX9D6E07/p1752552404895209, as the flatlist changes been reverted, we did not retest the transaction thread and system messages been showing above the transaction details

github-actions · 2025-07-16T19:54:17Z

🚀 Deployed to production by https://github.com/luacmartins in version: 9.1.81-7 🚀

platform	result
🖥 desktop 🖥	success ✅
🕸 web 🕸	success ✅
🤖 android 🤖	success ✅
🍎 iOS 🍎	success ✅

pac-guerreiro and others added 20 commits June 15, 2025 17:01

chore: add report skeleton when transaction data is available

e0aa222

fix: missing transaction thread report name

e24c819

fix: app crash when navigating between transaction threads

000f95c

chore: add missing type declarations

633e0ec

fix: transaction thread navigation issues

9f49b63

chore: resolve linter issues

0d38e80

Merge branch 'main' into pac-guerreiro/feature/63253-update-the-trans…

0456cdb

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

fix(63253): content shifting when opening a transaction thread view

045f688

reduce re-renders amount

18d35db

prettier

5440605

Merge branch 'main' into pac-guerreiro/feature/63253-update-the-trans…

5a2c2b8

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

Merge branch 'main' into pac-guerreiro/feature/63253-update-the-trans…

911dd9a

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

fix lint

928d46e

adjust backTo default logic

aaec8a5

fix: add missing allReportAction to dependency array

89f7cde

Merge branch 'main' into pac-guerreiro/feature/63253-update-the-trans…

d7a9328

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

Merge branch 'main' into pac-guerreiro/feature/63253-update-the-trans…

9bdbb37

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

Merge branch 'main' into pac-guerreiro/feature/63253-update-the-trans…

ed122ad

…action-thread-report-loading-to-show-skeleton-for-report-actions-only-if-transaction-data-available

rinej marked this pull request as ready for review July 9, 2025 13:52

rinej requested a review from a team as a code owner July 9, 2025 13:52

melvin-bot bot requested review from allgandalf and removed request for a team July 9, 2025 13:52

mountiny assigned rinej Jul 9, 2025

mountiny self-requested a review July 9, 2025 14:10

mountiny mentioned this pull request Jul 9, 2025

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

Closed

49 tasks

This comment has been minimized.

Sign in to view

mountiny reviewed Jul 10, 2025

View reviewed changes

rinej added 3 commits July 11, 2025 12:42

chore: add more graceful error handling

0762b0a

fix: apply necessary filter for transaction report

3a0bf79

chore: remove redundant default value

c32835c

Merge branch 'main' into Update-the-transaction-thread-report-loading…

1b1ba66

…-to-show-skeleton

Merge branch 'main' into Update-the-transaction-thread-report-loading…

cce7b40

…-to-show-skeleton

mountiny reviewed Jul 14, 2025

View reviewed changes

rinej and others added 2 commits July 14, 2025 14:12

revert using FlatList for transaction thread

d3d18de

docs: explain implementation changes

6b86334

mountiny approved these changes Jul 14, 2025

View reviewed changes

mountiny merged commit 6637833 into Expensify:main Jul 14, 2025
22 of 23 checks passed

github-actions bot mentioned this pull request Jul 15, 2025

Deploy Checklist: New Expensify 2025-07-15 #66099

Closed

95 tasks

nabi-ebrahimi mentioned this pull request Jul 15, 2025

Split - Splitted expense can be constantly splitted again. #66120

Closed

8 tasks

mountiny mentioned this pull request Jul 15, 2025

[CP Staging] Revert "Update the transaction thread report loading to show skeleton" #66126

Merged

Beamanator mentioned this pull request Jul 15, 2025

Expense - Hold and Delete option are missing in More menu in transaction thread #66109

Closed

8 tasks

This was referenced Jul 15, 2025

[Due for payment 2025-07-25] Expense - Missing "Waiting for you to add expense(s)" message in empty report #66123

Closed

Report - Extra empty space displayed above receipt thumbnail on expense report. #66110

Closed

Conversation

rinej commented Jul 9, 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!

melvin-bot bot commented Jul 9, 2025

Uh oh!

github-actions bot commented Jul 9, 2025

Uh oh!

This comment has been minimized.

github-actions bot commented Jul 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rinej Jul 10, 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!

mountiny commented Jul 10, 2025

Uh oh!

github-actions bot commented Jul 10, 2025

Uh oh!

mountiny commented Jul 11, 2025

Uh oh!

allgandalf commented Jul 12, 2025

Uh oh!

rinej commented Jul 14, 2025

Uh oh!

rinej commented Jul 14, 2025

Uh oh!

mountiny commented Jul 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pac-guerreiro commented Jul 14, 2025

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

OSBotify commented Jul 14, 2025

Uh oh!

github-actions bot commented Jul 15, 2025

Uh oh!

mountiny commented Jul 15, 2025

Uh oh!

github-actions bot commented Jul 16, 2025

Uh oh!

Reviewers

Assignees

Labels

rinej commented Jul 9, 2025 •

edited

Loading

rinej Jul 10, 2025 •

edited

Loading