[Better Expense Report View] Add sorting functionality to MoneyRequestReportView#58713
Conversation
src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
Outdated
Show resolved
Hide resolved
src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
Outdated
Show resolved
Hide resolved
|
@madmax330 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] |
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
Outdated
Show resolved
Hide resolved
|
Let's keep the order of transactions if all values of sortBy field are empty Screen.Recording.2025-03-20.at.15.07.29.mov |
Just to be sure, do we want to keep the order only if all values are empty or also if each value of the selected column is equal? |
Nice catch. It looks better |
src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
Outdated
Show resolved
Hide resolved
|
BUG: Can't sort the date field, because we always use Screen.Recording.2025-03-21.at.00.09.05.mov |
src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx
Outdated
Show resolved
Hide resolved
|
Fixed the mentioned bug, and after talking to Dylan we agreed that usePrevious can't really be used here, so I made it cleaner in another way. |
|
@DylanDylann @allgandalf whats your ETA? |
|
@DylanDylann took over this PR ! they asked to work on in later on slack, but happy to take over in case he's not around |
|
I will complete this today |
Reviewer Checklist
Screenshots/VideosAndroid: NativeSorting only function on large screen Android: mWeb ChromeSorting only function on large screen iOS: NativeSorting only function on large screen iOS: mWeb SafariSorting only function on large screen MacOS: Chrome / SafariScreen.Recording.2025-03-21.at.23.48.53.movMacOS: DesktopScreen.Recording.2025-03-21.at.23.55.31.mov |
mountiny
left a comment
There was a problem hiding this comment.
Thanks! I agree with the last point from @DylanDylann it might be related not exactly to this pr but to our changes in this area, lets address in a follow up.
|
Congrats, that's your 5th PR merged! 🎉 Do you know about the ContributorPlus role? It's an opportunity to earn more in the Expensify Open Source community. Keep up the great work - thanks! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.1.17-1 🚀
|
| const dateKey = transaction.modifiedCreated ? 'modifiedCreated' : 'created'; | ||
| return key === CONST.SEARCH.TABLE_COLUMNS.DATE ? dateKey : key; |
There was a problem hiding this comment.
Coming from #69302 checklist: we're missing the case where the TOTAL_AMOUNT column is also based on the modifiedCreated field

Explanation of Change
This PR extends 58360 by adding sorting logic for each sortable column.
Fixed Issues
$ #57508
PROPOSAL: N/A
Tests
Important: in order to test change function canUseTableReportView to return true, or have BETA tableReportView set
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Web.mov
MacOS: Desktop