Add selection to table on MoneyRequestReport page by jnowakow · Pull Request #59433 · Expensify/App

jnowakow · 2025-04-01T10:32:15Z

Explanation of Change

Follow up to new MoneyRequestReport page. It adds selection to transactions table. The implementation lack support for long press selection on mobile devices, it will be added in next follow up in order to keep PRs small.

Fixed Issues

$ #57508
$ #59459
PROPOSAL: N/A

Tests

Selection doesn't work on small screen. It will be fixed in separate PR.

Go to new report view -> /search/r/report-id
Verify that transactions in table can be selected
Verify that when any transaction is selected there is x selected button with dropdown in header

Verify that no errors appear in the JS console

Offline tests

Go to new report view -> /search/r/report-id
Verify that transactions in table can be selected
Verify that when any transaction is selected there is x selected button with dropdown in header

QA Steps

Selection doesn't work on small screen. It will be fixed in separate PR.

Go to new report view -> /search/r/report-id
Verify that transactions in table can be selected
Verify that when any transaction is selected there is x selected button with dropdown in header

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

…08-add-checkboxes-to-table

…dd-checkboxes-to-table

melvin-bot · 2025-04-01T10:32:21Z

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

…dd-checkboxes-to-table

github-actions · 2025-04-01T15:03:31Z

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

allgandalf · 2025-04-01T15:43:47Z

@jnowakow please add test steps

allgandalf

Code is almost ready, please update the comments and we should be good with testing this PR

src/pages/Search/SearchMoneyRequestReportHoldReasonPage.tsx

allgandalf · 2025-04-01T16:50:31Z

src/pages/Search/SearchMoneyRequestReportHoldReasonPage.tsx

+    useEffect(() => {
+        clearErrors(ONYXKEYS.FORMS.MONEY_REQUEST_HOLD_FORM);
+        clearErrorFields(ONYXKEYS.FORMS.MONEY_REQUEST_HOLD_FORM);
+    }, []);


won't there be any other dependency to clear this error?

We want to clear them only when leaving page that page. It's handled that way on HoldReasonPage as well.

src/pages/Search/SearchMoneyRequestReportHoldReasonPage.tsx

src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx

src/components/MoneyReportHeader.tsx

shawnborton · 2025-04-01T19:05:56Z

I think the gap between checkbox and receipt icon is too large:

This is what the reports page looks like for reference:

Is there any way to harmonize styles between these components? My fear is that we need to maintain two separate row components that basically need to look exactly the same.

shawnborton · 2025-04-01T19:06:52Z

The gaps between rows is now too large:

From staging, I see this which is correct:

shawnborton · 2025-04-01T19:07:36Z

The table header is too close to the top of the view as well:

When is the last time we merged in main? Maybe these changes are available and we just need to merge them in?

mountiny

Tests well for me and changes look good to me, but I would still wait for the design feedback from @shawnborton to be addressed unless he is happy to move on. This PR does hold some other changes so unless its some biggest design issues, I would recommend merging 🙌

src/pages/Search/SearchMoneyRequestReportHoldReasonPage.tsx

shawnborton · 2025-04-07T16:32:37Z

Looking good outside of my comment above about the header being slightly off, and only other comment is that if something is selected, it should not have any :hover styles - it should just keep the selectedBG color on hover.

CleanShot.2025-04-07.at.18.32.09.mp4

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

jnowakow · 2025-04-07T16:38:12Z

This feature is actively worked on so I would said it's good to merge this PR and fix those two UI issues in one of coming PRs

mountiny

Those seem like minor checks, since we are super close to another deploy, I would merge it now and add those 2 remarks to a follow up tracker

shawnborton · 2025-04-07T17:31:47Z

Fine by me!

mountiny · 2025-04-07T17:33:56Z

@shawnborton thank you! Added them here in case you could just add a bit more detail so its easier for folks to squash quickly 🚀 #59452 (comment) thank you!

