Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keyboard-friendly List Virtualization Behavior #335
base: main
Are you sure you want to change the base?
Keyboard-friendly List Virtualization Behavior #335
Changes from 4 commits
c1ca64d
1cbee9d
a09cffa
e78bff3
cdf77f0
4b055d0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this! I suggest this in facebook/react-native#29937 but that never was completed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Android also has this, with
PlatformConstants.constants.uiMode
being 'desk' for desktop.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this might be too much unnecessary configuration being able to adjust each
windowSize
. Any reason we'd ever want this to not be 1?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default
windowSize
for the viewport is fairly large, rendering a whole 10 screens above and below. This is a pretty aggressive amount of pre-rendering. It seems like existing setup optimizes for avoiding blanking on fast movement/interaction over memory consumption.The realization areas have the potential for similar issues. A conceivable user scenario is to quickly tab between items taller than a viewport. Items would possibly not be realized in time for the keyboard interaction. Keeping a larger window fixes this, but a viewport's worth of native controls could be a non-trivial amount of memory, or cause jank due to excessive native renders.
The right amount of prerendering here seems to depend on scenario. I think we can pick something sane as a default, but am not confident a specific size would work well in all scenarios
I think there might be some alternate APIs to make configuration cleaner, and want to play around with that. I think configurability would be useful here in a general-sense though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should call this
start
instead. The "home" terminology doesn't really fit well with our other APIs, and "start" is clear.