RFR: 8173321: TableView: Click on right trough has no effect when cell height is higher than viewport height [v3]

JoachimSchriek duke at openjdk.org
Sun Feb 5 10:46:57 UTC 2023


On Sat, 4 Feb 2023 15:16:28 GMT, Kevin Rushforth <kcr at openjdk.org> 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


More information about the openjfx-dev mailing list