[Due for payment 2025-05-28] [Better Expense Report View] [Followup] Deduplicate logic between ReportActionsList and MoneyRequestReportActionsList

[Kicu]

Followup up to this original issue: #57505

tl;dr while both reportActions lists are quite similar it was not easy to simply reuse the component. Instead I decided to create MoneyRequestReportActionsList as a fresh component a copy parts of logic from standard ReportScreen to it. Because of that a lot of report actions-centric logic was duplicated.

List of things to do: TBD

CC @mountiny creating this issue to be able to link it later in the PR. I will update this description later.

Issue Owner

Current Issue Owner: @Kicu

[mountiny]

Handled as a follow up by @Kicu

[Kicu]

About ReportScreen and new MoneyRequestReport

I made some decisions when working on Better Expense Report View that I thought require some deeper documenting, than what was written in the design doc.

Below I'm describing parts of the decision making process behind implementation of the new MoneyRequestReport components.

Why did we create separate MoneyRequestReport View?

• ReportScreen is the screen rendered at route: /r/<reportID>. It is quite complex and handles every possible report type: money reports (IOU and Expense), chats, and the very special "single transaction thread".
• The new MoneyRequestReportView is a component that will be rendered both at routes /r/<reportID> and /search/r/<reportID>. However it should only handle "money request" reports, which means: IOU, Expense and Invoice.

Because code of ReportScreen is not easy to read or modify I decided that adding even more conditions and bool flags to it will make the code too hard to follow. That is why new components were created. Every single reportAction is rendered in the same way for both views, using pre-exisiting: https://github.com/Expensify/App/blob/main/src/pages/home/report/ReportActionsListItemRenderer.tsx
However the *Page -> *View -> *List components are written separately for new view.

Why did we use non inverted FlatList in MoneyRequestReportActionsList

InvertedFLatList Is currently used in ReportScreen and works well for chats-style reports, because it autoscrolls to bottom. However in new MoneyRequest Views we want the user to start at the top.

tl;dr because chats naturally start at the newest/bottom, but our new view starts at the oldest/top I choose to use non-inverted FlatList and we need to rewrite and duplicated parts of code between ReportScreen and MoneyRequestReportView

Inverted FLatList pros/cons

• 🔴 requires extra work to have initial scroll be correct (because initial scroll is at the bottom)
  • lazy loading would work badly because when scrolling to top it might trigger extra fetching
• 🔴 requires extra work to scroll to top
• 🔴 when there is a very small amount of messages (~1-3) and they do not take full screen vertically, they are aligned to bottom - this requires tricky styling to make look good
• 🟢 BUT all the utils for grouping avatars and finding last unread message automatically work

Normal FLatList pros/cons

• 🔴 requires extra work to scroll to bottom - because bottom of the list requires knowledge about offset
• 🔴 need to update/rewrite utils for grouping avatars and finding last unread
• 🟢 initial scroll is correct - at the top of screen
• 🟢 lazy loading should work correctly
• 🟢 looks good when there is a very small amount of messages (~1-3) - they are aligned to top
• 🟢 scrollToTop is easy because top of list is on top of screen (list offset 0)

[mountiny]

Thanks for typing that out, your decision makes sense to me

[Kicu]

Thanks.

After the main parts of this view are done, we will need to spend some time to actually deduplicate this logic, and in the end we might still have parts of very similar logic partially duplicated between ReportScreen and new ReportView.
That is why I wanted to have a clear record of why it was done that way.

[melvin-bot]

@Kicu Huh... This is 4 days overdue. Who can take care of this?