Closed oliviertassinari closed 3 years ago
Looking at the reproduction, we are completely off. We have 34 rows of 50px height with a viewport of 290px height. So the max scroll top should be = 34 * 50 - 290 = 1410px. Instead, it's 1250px 🙃.
Looking at the reproduction, we are completely off. We have 34 rows of 50px height with a viewport of 290px height. So the max scroll top should be = 34 * 50 - 290 = 1410px. Instead, it's 1250px 🙃.
Yeah, when I was playing with the virtualization to understand it I noticed that we use a different calculation for the height of the container. If we do the right thing the user will have to scroll more, but there will be no discontinuation when reaching the top.
There's also a blank space that might appear when calling scrollToIndexes
. Making the scrollTop to honor the theoretical position of the row will fix it too. BTW, we could improve this API to allow the user to determine if he wants: 1. scroll as little as possible to make the row visible or 2. scroll as much necessary so the row is at the top of the viewport. AG Grid in ensureIndexVisible
even allows to scroll to place the row at the middle or bottom of the viewport.
we could improve this API to allow the user to determine if he wants: 1. scroll as little as possible to make the row visible or 2. scroll as much necessary so the row is at the top of the viewport. AG Grid in ensureIndexVisible even allows to scroll to place the row at the middle or bottom of the viewport.
True, makes me think about the different options of https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView
Fixed by #2673
Current Behavior 😯
End-users can feel a difference between virtualization enable and disabled. The scrolling experience is not continuous, it jumps. The scroll height is also wrong, it's too short.
Expected Behavior 🤔
End-users shouldn't feel any difference between virtualization enable and disabled. Scrolling should be:
Steps to Reproduce 🕹
Steps:
You can also see there is a blank space at the bottom, but it's #414.
Context 🔦
Raised by @m4theushw in #1871.
It's KO. It would only make sense for this use case: https://ag-grid.com/react-grid/massive-row-count/.
I suspect the best course of action is to rewrite the virtualization logic. Use this as an opportunity to take the following constraints into account in the design.