Enable Generic Discontiguous Regions within VirtualizedList #32638
Conversation
Builds upon the `CellRenderMask` data structure added with facebook#31420, and VirtualizedList coverage added with facebook#31401. VirtualizedList currently keeps a [first, last] range as state, tracking the region of cells to render. The render functions uses this as an input, along with a few special cases to render more (sticky headers, initial render region.) This change moves to instead keep state which describes discontiguous render regions. This mask is continually updated as the viewport changes, batch renders expand the region, etc. Special cases are baked into the render mask, with a relatively simple tranformation from the mask to render function. This representation makes it much easier to support keyboarding scenarios, which require keeping distinct regions (e.g. for last focused) realized while out of viewport. MS/FB folks have a video discussion about VirtualizedList here: https://msit.microsoftstream.com/video/fe01a1ff-0400-94b1-d4f1-f1eb924b1809 facebook#31401 added quite a few snapshot tests, centering around the logic this change is touching. I manually validated RNTester FlatList examples (and their should be some upstream UI testing for them).
facebook-github-bot
added
CLA Signed
| } | ||
| } | ||
| /> | ||
| <View |
There was a problem hiding this comment.
There are some cases like this, where we render less, but the new behavior looks more correct to me given inputs.
react-native-bot
added
the
Type: Enhancement
Base commit: 51fe190 |
|
FYI to reviewers, there is quite a bit of code where chunks of logic have moved. I've tried to keep most of the existing blocks where possible, but you might need to hop around a bit to see where some of the chunks came from. |
Base commit: 51fe190 |
This change also includes the contents of facebook#32638 This change makes VirtualizedList track the last focused cell, through the capture phase of `onFocus`. It will keep the last focus cell, and its neighbors rendered. This allows for some basic keyboard interactions, like tab/up/down when on an item out of viewport. We keep the last focused rendered even if blurred for the scenario of tabbing in and and out of the VirtualizedList. Validated via UT.
There was a problem hiding this comment.
Is it possible to split up this diff into just changing the state representation of the existing contiguous region and then the changes in supporting discontiguous?
Or perhaps even more granular if possible in other aspects in the case that something ends up breaking some of our products, it might be clearer which parts are problematic or reveals problematic assumptions. And additionally if we could use a feature flag
|
I didn't see a straightforward way to split this much further. The discontiguous regions added to the mask are present before, but special cased into the render function. I agree we should gate this though. Way long ago, @TheSavior recomended keeping the changes inline for review, then cloning to a second component that can be injected before merging. That would allow FB folks to flight the change (the infrastructure is not in OSS). We're separately doing a gated rollout of this to Office right now. |
|
I can also let this change rest until we get feedback from a production-level audience internally. |
|
This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
github-actions
bot
added
the
Stale
|
This PR was closed because it has been stalled for 7 days with no activity. |
This change also includes the contents of facebook#32638 This change makes VirtualizedList track the last focused cell, through the capture phase of `onFocus`. It will keep the last focus cell, and its neighbors rendered. This allows for some basic keyboard interactions, like tab/up/down when on an item out of viewport. We keep the last focused rendered even if blurred for the scenario of tabbing in and and out of the VirtualizedList. Validated via UT.
Summary
This builds upon the
CellRenderMaskdata structure added with #31420, and VirtualizedList coverage added with #31401, to allow more discontinuous structures in VirtualizedList. This enables keyboard/a11y scenarios, where focus may be outside of a viewport-derived region.VirtualizedList currently keeps a [first, last] range as state, tracking the region of cells to render. The render functions uses this as an input, along with a few special cases to render more (sticky headers, initial render region.)
This change moves to instead keep state which describes discontinuous render regions. This mask is continually updated as the viewport changes, batch renders expand the region, etc. Special cases are baked into the render mask, with a relatively simple transformation from the mask to render function.
MS/FB folks have a video discussion about VirtualizedList here: https://msit.microsoftstream.com/video/fe01a1ff-0400-94b1-d4f1-f1eb924b1809
Changelog
[Internal] [Added] - Discontiguous VirtualizedList Regions
Test Plan
#31401 added quite a few snapshot tests, centering around the logic this change is touching. I manually validated RNTester FlatList examples (and there should be some upstream UI testing for them).