RFR: 8323511 Scrollbar Click jumps inconsistent amount of pixels

Marius Hanl mhanl at openjdk.org
Sun Mar 31 14:27:37 UTC 2024


On Thu, 11 Jan 2024 12:17:11 GMT, Marius Hanl <mhanl at 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


More information about the openjfx-dev mailing list