On Thu, 9 May 2024 14:49:26 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: Adjust javadoc of the getViewportLength()

The API changes look good. I left one minor wording suggestion on the API docs.

The new behavior feels more consistent to me, so I have no concerns. I'll leave 
it to @johanvos and @andy-goryachev-oracle to formally review it.

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

> 1913: 
> 1914:     /**
> 1915:      * The length of the viewport portion of the {@code VirtualFlow} as 
> computed during the layout pass.

Suggested change:

"Returns the length of the viewport ..."

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

PR Review: https://git.openjdk.org/jfx/pull/1326#pullrequestreview-2061722236
PR Review Comment: https://git.openjdk.org/jfx/pull/1326#discussion_r1603939589

Reply via email to