fix: fix maintainVisibleContentPosition on iOS when _firstVisibleView is removed#52782
fix: fix maintainVisibleContentPosition on iOS when _firstVisibleView is removed#52782itsramiel wants to merge 1 commit into
Conversation
cipolleschi
left a comment
There was a problem hiding this comment.
Thanks for fixing this!
|
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this in D78804475. |
|
@cipolleschi should I be changing anything regarding the failed internal linter? |
|
@itsramiel no, don't worry... I just have to run the internal linter, which is different from the one in OSS, unfortunately, and we couldn't find a way to unify the two because internally we use a modified version of Clang... 😓 Anyway, just to set expectation, I have been asked to test the change on a few internal apps, so it will take me a little of time before landing this as I hve to manually run those tests.. 😅 |
|
Hi @cipolleschi. Sorry to annoy you, but any chance this moves on? |
|
Would love to see this merged 💯 |
|
this fix seems to be in, only checked 0.83.2 |
Summary:
Addresses #52757
On Fabric, when the
maintainVisibleContentPositionis passed withminIndexForVisibleand that index is deleted, the_firstVisibleViewwill point to the wrong view because of Fabric's view recycling.The behaviour observed in #52757 is that when
minIndexForVisibleis 0 and that item is removed, the list jumps to the end and then scrolls back all the way to the top because ofautoscrollToTopThresholdsince item 0 is deleted and added to the recycled view pool and the newly to render item(all the way at the bottom) dequeues that same view resulting in_firstVisibleViewwhich was pointing to the item of index 0 now points at item of index ~203.This was not happening in old arch(Paper) because old arch doesnt recycle views.
My fix is to validate whether
_firstVisibleViewis the same view we stored in_prepareForMaintainVisibleScrollPositionand to do that I thought of storing the view's tag as the id. If the tag changed then we do not need to adjust the scrolling depending on that view since that view pointer is pointing to a different view and I reset the identifiers related to it.Changelog:
[IOS] [FIXED] - Fix maintainVisibleContentPosition on iOS when item with index equal to minIndexForVisible is removed
Test Plan:
I am open to the maintainers' suggestion on what is the best way to test this.
But here is a recording from the repro of #52757 with a patch including the changes in this pr implemented in this branch.
Screen Recordings
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-07-22.at.12.27.19.mp4
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-07-23.at.13.00.01.mp4