Fix: force FlashList to use natural DOM order on web#85825
Fix: force FlashList to use natural DOM order on web#85825sharabai wants to merge 3 commits intoExpensify:mainfrom
Conversation
|
|
sharabai
changed the title
|
This patch starts with 003 because another one of my PRs takes 002. |
FlashList isn't used in Mobile-Expensify, we're good here. |
melvin-bot
bot
requested review from
eVoloshchak
and removed request for
a team
|
@eVoloshchak 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] |
rushatgabhane
requested review from
shubham1206agra
and removed request for
eVoloshchak
![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: 2ca24ea86e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| + if (Platform.OS === "web") { | ||
| + renderEntries.sort(([, a], [, b]) => a.index - b.index); |
There was a problem hiding this comment.
This sort always uses ascending index, but FlashList still supports inverted on web (RecyclerView/ViewHolder reverse the visual order via the inverted transform helpers). In that mode the DOM will now be deterministically ordered opposite to what users see, so keyboard and screen-reader traversal stays backwards for any inverted FlashList even though this patch is meant to fix accessibility ordering. The web sort needs to account for inverted (and horizontal inverted) instead of always using a.index - b.index.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We don't have yet any components that are using inverted prop with FlashList. I did test it on FlashList 2.3.0 without the patch and let's just say inverted does a lot of things at once and navigation there looks weird in general. For example, list starts from the bottom and tabbing goes up, not down (without the patch). The patch does not change that behavior. Maybe it's warranted to come back to it when we have some components that do use that prop and reevaluate, but the behavior looks good to me at this point.
There was a problem hiding this comment.
@VickyStash Can you please look into this when you use this prop?
There was a problem hiding this comment.
Here is the PR #85114 where I'm working on migration main chat to FlashList with the inverted flag (P.S. I have three patches there as well). So what should I test specifically?
There was a problem hiding this comment.
FYI I've also tested this PR over the scenario when main chat (so expense details too) are migrated to FlashList, and I see some wrong ordering when I use voice over:
Monosnap.screencast.2026-03-20.09-32-05.mp4
But since FlashList is not used for the main chat in the current prod, I guess it can be solved later.
Just one question from me, does this PR then fixes the original issue for production? Since at this moment FlatList is used for main chat/expense details on prod?
Screen.Recording.2026-03-19.at.11.35.42.PM.movBUG: Multiple items in LHN gets highlighted when scrolling fast (Not repro on staging) |
|
🚧 @luacmartins has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
3 participants
Explanation of Change
FlashList uses absolute positioning, so the DOM order of elements doesn’t affect layout from its perspective. However, this creates an accessibility issue: mobile screen readers navigate based on visual position, while web screen readers follow the DOM order.
This patch ensures that list items are rendered in a natural DOM order, aligning behavior across platforms.
The performance impact is minimal. Sorting only occurs when the pool of reusable DOM elements changes. Even in a heavy scenario, say, 30 visible items plus a buffer above and below (around 50 elements total) the cost of sorting is negligible.
Fixed Issues
$ #76923
PROPOSAL:
Tests
You can test in on any list in the app that uses FlashList and has lots of elements.
Example test for
src/pages/iou/request/MoneyRequestParticipantsSelector.tsxOffline tests
QA Steps
Same as tests
I'm open to suggestions what type of QA testing is best here. In my opinion a general regression testing to check for bugs on pages that use FlashList.
A good place to check different places in the app that use FlashList can be found here
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.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
VoiceOver-web.mp4