-
-
Notifications
You must be signed in to change notification settings - Fork 331
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
fix(virtual-core): ensure all items render when content height is less than or equal to viewport height #955
Conversation
…s than or equal to viewport height
|
View your CI Pipeline Execution ↗ for commit 2dbd2c3.
☁️ Nx Cloud last updated this comment at |
umm.. I think the test fails because the number of renders increases by one in addition to the dependency array. Is there a better way to fix the bug without fixing the test? |
@@ -456,6 +456,7 @@ export class Virtualizer< | |||
this.isScrolling, | |||
this.range ? this.range.startIndex : null, | |||
this.range ? this.range.endIndex : null, | |||
this.scrollRect, |
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.
hmm we are reading the scrollRect in calculateRange via getSize, so the range should be correct
Thanks, @SaeWooKKang, for your work on this! I’ve found a better approach to address this issue in #957, so I’ll be closing this PR in favor of that solution. I really appreciate your effort!. |
@piecyk I'm glad it was solved in a better way. Thank you for your efforts! |
Fixes #871
Problem:
This issue occurs because when the scroll container size (scrollRect) changes and the
maybeNotify
function is called, scrollRect is not included in the dependency array, causing the virtual item range to not be calculated correctly.Solution:
I added scrollRect value to the dependency array of the
maybeNotify
function, ensuring that the virtual item range is recalculated correctly when the scroll container size changes.