Skip to content
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

Bug Fix: Calculate getTotalSize correctly when using multiple lanes #660

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

AAAstorga
Copy link
Contributor

In my application I have a waterfall style layout so the last item is not guaranteed to be the furthest away. I was having issues with the sizing not being correct, so after digging in I found the root cause and implemented a fix.

Example of my measurements array (look at the last 2 items and notice the 2nd to last has a greater end property):
virtual

I also included a small optimization for one lane. I am happy to tweak the logic in any way so just let me know. Happy to contribute!

// If lanes is 1, use the last measurement's end, otherwise find the maximum end value among all measurements
end = this.options.lanes === 1
? (measurements[measurements.length - 1]?.end ?? 0)
: Math.max(...measurements.map((m) => m.end));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need to map over the whole measurements, this should work the same

Suggested change
: Math.max(...measurements.map((m) => m.end));
: Math.max(...measurements.slice(-this.options.lanes).map((m) => m.end));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great optimization. Just pushed a fix.

@piecyk piecyk merged commit 5bd44f6 into TanStack:main Feb 2, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants