On Tue, 7 May 2024 09:39:23 GMT, Florian Kirmaier <fkirma...@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
>
> Florian Kirmaier has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   JDK-8323511: Use viewport length as block increment

modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
 line 1808:

> 1806:      * @return the value in pixels
> 1807:      */
> 1808:     public double getBlockIncrement() {

I'd rather not add that method but instead make `getViewportLength()` public. 
That would benefit other use cases, and it is more behavior-agnostic. The 
"viewportLength" is easier to define in a contract than a "blockIncrement" and 
since this is exactly what you return, it sounds like a reasonable thing to do.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1326#discussion_r1595180937

Reply via email to