[cp staging] fix(iOS): keep ScrollAnchor mounted to prevent scroll reset when MVCP toggles#88923
Conversation
|
|
|
Before: ios_before.mov |
|
After: ios_after.mov |
|
@ShridharGoel 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 159c90273d
ℹ️ 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 (maintainVisibleContentPosition != null) { | ||
| return { | ||
| - ...maintainVisibleContentPosition, | ||
| minIndexForVisible: 0, |
There was a problem hiding this comment.
Preserve MVCP options when building internal config
The new maintainVisibleContentPositionInternal object drops ...maintainVisibleContentPosition, so all caller-provided fields except minIndexForVisible are discarded. This means options like disabled and autoscrollToTopThreshold are no longer forwarded to the internal MVCP prop, which changes behavior for existing call sites that intentionally pass maintainVisibleContentPosition={{disabled: ...}} (for example in selection/search lists) and can cause unexpected scroll pinning/jumps when list data updates. The anchor-lifetime fix can be kept while still preserving non-overridden MVCP fields.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-04-28.at.6.12.52.PM.moviOS: mWeb SafariMacOS: Chrome / Safari |
…or-mounted fix(iOS): keep ScrollAnchor mounted to prevent scroll reset when MVCP toggles (cherry picked from commit c79b2d0) (cherry-picked to staging by arosiclair)
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by https://github.com/arosiclair in version: 9.3.64-9 🚀
Bundle Size Analysis (Sentry): |
|
🚧 @arosiclair has triggered a test Expensify/App build. You can view the workflow run here. |
|
🚀 Cherry-picked to staging by https://github.com/arosiclair in version: 9.3.64-10 🚀
Bundle Size Analysis (Sentry): |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Explanation of Change
Adds FlashList patch 007 that keeps
ScrollAnchoralways mounted when themaintainVisibleContentPosition(MVCP) prop is defined, regardless of thedisabledflag on that prop.Root cause: When
maintainVisibleContentPosition.disabledtoggled fromtruetofalse(e.g. when a user scrolls up in a permalinked report), theScrollAnchorcomponent unmounted. On the native Fabric side, this caused_firstVisibleView's weak reference to becomenil. On remount,_prevFirstVisibleFrameheld a stale origin of1,000,000 + X, so the computeddeltaY = -(1,000,000 + X)produced a massive upward scroll jump, resetting the list to the top.Fix: Keep
ScrollAnchoralways mounted when the MVCP prop is present. JS-levelscrollBycorrections remain gated byshouldMaintainVisibleContentPosition(), so the anchor stays visually inert when MVCP is logically off — only the native view lifecycle changes.Fixed Issues
$ #88238
$ #88955
$ #89052
PROPOSAL:
Tests
Scenario 1
Prequisities: have a chat between user A and user B
a. The idea is there is scrolling possible and money request is in between
Scenario 2
Scenario 3
Preconditions:
a. Set Submissions to Instantly
b. Disable Approvals and Payments
Steps:
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
Nagranie.z.ekranu.2026-04-27.o.14.11.05.mov
Android: mWeb Chrome
iOS: Native
ios_after.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Nagranie.z.ekranu.2026-04-27.o.14.12.28.mov