List View: Recalculate window list when expanded state changes (fix logic for long nested lists) #53716
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewserong
commented
Aug 16, 2023
|
Size Change: -53 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
andrewserong
added
[Feature] List View
…e windowing logic
|
Update: getting closer. I think I've figured out a reliable way to do it by recalculating the windowing logic whenever the expanded state changes (in 04dc42e). I'll do some more testing tomorrow, but if this works, I think the next step will be to decide on a good prop name for it. |
|
Flaky tests detected in d5b02fb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5898964798
|
andrewserong
changed the title
andrewserong
changed the title
I've gone with |
This was referenced Aug 18, 2023
t-hamano
approved these changes
Aug 18, 2023
|
Thanks for reviewing @t-hamano! I think this change is fairly safe (as the main change is that recalculation happens more frequently, and only on expanded state change), so I'll merge this in now. Happy to follow up if anyone runs into any issues! |
25 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Fixes #41613, fixes #42031
Fix an issue with the windowing logic in the list view, where a short post containing a nested block with heaps of inner blocks (e.g. a Gallery with > 40 images) results in the windowing logic getting stuck and not updating when a user scrolls up and down the list.
Why?
When the list view is initially opened, if there are not many blocks at the root level, then no scrollbar will be present, so the fixed window list logic will not find the list view's scroll container in order to set up the windowing logic when a user scrolls up and down the list.
In normal circumstances (at the root level) this is fine — if you add or remove blocks, then
totalItemswithinuseFixedWindowListwill change, causing the windowing logic to be recalculated. However, if you have a container block with heaps of children (i.e. 40 or more), then expanding and collapsing the container block does not cause the recalculation to happen. In these cases (short root list, but a container block with heaps of children), then prior to this PR, the windowing logic can fail to update.How?
The fix is to ensure the fixed window list is recalculated whenever a block is expanded or collapsed. We can do this by passing the
expandedStatetouseFixedWindowListand then place theexpandedStatein the hook'suseLayoutEffectdependency arrays. This causes the windowing to be recalculated whenever the expanded state is changed, anywhere in the tree.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Note in the before screengrab below, when expanding the collapsed block and then scrolling down the list, that the window list is not updated, resulting in an expanse of white further down the list. In the After screengrab, the list is updated as the user scrolls up and down the list.