OSBotify · 2025-04-07T17:42:42Z

✋ 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-07T23:09:13Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.24-2 🚀

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

github-actions · 2025-04-09T02:14:29Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.24-10 🚀

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

github-actions · 2025-04-09T04:32:50Z

🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.24-10 🚀

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

jnowakow · 2025-04-16T11:26:42Z

@shawnborton I've wanted to create follow up and add selection to mobile but I can't find designs. It should look like this:

but with checkboxes on items? Or like on the reports page it should go into selection mode and Report header should be replaced with something like this:

shawnborton · 2025-04-16T15:01:27Z

Or like on the reports page it should go into selection mode and Report header should be replaced with something like this:

I think it makes sense to follow what we have for the reports page, where the whole header is replaced when we enter into select mode. cc @Expensify/design for the gut check there though!

shawnborton · 2025-04-16T15:01:54Z

And yes, we need to add the checkboxes to the mobile expense rows. Again, use the Reports page as your guiding light!

dannymcclain · 2025-04-16T15:11:57Z

Agree with Shawn above!

allroundexperts · 2025-05-14T22:21:54Z

src/components/MoneyReportHeader.tsx

+        const anyTransactionOnHold = selectedTransactions.some(isOnHoldTransactionUtils);
+        const allTransactionOnHold = selectedTransactions.every(isOnHoldTransactionUtils);
+
+        if (!anyTransactionOnHold && selectedTransactions.length === 1) {


Why was not the hold option shown when bulk selecting? Adding this line caused #60111

I can't find conversation on slack because it was discussed in long thread but it was agreed that we don't want to iterate over selected issues and send many requests. The go to solution was to temporarily disable that option if many transactions are selected and add it when there's api to hold many transaction in one request.

We have a BulkHoldRequest API in place, so we can remove this limitation and start working on a fix for that.

@luacmartins great! Is there issue for it? I can replace it

I don't think we have an issue yet, but I think @Tony-MK might be working on that

hoangzinh · 2025-06-10T22:35:07Z

src/pages/Search/SearchMoneyRequestReportHoldReasonPage.tsx

+            return;
+        }
+
+        putOnHold(firstTransactionID, values.comment, reportID);


putOnHold expects the report id of the transaction thread id (not the expense report) so we should pass the transaction thread report id here. More details here

DylanDylann · 2025-06-22T04:39:21Z

src/components/TransactionItemRow/DataCells/ChatBubbleCell.tsx

