Fix scrolling to bottom list on MoneyRequestReportView

[Kicu]

Make scrolling to bottom and unread message marker work on MoneyRequestReportView.

Explanation of Change

• for unread marker I extracted the common logic and reused it in MoneyRequestReport; since the list is not inverted there I needed to traverse the message in reverse order
• scrolling is more complicated. In the inverse list we're always scrolling to offset 0 which works; however scrollToEnd will sometimes not work if its called before all the new items rendered, because the list will not know their size. Hence I had to use setTimeout
• Extra note: the solution is tested, but my emulator + simulator were running quite slow so on the videos app looks much more slow than in reality

Fixed Issues

$ #58808
PROPOSAL:

Tests

• go to Reports > Expense Reports > click on report
• verify that chat scrolls to bottom to newest message when a user types a new message in
• verify that unread message marker behaves the same as on chat reports (from Inbox)

Offline tests

QA Steps

• go to Reports > Expense Reports > click on report
• verify that chat scrolls to bottom to newest message when a user types a new message in
• verify that unread message marker behaves the same as on chat reports (from Inbox)

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
  • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (see Reviewing the code)
  • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
  • I verified that comments were added to code that is not self explanatory
  • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
  • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
  • I verified the JSDocs style guidelines (in STYLE.md) were followed
• If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
• I followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
• I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
• I verified that if a function's arguments changed that all usages have also been updated correctly
• If any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• If a new CSS style is added I verified that:
  • A similar style doesn't already exist
  • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
• If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
• If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
• If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
• If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
• If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
  • I verified that all the inputs inside a form are aligned with each other.
  • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

rec-andr.mp4

Android: mWeb Chrome

iOS: Native

rec-ios.mp4

iOS: mWeb Safari

MacOS: Chrome / Safari

rec-web.mp4

MacOS: Desktop

[Kicu]

noticed this was unused

[Kicu]

this function was extracted from ReportActionsList almost without change - I'm just passing down arguments so that it can be reused in 2 places.
No logic changes are intended for ReportActionsList

[melvin-bot]

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

[Kicu]

Cleaned and ready for review

[shawnborton]

Will run a test build 🚀

[github-actions]

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

[shawnborton]

Hmm this doesn't feel like a viable solution to me... the LHN shows a loading bar after I send a comment, and there is a delay in scrolling me down the page:

CleanShot.2025-04-09.at.10.53.24.mp4

cc @mountiny for thoughts, but this doesn't feel like smooth UX to me.

[Kicu]

The loading bar we can probably disable - if that is what you think makes more sense - this more of an oversight on my part.

The delay in scrolling to bottom however will be more problematic.
The main problem is that this is a dynamic list with items that have dynamic size and scrolling to bottom simply doesn't work out of the box and needs some hacks :|
I can definitely work more on making this feel smooth, but its not something that can be done super quick.

In "normal" chats we use the reverse list, and what to you is bottom is really the top of the list and so scrolling there works much better.

[shawnborton]

Cool, yeah I think in the very least removing the LHN loader would help it feel a little smoother

[DylanDylann]

New message indicator should only appear if the message come from another user. If the new action from the current user, we shouldn't display the new message indicator

Screen.Recording.2025-04-09.at.18.08.03.mov

[Kicu]

I will come back to this tomorrow to update and fix things

[Kicu]

@DylanDylann I manged to fix the unreadIndicator, I believe it now behaves same as on normal chats

rec-scrolling-web.mp4

However the request from Shawn to disable loading bar is not yet done, because this Loading bar is kinda global and unrelated to search, and I need more time ot make this work.
Will come back to this tomorrow.

[Kicu]

@DylanDylann ready for re-review.

@shawnborton at this point the Loading bar is disable on Report in Search/Reports context, however I did not modify any behaviour related to scrolling to bottom.

[shawnborton]

Will spin up a new testie for us 🚀

[github-actions]

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

[Kicu]

@DylanDylann I fixed the minor mistake and moved 1 extra line to inside the function, left the rest unchanged.

Will try to check if I can reproduce the weird error

[Kicu]

@DylanDylann confirming that I can repro this mweb error on android emulator: #59664 (comment)

Seems to be coming from here:

App/src/libs/Pusher/index.ts

Line 203 in dbd0421

throw new Error(`[Pusher] instance not found. Pusher.subscribe()

I also cannot reproduce it on main :/ not sure what happens here and why

[DylanDylann]

Opsss.... You mean it comes from our change.

[Kicu]

Ok @DylanDylann
I finally managed to reproduce this also on main, and spend some time digging around this.
Feels to me like it does not directly come from my change, but my code somehow surfaces an existing error. Perhaps we are using some kind of old way of using Browser notifications? I will try to create an issue for this later on github.

For now I think we don't have to block this PR.
Here is my repro:

rec-error.mp4

[DylanDylann]

@mountiny What do you think? Are we good to merge and fix above error later in another PR?

[jnowakow]

There is usePrevious hook

[Kicu]

thanks, I agree with you, but at this point I'd prefer to keep code the same both inMoneyRequestReportActionsList and standard ReportActionsList.

I will add this comment to another issue where I will be de-duplicating common code and fix it there.

[mountiny]

Small suggestion

[mountiny]

This is already large and complex change so I would avoid adding more complexity by the optional es lint fixes

[melvin-bot]

@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

[mountiny]

Noted above, due to the existing complexity of the pr we have skipped the optional eslint changes to the touched file

[OSBotify]

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

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

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

[kavimuru]

Failing with original bug ID #58808 on web mweb and android
https://platform.applause.com/services/links/v1/external/786062acb0485551bc419cb3dadd68ffe1512b0b022a35b247aeecc941cb061b
https://platform.applause.com/services/links/v1/external/dfe98b5756ea8ded7da15f7373604d22413062c677f62cfc8335c077243f5c70
https://platform.applause.com/services/links/v1/external/4c15ff6aa8e0b3101369882195b774b156335e115fcc9d326f22a5dd8be817c8

[mountiny]

cc @Kicu

[github-actions]

🚀 Deployed to production by https://github.com/marcaaron in version: 9.1.29-10 🚀

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

[Kicu]

Looking

[Kicu]

sorry I in fact did not look - will do so tomorrow, since I got more work on my other PR than expected.