fix(virtual-core): expand range in masonry layouts to catch items from all lanes #937
+44
−14
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
|
View your CI Pipeline Execution ↗ for commit bc1b031.
☁️ Nx Cloud last updated this comment at |
|
I closed this temporarily because I was still hitting problems in my application, but I was able to verify that the bug was just in my application and not this PR. |
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.
Fixes #936
Fixes #665
This PR resolves two issues related to items being "missed" (i.e. unexpectedly cut-off or not rendered) in masonry layouts.
The first problem is that
calculateRangewas not changed when we added support for masonry layouts. This function assumes that items are purely sequential in the layout: it finds an item close to the scroll offset, and expands forward until it finds an item that exceeds the window. Masonry layouts do not behave like this. Even though ourmeasurementsarray is one-dimensional, the view is two-dimensional. Items which are far apart inmeasurementsmay still be rendered side-by-side, and that needs to be accounted for.The solution I came up with is to expand the range in both directions until we've accounted for items in-view from all lanes. This feels somewhat naive, and perhaps there's a more performant solution, but I think for the majority of masonry layouts, it should perform just fine.
The second problem is in the same vein:
getTotalSizemisses some items because it's not always true that the last items in each lane will be the last N items inmeasurements(where N is the number of lanes.) This is becausemeasurementsis sorted by thestartposition, and if you have very uneven item sizes, theendpositions could be very different from each other. I changed this to work in a similar way: it scans from the bottom up until it's found end positions for all lanes, and then selects the maximum.Both of the issues can be seen in the reproduction link in #936.