Improve MoneyRequestReportView by Kicu · Pull Request #58903 · Expensify/App

Kicu · 2025-03-21T12:23:48Z

Work in progress

This adds more updates to MoneyRequestReportView.
Also updates existing header that is displayed for single-transaction view to add navigation to it (< >).

Things fixed:

date column size improved
add basic scrolling to bottom functionality
working navigation when clicking on transaction row
- I needed to modify the config of one of RHP navigators to disable animation, since we don't want it when using new < > buttons on TransactionReport opened in RHP
totals under the table added (by @jnowakow)
skeleton and loading indicators added (fixes [Due for payment 2025-04-10] [Better Expense Report View] iOS - Reports - Report view is blank and does not show skeleton when fetching data #58813)
spacing and sizes are aligned much closer with Search results list, after Shawn's initial review

list of required followups

automatic scrolling to bottom when user writes a new message

Explanation of Change

Fixed Issues

$ #57508
$ #58851
$ #58854
$ #58813
$ #58857

PROPOSAL:

Tests

go to Reports -> Expense Reports and click on any report
it should open in the new view with transactions list at the top
verify that all the chat messages that should be seen by user are displayed
verify that sorting still works
verify that clicking on transaction row works and opens RHP with transaction-report
verify that totals below transaction list displays correctly

Offline tests

QA Steps

Same as Tests

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

- fix date column size - add basic scrolling to bottom functionality

src/components/MoneyRequestReportView/MoneyRequestReportTableHeader.tsx

DylanDylann · 2025-03-24T10:02:30Z

@Kicu could you list out what is done here? Then I can review it first

* Add totals under table * Move summary row to TransactionList

Kicu · 2025-03-25T09:59:42Z

@DylanDylann I will list this and make it much better for reviewing in a few hours todaym, once I finalize a few more things 👍

Kicu · 2025-03-25T15:21:41Z

hey @DylanDylann sorry it took so long, this is quite a complicated view 😅

Here's what I'm expecting to work:

clicking on 1 row, which opens RHP and RHP has the navigation
totals under the table (done by @jnowakow)
visual bugs here: [Better Expense Reports] Add MoneyRequestReportView #58360 (comment)
almost everything here: [Due for payment 2025-04-16] [Due for payment 2025-04-10] [Better Expense Report View] Implement updated Expense Report View #57508 (comment)

What's expected to NOT work:

will not scroll to bottom in every case (I will be working on it in the upcoming days)
there is no selection checkboxes - @jnowakow is working on them
no "new messages" indicator

You can start reviewing these parts now. I will try to open it tomorrow for review and will ask you before pushing any more changes. Thanks! 🙏

Kicu · 2025-03-25T15:43:25Z

@Expensify/design feel free to drop any comments with the caveats described here: https://swmansion.slack.com/archives/C07NMDKEFMH/p1742917369637919?thread_ts=1742890386.286389&cid=C07NMDKEFMH

Kicu · 2025-03-25T15:45:34Z

quick demo:

rec-demo.mp4

Kicu · 2025-03-25T16:15:41Z

@DylanDylann @mountiny @luacmartins in here I described why I'm rewriting parts of logic related to scrolling etc for anyone that might think it's weird or unnecessary 😅

github-actions · 2025-03-25T16:36:46Z

🚧 @luacmartins has triggered a test app build. You can view the workflow run here.

shawnborton · 2025-03-25T18:02:19Z

Minor comment: maybe 16px of top padding feels a bit better for the report header?

shawnborton · 2025-03-25T18:03:20Z

If a report has no comments, we don't need the floating "Comments" label here:

shawnborton · 2025-03-25T18:04:17Z

Let's add 12px of right padding to the totals area, so it aligns with the total above:

shawnborton · 2025-03-25T18:05:17Z

Let's reduce the padding under the table to 16px so the totals sit slightly closer:

shawnborton · 2025-03-25T18:07:02Z

If there's only one expense on the report, we don't need the carousel buttons:

shawnborton · 2025-03-25T18:09:39Z

We want a gap of 8px between all of the sub-totals and totals, right now I think it's just 4px:

shawnborton · 2025-03-25T18:15:32Z

I don't think this is related to this PR but just noting that some reports aren't displaying merchant or amount in the table:

Kicu · 2025-03-31T10:02:09Z

@DylanDylann conflicts resolved, I needed to slightly change logic of showing empty state/skeletons.
Fixed 2 small problems that you found.

BUG: In the first time go to MoneyRequestReportView, the transactions doesn't display

I can't reproduce this :/ the only thing I can reproduce is that initially smaller amount of transactions load and then after a second all transactions load which looks bad - but I didn't have the case where no transactions were displayed.

There is small bug we found in related to this which @jnowakow will fix in his PR.
This state can become stale: https://github.com/Expensify/App/pull/58713/files#diff-fdae540f324f25f7a8b7f7c44ebf5eb8681a7cef16b937fd24ef2546618561baR56-R60

But this change will come in a separate PR

github-actions · 2025-03-31T12:00:25Z

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

mountiny

Big PR, I expect there will be some new things which pop up after QA testing in staging, but great to move this ahead

mountiny · 2025-03-31T12:14:23Z

src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

+            // Single transaction report will open in RHP, and we need to find every other report ID for every sibling to `activeTransaction`
+            // we use this data to display prev/next arrows in RHP for navigating between transactions
+            const sortedSiblingTransactionReportIDs = sortedData.transactions
+                .map((transaction) => {
+                    const action = getIOUActionForTransactionID(reportActions, transaction.transactionID);
+                    return action?.childReportID;
+                })
+                .filter((reportID): reportID is string => !!reportID);
+
+            setActiveTransactionReportIDs(sortedSiblingTransactionReportIDs);
+


Coul dbe nice to cover this logic with unit test

mountiny · 2025-03-31T12:17:07Z

src/components/MoneyRequestReportView/SearchMoneyRequestReportEmptyState.tsx

                headerStyles={[styles.emptyStateMoneyRequestReport]}
                lottieWebViewStyles={styles.emptyStateFolderWebStyles}
                headerContentStyles={styles.emptyStateFolderWebStyles}
+                minModalHeight={380}


Could we add this as a CONST?

we could, but I don't think it's that important. This component itself is a specific component dedicated to MoneyRequestReport. Also empty state inside it also uses a specific number as a default:

App/src/components/EmptyStateComponent/index.tsx

Line 32 in e2e322f

minModalHeight = 400,

If I'm to make this a const, I would still keep it inside this file. I don't think is a value that anyone else should read or use. Its a specific height for this specific component

mountiny · 2025-03-31T12:19:48Z

src/libs/API/parameters/OpenReportParams.ts

+     * By default api returns report actions newest-first, and then older ones on subsequent pagination calls.
+     * If this flag is set to true, api will return oldest first starting from the beginning of report.
+     */
+    useTableReportView?: boolean;


I think it now can be set as true always and basically indicates the client version can handle this new view. in BE we check if the user has the beta and only then we return the reversed order

src/libs/actions/Report.ts

mountiny · 2025-03-31T12:22:06Z

src/libs/actions/Report.ts


+    // temporary flag will be removed once ReportScreen supports MoneyRequestReportView - https://github.com/Expensify/App/issues/57509
+    // Permission will be removed before the end of the project
+    if (temporaryShouldUseTableReportView && Permissions.canUseTableReportView(allBetas)) {


But you do not have to check for the beta anymore, its handled in the BE

github-actions · 2025-03-31T12:41:34Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
Android 🤖🔄	iOS 🍎🔄
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/58903-hybrid/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/58903-hybrid/index.html

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

👀 View the workflow run that generated this build 👀

Kicu · 2025-03-31T13:48:33Z

@mountiny I can add some unit tests and fix the permissions check in ~30mins

mountiny · 2025-03-31T13:58:04Z

Great, thank you!

shawnborton · 2025-03-31T14:19:13Z

Excited to get this one merged, looking good on my end 👍

Kicu · 2025-03-31T14:54:52Z

@mountiny removed the permissions check and added some unit tests for picking reportIDs.
Please note that "mockData" is a work in progress, which we will be editing together with other guys as we build this feature.

It will be used for tests (and stories). So right now the file is a bit messy, but it will be updating it whenever me (or one of the other guys from SWM) will be adding any tests :)

luacmartins

Looking great! Left a couple of NAB. Also, this has been reported before, but just wanted to mention it again, the empty receipt background color is still incorrect (NAB for this PR though):

luacmartins · 2025-03-31T15:07:20Z

src/components/MoneyRequestReportView/TransactionReportIDRepository.ts

+ */
+
+function setActiveTransactionReportIDs(ids: string[]) {
+    // Cleaning previously saved keys because


NAB was there an end to this sentence?

Suggested change

// Cleaning previously saved keys because

// Cleaning previously saved keys because

I will fix in another PR 😅

luacmartins · 2025-03-31T15:08:22Z

src/components/TransactionItemRow/DataCells/TypeCell.tsx

        case undefined:
-            return 'Cash';
+            return 'iou.cash';


NAB do we need this specific case? Can't we just use the default?

will see if default handles it, but most likely we can make improve it

mountiny

Thanks!

src/libs/MoneyRequestReportUtils.ts

OSBotify · 2025-03-31T15:49:10Z

✋ 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-04-01T15:38:52Z

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

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

github-actions · 2025-04-03T11:40:14Z

🚀 Deployed to production by https://github.com/grgia in version: 9.1.22-10 🚀

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

jjcoffee · 2025-05-06T07:25:22Z

src/pages/Search/SearchMoneyRequestReportPage.tsx

+                    shouldDisplaySearchRouter
+                    shouldShowBackButton={shouldUseNarrowLayout}
+                    onBackButtonPress={Navigation.goBack}
+                    linkKey="notFound.noAccess"


Coming from #60014. This caused an issue where the This chat doesn't exist message was duplicated, since we're setting both subtitleKey and linkKey. We removed linkKey since it's unnecessary in this context.

s77rt · 2025-06-15T10:57:08Z

src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

+                </View>
+            )}
+            <View style={[styles.dFlex, styles.flexRow, listHorizontalPadding, styles.justifyContentBetween, styles.mb2]}>
+                <Text style={[styles.textLabelSupporting]}>{hasComments ? translate('common.comments') : ''}</Text>


