Repository navigation
[LegendList 2/7] fix: use list-aware message edit scrolling - #100883
roryabraham merged 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
a796f48 to
e9e6f3b
Compare
dylanexpensify
left a comment
There was a problem hiding this comment.
Performance related vs pure product update. Input not needed
|
@roryabraham @dmkt9 this is ready for review! |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.mp4Android: mWeb ChromeiOS: HybridAppios.mp4iOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.4.80-0 🚀
|
|
🤖 No help site changes are required for this PR. I did not create a docs PR. WhyThis PR only changes internal scroll mechanics after a message edit is saved. It adds no user-facing copy, no new setting, no new button, and no change to any documented step. The relevant help article is Chat Features and Moderation Tools. Its Edit or Delete Messages section documents this flow:
Every one of those steps is still accurate after this PR. The article documents how to edit a message, not where the list scrolls afterwards — and scroll position is the only thing this PR touches. What the diff actually changesThe change replaces a hardcoded "index 0 means newest" assumption with an explicit, list-declared ordering:
The remaining three files are tests. Net user-visible effect: editing the newest comment in an expense report now keeps that comment visible, which is what the help site already implies should happen. It restores expected behavior rather than introducing new behavior worth documenting. Since no docs changes were needed, there is no help site PR to link or mark |
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.80-0 🚀
Bundle Size Analysis (Sentry): |
In expense views, message-edit scrolling currently assumes the main report action list is inverted.
MoneyRequestReportActionsListorders items chronologically, so editing its newest message can ask the wrong list position to stay visible. This WIP extraction makes message-edit scrolling list-aware while preserving existing report behavior.@roryabraham @dmkt9
Explanation of Change
The existing report action position context now carries both the action index and whether the action is newest, so each list declares its own ordering semantics.
MoneyRequestReportActionsListalso provides a separate list-level callback that scrolls to its newest position, while the inverted main report list keeps the existing report scroll manager fallback.This is layer 2 of the seven-part LegendList series. Review order is #100732 → this PR → #100884 → #100733 → #100740 → #100734 → #100736. The branches are maintained with GH Stacks in the Margelo fork; because GitHub does not support linking cross-fork heads into an upstream Stack, each upstream PR targets
Expensify/App:mainand records the dependency order here.Fixed Issues
$ #98994
PROPOSAL:
No approved proposal is linked.
Tests
Offline tests
QA Steps
Same as Tests and Offline tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
Screen_Recording_20260914_113442_Expensify.Debug.legendlist.mp4
iOS: Native
Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-09-14.at.10.48.28.mov
MacOS: Chrome / Safari
Screen.Recording.2026-09-14.at.11.21.12.mov