fix: make sure initialScrollIndex is bigger than 0 #36844
Conversation
facebook-github-bot
added
the
CLA Signed
okwasniewski
force-pushed
the
fix/@okwasniewski/initalScrollToIndex
branch
from
f05d994 to
c6562b9
Compare
okwasniewski
force-pushed
the
fix/@okwasniewski/initalScrollToIndex
branch
2 times, most recently
from
1492afc to
07d4d0e
Compare
okwasniewski
force-pushed
the
fix/@okwasniewski/initalScrollToIndex
branch
from
07d4d0e to
41c9940
Compare
Base commit: a27270f |
| } | ||
| } | ||
| /> | ||
| onFocusCapture={[Function]} |
There was a problem hiding this comment.
Is this change expected? It looks this is the snapshot from
which lists initialNumToRender as 4
There was a problem hiding this comment.
It looks like the snapshot is taken after we run fake layout and timers (after initial render), so I think this is showing that we are now rendering cells we previously failed to.
|
It would be a good idea to re-test this against current VirtualizedList, if this issue was happening on tvOS using 0.69. There have been some large changes since then. The change makes sense at a glance, and the snapshot change I think is showing the described bug fixed. But it would be helpful to also include the code this is triggering which causes the issue (it's hard to tell just from this condition). |
|
@NickGerleman Thanks for reviewing the changes. It looks like the new list (from master) on tvOS is working as expected and the bug with rendering only first batch of items is fixed. I can't reproduce this now, maybe some other scenario will. Let me know if you want to merge this change or I should close this PR. |
|
Snapshots make it look like this is still fixing a bug on main, so change makes sense to me. I am currently on vacation and away from internal tools, but someone else should be able to merge. |
|
@lunaleaps has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@NickGerleman merged this pull request in eb30a80. |
Summary: Hey, `adjustCellsAroundViewport` function was checking if `props.initialScrollIndex` is truthy and -1 was returning true. This caused bugs with rendering for tvOS: react-native-tvos/react-native-tvos#485 There are warnings in the code about `initalScrollIndex` being smaller than 0 but this if statement would still allow that. ## Changelog: [General] [Fixed] - Make sure initialScrollToIndex is bigger than 0 when adjusting cells Pull Request resolved: facebook#36844 Test Plan: Pass -1 as initialScrollToIndex. Check that this code is executed. Reviewed By: cipolleschi Differential Revision: D44856266 Pulled By: NickGerleman fbshipit-source-id: 781a1c0efeae93f00766eede4a42559dcd066d7d
Summary: Hey, `adjustCellsAroundViewport` function was checking if `props.initialScrollIndex` is truthy and -1 was returning true. This caused bugs with rendering for tvOS: react-native-tvos/react-native-tvos#485 There are warnings in the code about `initalScrollIndex` being smaller than 0 but this if statement would still allow that. ## Changelog: [General] [Fixed] - Make sure initialScrollToIndex is bigger than 0 when adjusting cells Pull Request resolved: facebook#36844 Test Plan: Pass -1 as initialScrollToIndex. Check that this code is executed. Reviewed By: cipolleschi Differential Revision: D44856266 Pulled By: NickGerleman fbshipit-source-id: 781a1c0efeae93f00766eede4a42559dcd066d7d
Summary: Hey, `adjustCellsAroundViewport` function was checking if `props.initialScrollIndex` is truthy and -1 was returning true. This caused bugs with rendering for tvOS: react-native-tvos/react-native-tvos#485 There are warnings in the code about `initalScrollIndex` being smaller than 0 but this if statement would still allow that. ## Changelog: [General] [Fixed] - Make sure initialScrollToIndex is bigger than 0 when adjusting cells Pull Request resolved: facebook#36844 Test Plan: Pass -1 as initialScrollToIndex. Check that this code is executed. Reviewed By: cipolleschi Differential Revision: D44856266 Pulled By: NickGerleman fbshipit-source-id: 781a1c0efeae93f00766eede4a42559dcd066d7d
Summary:
Hey,
adjustCellsAroundViewportfunction was checking ifprops.initialScrollIndexis truthy and -1 was returning true. This caused bugs with rendering for tvOS: react-native-tvos/react-native-tvos#485 There are warnings in the code aboutinitalScrollIndexbeing smaller than 0 but this if statement would still allow that.Changelog:
[General] [Fixed] - Make sure initialScrollToIndex is bigger than 0 when adjusting cells
Test Plan:
Pass -1 as initialScrollToIndex. Check that this code is executed.