The comments subheader should have been moved outside the !isEmpty(transactions) condition so that it's displayed for reports with no transactions too.

Coming from #63076

Pujan92 · 2025-11-08T12:50:23Z

src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx

+
+            // Single transaction report will open in RHP, and we need to find every other report ID for the rest of transactions
+            // to display prev/next arrows in RHP for navigating between transactions
+            const sortedSiblingTransactionReportIDs = getThreadReportIDsForTransactions(reportActions, sortedData.transactions);


We need to filter out pending delete transactions to avoid issue of navigating to those transactions.

Kicu mentioned this pull request Mar 21, 2025

[Better Expense Report View] Reports - LHN tab highlights Reports when expense report is opened #58810

Closed

8 tasks

Kicu force-pushed the kicu/57508-improve-money-report-view branch from 96e3b43 to 5f01212 Compare March 21, 2025 15:02

Kicu added 3 commits March 24, 2025 08:42

Improve MoneyRequestReportView

0a90b85

- fix date column size - add basic scrolling to bottom functionality

Make SearchTypeMenu look correct with undefined Search query

022d3a3

Improve SearchMoneyRequestReportPage loading and not found page

0512fc9

Kicu mentioned this pull request Mar 24, 2025

[Due for payment 2025-04-10] [Better Expense Report View] Expense Reports - Table header remains after all the expenses are deleted #58854

