You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or 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
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 calculateRange was 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 our measurements array is one-dimensional, the view is two-dimensional. Items which are far apart in measurements may 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 found every item that should be in-view in each lane. 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: getTotalSize misses some items because it's not always true that the last items in each lane will be the last N items in measurements (where N is the number of lanes.) This is because measurements is sorted by the start position, and if you have very uneven item sizes, the end positions 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.
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.
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
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 found every item that should be in-view in each lane. 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.