On Thu, 21 Jul 2022 21:10:10 GMT, Andy Goryachev <[email protected]> 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
