On Sat, 4 Feb 2023 15:16:28 GMT, Kevin Rushforth <[email protected]> wrote:
>> JoachimSchriek has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Deleted trailing whitespace
>
> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/VirtualScrollBar.java
> line 135:
>
>> 133: /**
>> 134: * JDK-8173321: Click on trough has no effect when cell
>> height > viewport height:
>> 135: * Solution: Scroll one cell further up resp. down if only
>> one cell is shown.
>
> We generally do not refer to specific bug IDs in source code comments.
> Rather, describe the purpose of this block. Also, please spell out `resp.` --
> I can't figure out what it is supposed to mean.
I will change the comment in my next commit to:
Scroll one cell further in the direction the user has clicked if only one cell
is shown.
Otherwise, a click on the trough would have no effect when cell height >
viewport height.
> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/VirtualScrollBar.java
> line 151:
>
>> 149: IndexedCell cell = firstVisibleCell;
>> 150: if (cell == null)
>> 151: return;
>
> Unless the if statement, including its target, is entirely on one line, you
> _must_ enclose the target of the `if` with curly braces, like this:
>
>
> if (cell == null) {
> return;
> }
>
>
> Since it fits on one line, the prior code is also acceptable:
>
>
> if (cell == null) return;
I will change this to the first alternative in my next commit.
> modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/VirtualScrollBar.java
> line 156:
>
>> 154: IndexedCell cell = lastVisibleCell;
>> 155: if (cell == null)
>> 156: return;
>
> Either revert this or enclose in curly braces.
I will change this to the first alternative in my next commit.
-------------
PR: https://git.openjdk.org/jfx/pull/985
