RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v2]

[Ajit Ghaisas]

On Thu, 21 Jul 2022 21:10:10 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> …ontract
>> 
>> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect statement "Is functionally equivalent to calling <code>getSelectedIndices().contains(index)</code>." 
>> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) method to return true when at least one cell in *any* column is selected on the given row (was: *all* columns)
>> 3. change selectRowWhenInSingleCellSelectionMode() and selectRowWhenInSingleCellSelectionMode2() in TableViewSelectionModelImplTest to reflect new reality.
>> 
>> NOTE: proposed change alters semantics of isSelected(int) method (in the right direction, in my opinion).
>
> Andy Goryachev has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - 8235491: whitespace
>  - 8235491: javadoc

1. I see that you have made below change for cell selection mode -

- Before : `isSelected(int)` method used to return `True` only if **all** of the cells in that row were selected.
- After : `isSelected(int)` method returns `True` if **any** of the cells in that row are selected.

     Now, if someone wants to know - while in Cell selection mode, whether all of the cells in a particular row are selected? then they must use another already existing API `isSelected(int row, TableColumn<S,?> column)` with `column` parameter as `null`.
This is important and should be captured somewhere as recommendation.

2. Similar change needs to be made for `TreeTableView` as well
3. I have listed a minor comment on coding style

modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java line 2819:

> 2817:         public boolean isSelected(int index) {
> 2818:             final boolean isCellSelectionEnabled = isCellSelectionEnabled();
> 2819:             if (isCellSelectionEnabled) {

Code Style : Why to create a local variable with the same name as method it gets its value from?
In this case changing to `if (isCellSelectionEnabled())` condition check looks simple enough.

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

Changes requested by aghaisas (Reviewer).

PR: https://git.openjdk.org/jfx/pull/839