Closed

8 tasks

Kicu force-pushed the kicu/57508-improve-money-report-view branch from 58136d4 to 0512fc9 Compare March 24, 2025 09:37

DylanDylann reviewed Mar 24, 2025

View reviewed changes

src/components/MoneyRequestReportView/MoneyRequestReportTableHeader.tsx Outdated Show resolved Hide resolved

Kicu and others added 3 commits March 24, 2025 13:52

Add navigating to transaction reportID

228e7ab

Add totals under table (#213)

1378cef

* Add totals under table * Move summary row to TransactionList

Add navigation when clicking on Transaction row

20e5c22

Improve styling and types in MoneyRequestReportView

22606bc

Kicu force-pushed the kicu/57508-improve-money-report-view branch from b384ab4 to 22606bc Compare March 25, 2025 15:11

Kicu added 2 commits March 25, 2025 16:12

Merge branch 'main' into kicu/57508-improve-money-report-view

e2c3b27

Merge branch 'main' into kicu/57508-improve-money-report-view

52710c4

This comment has been minimized.

Sign in to view

DylanDylann approved these changes Mar 31, 2025

View reviewed changes

mountiny previously approved these changes Mar 31, 2025

View reviewed changes

Kicu dismissed mountiny’s stale review via 4eea9f6 March 31, 2025 14:51

Add tests for MoneyRequestReportTransactionList

8d628fe

Kicu force-pushed the kicu/57508-improve-money-report-view branch from 4eea9f6 to 8d628fe Compare March 31, 2025 14:53

Fix lint

739e58d

Kicu force-pushed the kicu/57508-improve-money-report-view branch from bb9f6a0 to 739e58d Compare March 31, 2025 15:00

luacmartins approved these changes Mar 31, 2025

View reviewed changes

mountiny approved these changes Mar 31, 2025

View reviewed changes

src/libs/MoneyRequestReportUtils.ts Show resolved Hide resolved

mountiny merged commit 3d60ce6 into Expensify:main Mar 31, 2025
21 checks passed

linhvovan29546 mentioned this pull request Apr 1, 2025

[HOLD for payment 2025-04-11] [$50] Chat - New message button in conversation page is disappeared when returning from thread page. #59427

Closed

8 tasks

github-actions bot mentioned this pull request Apr 1, 2025

Deploy Checklist: New Expensify 2025-04-01 #59448

Closed

60 tasks

trjExpensify mentioned this pull request Apr 1, 2025

[Due for payment 2025-05-14] [Better Expense Report View] Report view follow-ups #59459

Closed

16 tasks

Guccio163 mentioned this pull request Apr 1, 2025

[Due for payment 2025-04-14] [$250] [Due for payment 2025-04-10] Reports - Total and action section fields are messed up #59474

Closed

8 tasks

Kicu mentioned this pull request Apr 2, 2025

Add selection to table on MoneyRequestReport page #59433

Merged

50 tasks

jponikarchuk mentioned this pull request Apr 16, 2025

[Due for payment 2025-05-30] [$250] Reports - Report view is not preserved after going to other page and returning to Reports #58957

Closed

8 tasks

jjcoffee reviewed May 6, 2025

View reviewed changes

s77rt reviewed Jun 15, 2025

View reviewed changes

s77rt mentioned this pull request Jun 15, 2025

[Due for payment 2025-06-19] [$250] Expense report - 'Comments' sub-header is not displayed in the chat. #63076

Closed

8 tasks

Pujan92 reviewed Nov 8, 2025

View reviewed changes

	// Cleaning previously saved keys because
	// Cleaning previously saved keys because

Conversation

Kicu commented Mar 21, 2025 • edited by trjExpensify Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things fixed:

list of required followups

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

Uh oh!

DylanDylann commented Mar 24, 2025

Uh oh!

Kicu commented Mar 25, 2025

Uh oh!

Kicu commented Mar 25, 2025

Uh oh!

Kicu commented Mar 25, 2025

Uh oh!

Kicu commented Mar 25, 2025

Uh oh!

Kicu commented Mar 25, 2025

Uh oh!

github-actions bot commented Mar 25, 2025

Uh oh!

This comment has been minimized.

shawnborton commented Mar 25, 2025

Uh oh!

shawnborton commented Mar 25, 2025

Uh oh!

shawnborton commented Mar 25, 2025

Uh oh!

shawnborton commented Mar 25, 2025

Uh oh!

shawnborton commented Mar 25, 2025

Uh oh!

shawnborton commented Mar 25, 2025

Uh oh!

shawnborton commented Mar 25, 2025

Uh oh!

Kicu commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 31, 2025

Uh oh!

mountiny 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 31, 2025

Uh oh!

Kicu commented Mar 31, 2025

Uh oh!

mountiny commented Mar 31, 2025

Uh oh!

shawnborton commented Mar 31, 2025

Uh oh!

Kicu commented Mar 31, 2025

Uh oh!

luacmartins 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

Kicu commented Mar 21, 2025 •

edited by trjExpensify

Loading

Kicu commented Mar 31, 2025 •

edited

Loading

s77rt Jun 15, 2025 •

edited

Loading