On Thu, 11 Jan 2024 12:17:11 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> As seen in the unit test of the PR, when we click on the area above/below >> the scrollbar the position jumps - but the jump is now not always consistent. >> In the current version on the last cell - the UI always jumps to the top. In >> the other cases, the assumed default cell height is used. >> >> With this PR, always the default cell height is used, to determine how much >> is scrolled. >> This makes the behavior more consistent. >> >> Especially from the unit-test, it's clear that with this PR the behavior is >> much more consistent. >> >> This is also related to the following PR: >> https://github.com/openjdk/jfx/pull/1194 > > Agree, with all the tests added, especially in this area in > https://github.com/openjdk/jfx/pull/1194 and in > https://github.com/openjdk/jfx/pull/1246, it is much easier for us to catch > regression. I will also have a look in the next days. I also noted that I got > weird scrolling behaviour once before, but could never reproduce it. > The suggested patch changes the conceptual idea of `VirtualFlow.scrollTo(int > index)` where a negative index is not specified (this is probably what > @Maran23 asked at [#1326 > (comment)](https://github.com/openjdk/jfx/pull/1326#discussion_r1530233902) . Yes, this is exactly what I mean and where I do not know if this is the right approach. > The way the scrollTo(int index) is modified doesn't sound right to me Agree, it sounds somewhat weird to me that when `scrollTo` is called with index = -1, that means we just scroll up more gradually (not to the top of the cell). > If the latter is the preferred case, this looks to a behavior that is more > similar to the Event that is received when the mousewheel is used (and which > invokes VirtualFlow.scrollPixels(double delta)) I completely agree. This sounds like we may should call `scrollPixels` directly instead. As @johanvos mentioned, we also need to change the "specification" in the comment at least. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1326#issuecomment-2028769290