+        selector: (reportActions) => getIOUActionForTransactionID(Object.values(reportActions ?? {}), transaction.transactionID),
+    });
+
+    const [transactionReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${iouReportAction?.childReportID}`);


It caused this bug #62579

we didn’t distinguish between the regular view and the single transaction view. The single transaction view displays not only messages from the thread but also from the entire report, which is why the timestamp of the last read report action was missing.

jnowakow added 10 commits March 26, 2025 15:26

Add checkboxes to table

df1e9f1

Merge branch 'kicu/57508-improve-money-report-view' into jnowakow/575…

1e4e8a8

…08-add-checkboxes-to-table

Merge branch 'kicu/57508-improve-money-report-view' into jnowakow/575…

aa7d938

…08-add-checkboxes-to-table

Add actions on selected transaction to the header

a4f1e84

Add hold page

cd3eae9

Refresh actions on hold/unhold

024d3bd

Small refactor

2b910eb

Merge branch 'main' of github.com:Expensify/App into jnowakow/57508-a…

4996cf8

…dd-checkboxes-to-table

Rerender transactions list on transaction change

35056a6

Link follow-up issue and add new call

bb135a7

jnowakow requested review from a team as code owners April 1, 2025 10:32

melvin-bot bot requested review from DylanDylann and removed request for a team April 1, 2025 10:32

jnowakow marked this pull request as draft April 1, 2025 10:34

mountiny self-requested a review April 1, 2025 10:38

mountiny assigned jnowakow Apr 1, 2025

jnowakow added 3 commits April 1, 2025 13:17

Merge branch 'main' of github.com:Expensify/App into jnowakow/57508-a…

97c2185

…dd-checkboxes-to-table

Fix linter issues

11a424a

Fix linter and typecheck

887a779

jnowakow marked this pull request as ready for review April 1, 2025 14:46

This comment has been minimized.

Sign in to view

allgandalf suggested changes Apr 1, 2025

View reviewed changes

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

luacmartins previously approved these changes Apr 7, 2025

View reviewed changes

mountiny previously approved these changes Apr 7, 2025

View reviewed changes

src/pages/Search/SearchMoneyRequestReportHoldReasonPage.tsx Outdated Show resolved Hide resolved

Update src/pages/Search/SearchMoneyRequestReportHoldReasonPage.tsx

8e629ce

Co-authored-by: Vit Horacek <36083550+mountiny@users.noreply.github.com>

jnowakow dismissed stale reviews from mountiny and luacmartins via 8e629ce April 7, 2025 16:34

mountiny approved these changes Apr 7, 2025

View reviewed changes

mountiny mentioned this pull request Apr 7, 2025

[Better Expense Report View] Preview follow-ups #59452

Closed

17 tasks

mountiny merged commit e0d1735 into Expensify:main Apr 7, 2025
19 checks passed

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

Deploy Checklist: New Expensify 2025-04-07 #59756

Closed

allgandalf mentioned this pull request Apr 8, 2025

[Due for payment 2025-04-28] [Better Expense Report View] Checkbox state is not reset after switching tabs #59833

Closed

18 tasks

melvin-bot bot mentioned this pull request Apr 9, 2025

[Due for payment 2025-04-16] [Due for payment 2025-04-10] [Better Expense Report View] Implement updated Expense Report View #57508

Closed

jnowakow mentioned this pull request Apr 17, 2025

Add long press selection to money request report list #60430

Merged

50 tasks

Kicu mentioned this pull request Apr 30, 2025

Add Transaction and MoneyRequestReport UI tweaks #61159

Merged

49 tasks

allroundexperts reviewed May 14, 2025

View reviewed changes

Tony-MK mentioned this pull request May 15, 2025

Fixing displaying the hold and unhold option in expense report #60436

Merged

49 tasks

hoangzinh reviewed Jun 10, 2025

View reviewed changes

DylanDylann reviewed Jun 22, 2025

View reviewed changes

DylanDylann mentioned this pull request Jun 22, 2025

[Due for payment 2025-06-23] [Better Expense Report View] [$250] Chat bubble is always green in the expense preview when hold messages are read #62579

Closed

8 tasks

Conversation

jnowakow commented Apr 1, 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 Apr 1, 2025

Uh oh!

github-actions bot commented Apr 1, 2025

Uh oh!

allgandalf commented Apr 1, 2025

Uh oh!

This comment has been minimized.

allgandalf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shawnborton commented Apr 1, 2025

Uh oh!

shawnborton commented Apr 1, 2025

Uh oh!

shawnborton commented Apr 1, 2025

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shawnborton commented Apr 7, 2025

Uh oh!

jnowakow commented Apr 7, 2025

Uh oh!

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

shawnborton commented Apr 7, 2025

Uh oh!

mountiny commented Apr 7, 2025

Uh oh!

Uh oh!

OSBotify commented Apr 7, 2025

Uh oh!

github-actions bot commented Apr 7, 2025

Uh oh!

github-actions bot commented Apr 9, 2025

Uh oh!

github-actions bot commented Apr 9, 2025

Uh oh!

jnowakow commented Apr 16, 2025

Uh oh!

shawnborton commented Apr 16, 2025

Uh oh!

shawnborton commented Apr 16, 2025

Uh oh!

dannymcclain commented Apr 16, 2025

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!

jnowakow commented Apr 1, 2025 •

edited

Loading