Repository navigation
[HOLD on #106626] [LegendList 12/15] feat: migrate report actions to chronological LegendList - #100733
[HOLD on #106626] [LegendList 12/15] feat: migrate report actions to chronological LegendList#100733chrispader wants to merge 148 commits into
Conversation
1bac411 to
c067b0c
Compare
c067b0c to
e784ebe
Compare
chrispader
changed the title
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. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f616302cdb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
chrispader
changed the title
f616302 to
2a3323a
Compare
|
|
chrispader
changed the title
On Android, a chat opened at a linked message appeared with the message at the top of the list and then moved down to its offset. LegendList applied the native anchor adjustments for rows measured during its initial scroll a second time, and its retries reused a stale target. Patch LegendList to skip anchor moves a later native scroll already includes, move the pending target with each adjustment, and finish its silent-scroll retry before the scroll completes (LegendApp/legend-list#604). The list also requested older actions from scroll events that arrive before the initial scroll lands, which inserted a page above the linked message while it was being positioned. Older actions now load from scroll events only once the list is ready. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@dmkt9 @roryabraham i've addressed some more issues with the initial report opening after the cache has been cleared. Other than that, i don't see any other issues so far. @dmkt9 let me know about your final review 🙌🏼 |
…nd-list-report-actions Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chrispader Great! I'll finish my review soon |
|
Chris's AI deputy reporting for duty. 🤖 @dmkt9 @roryabraham After clearing the cache, opening a link to an older message could briefly show newer messages out of order: a later expense preview appeared directly below the linked message until the real messages replaced it. The cause is in the pagination middleware, not in this list, so I fixed it in a separate PR, #106359. For read requests like GetNewerActions, it now saves the page after the messages it lists. That PR has before and after recordings made with this branch. #106359 is now layer 9 of the rollout and merges before this PR, which is now layer 10. |
chrispader
changed the title
Fixed ✅
Detailsbug1.mp4
Detailshttps://github.com/user-attachments/assets/0c461e30-fcc8-450a-847b-29a837e2115d
Details**Steps to reproduce:**
bug4.mp4
Detailsbug1.mp4
Detailsbug2.mp4
Detailsbug3.mp4
Detailsbug4.mp4
Detailsbug71.mp4
Detailsbug8.mp4The keyboard appears briefly: Detailsbug5.mp4The keyboard doesn't appear when editing a long comment: Detailsbug6.mp4On Android, the highlighted item is out of the viewport if there is a long comment above it: Detailsbug7.mp4Unable to test due to a new bug: The app automatically navigates back to the Home screen when "Reply in thread". Detailsbug2.mp4Android: The highlighted item scrolls down and then back up: bug1.mp4Android: Scrolling slightly right after the loading state disappears causes the list to jump: bug3.mp4Web: The "Submit" badge needs to be clicked multiple times (sometimes 3–4 times) to scroll to the highlighted item. Sometimes, the top of the highlighted item or the bottom of the last item in the list is obscured. bug4.mp4Bug: Automatically scroll to the highlighted item when opening the PDF viewer: Detailsbug1.mp4Bug: In a DM chat, the initial viewport isn't scrolled to the bottom when there's a new message, especially when the chat contains an IOU. Detailsbug2.mp4Web: The chat in a thread isn't inverted. Detailsbug3.mp4Bug 1: The loading skeleton disappears when opening a report in offline mode. Detailsbug1.mp4Bug 4: The list anchor isn't preserved when loading older messages, causing the current viewport to jump. Detailsbug4.mp4Bug 3: Scrolling to the bottom using the Latest Message badge causes the loading skeleton to appear, and the viewport ends up above the bottom of the chat. Detailsbug3.mp4Bug 5: The list automatically scrolls to the bottom when opening a report marked as unread. Detailsbug5.mp4NAB: Can we make sure the unread indicator is fully visible when using the "New message" badge? Detailsnab1.mp4Bug 7: The "New message" badge flashes briefly. Detailsbug7.mp4Bug 6: The composer is hidden behind the keyboard when there's a draft message. The same issue occurs if we focus the composer while the report is loading. Additionally, if we send a message while the report is loading, the list doesn't scroll to the bottom once the loading state ends. Detailsbug6.mp4Remaining
|
|
Thanks @dmkt9, looking into those! Hopefully these are the last ones 🤞🏼 |
…ender When a chat opens at a linked message that is the newest loaded one, the newer messages load afterwards. LegendList hides each new row until it is measured, and rendering a page of them can take seconds on a slow device. The footer skeleton moved below the hidden rows as soon as they entered the data, so the area below the link stayed blank until they appeared. The linked row now draws its own absolutely positioned skeleton over that area. It hides itself in the frame LegendList measures the first newer row, so the rows replace it in place without a blank frame or a jump. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep cached parent actions linked offline and focus restored drafts only after the message list is ready. Preserve the loaded list and its native header anchor on reconnect. Let offline Latest messages jumps complete using the newest cached page. Co-authored-by: Codex <noreply@openai.com>
chrispader
changed the title
Merge main after the shared LegendList upgrade and scroll patches landed. Preserve the migration-specific positioning and offline reconnect fix. Co-authored-by: Codex <noreply@openai.com>
Give compound scrolling, loading, and composer focus conditions semantic names while keeping their evaluation in the existing execution paths. Co-authored-by: Codex <noreply@openai.com>
Each LegendList patch is now the build output of its upstream PR, so the patches change exactly what the PRs change, in every bundle the PRs touch. - 001 and 003 now also patch the native bundles, as their PRs change shared code. - 002 adds the explicit maintainVisibleContentPosition anchor option that LegendApp/legend-list#597 now proposes, instead of anchoring the last visible item in every list with alignItemsAtEnd. - 004 contains LegendApp/legend-list#600, which now also skips deferred adjustments that a later native scroll already includes and moves the initial scroll target with each adjustment. - 006 keeps only the silent-retry fix from LegendApp/legend-list#604. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LegendList patch 002 now follows LegendApp/legend-list#597, which makes anchoring the last visible item an explicit maintainVisibleContentPosition option instead of deriving it from alignItemsAtEnd. The chat list passes anchor "end" whenever it aligns its messages to the end, so rows that resize or load above the reader's position keep moving only the content above it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…otes LegendList patch 005 places its scroll anchor at 1,000,000 points instead of 10,000,000. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The patch now is the build output of LegendApp/legend-list#607. That PR applies LegendList's header-size adjustment on every platform and reuses the Android clamp handling that item resizes already have, instead of the header-only copy of it. The patch also gets its details.md entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
chrispader
changed the title
|
WIP
WIP
Yes, thanks. Fixed!
Fixed in #106687 |
chrispader
changed the title
Keep displayed offline actions ready while queued app and report loads finish. This prevents loading skeletons and the first successful response from recreating the list and resetting its viewport. Co-authored-by: Codex <noreply@openai.com>
Merge the independent cached-chat reconnect fix and the latest merged chat pagination prerequisites while retaining the LegendList migration. Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
chrispader
changed the title
Backport react/react-native#58187 so Android redraws the ScrollView after maintainVisibleContentPosition corrects its offset. This prevents a stale frame when messages above a linked report action finish measuring. Co-authored-by: Codex <noreply@openai.com>
chrispader
changed the title
chrispader
changed the title
The report-actions list currently uses inversion, so rendering order and scroll coordinates differ from chronological message order. This PR moves the main chat list to LegendList and updates pagination, unread tracking, message editing, recycling, and initial loading behavior to work with chronological data.
@roryabraham @dmkt9
@Expensify/design Please review the initial chat-loading skeleton shown below.
Explanation of Change
This is layer 12 of the fifteen-part LegendList rollout. It moves the main report-actions list to chronological LegendList data and updates pagination, unread tracking, linked-message positioning, end following, and initial loading.
The migration builds on the LegendList foundation merged in layer 10, #106095 and the keyboard fix merged in #106003.
The shared list intentionally selects platform-specific accessibility attributes and the native action-sheet scroller at its LegendList boundary so they use the same list ref and positioning lifecycle.
The pagination ordering fix in #106359 is now merged. This migration builds on layer 11, #106626 for the upstream LegendList patches and #106687 for cached-chat scroll preservation on reconnect. The reconnect fix applies to the existing FlashList implementation independently of this migration.
On an initial online load, the skeleton covers cached messages until OpenReport finishes and the list is positioned. Cached chats stay mounted on reconnect, and restored drafts wait for the loading cover to disappear before taking focus. The warm open-report span ends when the positioned messages replace that cover.
Android's native scroll anchoring could briefly show the updated message positions at the previous visible offset when earlier messages finished measuring. This PR backports the redraw fix from react/react-native#58187 so the corrected offset is visible in the same frame.
Fixed Issues
$ #98994
This PR is part of the migration split from #98270. No approved proposal is linked.
Tests
All platforms means desktop web, mobile web, Android, and iOS.
Use a second test account in the same chat for the incoming-message cases.
Web, first open and return to a chat
Android and iOS apps, first open and return to a chat
Android app, returning from an older thread
All platforms, message history and incoming messages
All platforms, a link to an older message
All platforms, reopening a chat with a recent unread message
All platforms, edit the newest and an older message
All platforms, formatted messages and attachments after scrolling
*bold* _italic_ ~strike~ https://example.com. The formatting should render correctly.Web, opening a chat URL after signing in
Android and iOS apps, keyboard and message-box resizing
Offline tests
All platforms, cached chats offline and history after reconnecting
QA Steps
Run the All platforms cases and the blocks for the platform being tested, including 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
Returning from an older thread
Before
android-thread-return-before.mp4
After
android-thread-return-after-direct.mp4
Cold chat open and initial loading
FlashList is on the left and LegendList is on the right. FlashList leaves the report body blank during loading; LegendList shows the report-action skeleton before messages appear.
ColdOpenLoading.mp4
Cold expense-chat open
FlashList is on the left and LegendList is on the right. Both progress from the loading skeleton to an expense preview and its surrounding report actions.
Android screen recording TBD.
Warm expense-chat open
FlashList is on the left and LegendList is on the right. Both open the cached expense preview and report actions from the inbox.
Android screen recording TBD.
Warm chat open
Side-by-side opens of the same cached chat, ending on the same report-action history. The implementations are not labeled in this recording.
Android screen recording TBD.
Android: mWeb Chrome
iOS: Native
Cold chat open and initial loading
FlashList is on the left and LegendList is on the right. FlashList leaves the report body blank during loading; LegendList shows the report-action skeleton before messages appear.
LegendListLoadingSkeletonVsFlashList.mp4
Cold expense-chat open
FlashList is on the left and LegendList is on the right. Both progress from the loading skeleton to an expense preview and its surrounding report actions.
ExpenseChatCold.mp4
Warm expense-chat open
FlashList is on the left and LegendList is on the right. Both open the cached expense preview and report actions from the inbox.
ExpenseChatWarm.mp4
Warm chat open
Side-by-side opens of the same cached chat, ending on the same report-action history. The implementations are not labeled in this recording.
LegendListWarmReportOpen.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
🤖 Generated with Codex