RFR: 8323511: Scrollbar Click jumps inconsistent amount of pixels [v5]

Kevin Rushforth kcr at openjdk.org
Thu May 9 14:40:03 UTC 2024


On Thu, 9 May 2024 10:01:34 GMT, Florian Kirmaier <fkirmaier 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
>
> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   JDK-8323511: Mage getViewportLength() public and use it instead of getBlockIncrement()

@eduardsdv I left a couple comments on the API docs for the new public method.

Regarding the CSR, if you don't have a JBS account, perhaps you can ask @FlorianKirmaier or @johanvos to create the CSR for you.

And I agree with @andy-goryachev-oracle that you should be added a contributor. @FlorianKirmaier will need to do that, since only the author of the PR can add contributors.

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 VirtualFlow as computed during the layout pass.

Suggestion: use code case, `{@code VirtualFlow}`

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

> 1917:      * but it does not necessarily have to be the same value.
> 1918:      *
> 1919:      * @return the viewport length in pixel

That should be "in pixels" (plural).

Also, please add an `@since 23` tag after the return tag.

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

PR Review: https://git.openjdk.org/jfx/pull/1326#pullrequestreview-2048102268
PR Review Comment: https://git.openjdk.org/jfx/pull/1326#discussion_r1595534529
PR Review Comment: https://git.openjdk.org/jfx/pull/1326#discussion_r1595535571


More information about the openjfx-dev mailing list