On Mon, 17 May 2021 16:16:16 GMT, Michael Strauß <[email protected]> wrote:
>> The documentation for `ObservableListBase.nextRemove` states that a single
>> change always refers to the current state of the list, which likely means
>> that multiple disjoint removed ranges need to be applied in order, otherwise
>> the next change's `getFrom` doesn't refer to the correct index.
>>
>> `SelectedItemsReadOnlyObservableList` doesn't apply removals to
>> `itemsRefList`, which means that subsequent removals will refer to the wrong
>> index when retrieving the removed elements. This PR fixes the calculation of
>> the current index.
>
> Michael Strauß has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 10 commits:
>
> - Merge branch 'master' into fixes/JDK-8196065
>
> # Conflicts:
> #
> modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java
> - Merge branch 'master' into fixes/JDK-8196065
> - Merge branch 'master' into fixes/JDK-8196065
>
> # Conflicts:
> #
> modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java
> - Cleanup
> - Fixed clear-and-select change notification
> - Add failing tests
> - Cleanup
> - Added tests
> - Fix incorrect index when multiple remove changes are handled in
> SelectedItemsReadOnlyObservableList
> - Add failing test
modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/SelectedItemsReadOnlyObservableListTest.java
line 120:
> 118: assertEquals(change(replaced(0, range("foo"), range("bar"))),
> changes.get(0));
> 119: }
> 120:
Another thingy I don't quite understand, sry ;)
What do you want to test here? We have:
selectedItems before: [foo, bar]
selectedItems after: [bar, foo]
so we need a change (the "should be" snippet in the description of this method)
replaced [foo, bar] by [bar, foo] at 0
we get:
replaced [foo] by [bar] at 0
which is an incorrect notification (because it's incomplete, doesn't notify at
all about the replace at 1) covered by a passing test .. feels fishy. The other
way round: if selectedItems cannot react to such a use-case with a correct
notification, it might be broken?
-------------
PR: https://git.openjdk.java.net/jfx/pull/478
