On Sat, 10 Aug 2024 07:10:03 GMT, Loay Ghreeb <d...@openjdk.org> wrote:
>> The PR title "The sorting of the SortedList can become invalid" seems to >> imply that the sorting can become incorrect, as in the incorrect order. >> >> This is not the case. The sorting is still correct. Claiming that the >> sorting is invalid because a newly added item was not placed in a specific >> location relative to other equal items is a bit of misrepresentation. >> >> So, if this PR wants to move forward, I think we first need to decide if we >> want to guarantee a specific behavior when inserting new but equal elements. >> If we do, then it should be documented, and also maintained when the list >> gets resorted (ie. resorts must be stable). > >> This is not the case. The sorting is still correct. Claiming that the >> sorting is invalid because a newly added item was not placed in a specific >> location relative to other equal items is a bit of misrepresentation. >> >> So, if this PR wants to move forward, I think we first need to decide if we >> want to guarantee a specific behavior when inserting new but equal elements. > > The issue isn’t just with adding new elements. It also affects the > `TableView` that populated with items from a `SortedList`, The `SortedList`'s > comparator is bound to the `TableView`'s comparator. > When the table is sorted by two columns and the second sort is removed, the > table should revert to sorting by just the first column. However, currently, > removing the second column from the sort order doesn't change the items order. > > For example: >  > > 1. The initial order is [0, 1, 2, 3] (index column). > 2. Sort by `Col1`, the order will be: [1, 0, 2, 3]. > 3. Add a second sort (Shift + Click) on `Col2`, the order will be: [1, 2, 3, > 0]. > 4. Shift + Click again on `Col2` to sort in descending order: [1, 0, 3, 2]. > 5. Shift + Click again on `Col2` to remove the sort from `Col2`. The order > will stay the same [1, 0, 3, 2], but the expected order is [1, 0, 2, 3] as in > step 2. > > <details> > <summary> Code </summary> > > > import javafx.application.Application; > import javafx.beans.property.ReadOnlyObjectWrapper; > import javafx.collections.FXCollections; > import javafx.collections.ObservableList; > import javafx.collections.transformation.SortedList; > import javafx.scene.Scene; > import javafx.scene.control.TableColumn; > import javafx.scene.control.TableView; > import javafx.stage.Stage; > > public class Main extends Application { > > @Override > public void start(Stage primaryStage) { > primaryStage.setScene(new Scene(createTableView())); > primaryStage.show(); > } > > private TableView<Data> createTableView() { > TableView<Data> tableView = new TableView<>(); > > TableColumn<Data, Integer> indexColumn = new TableColumn<>("#"); > indexColumn.setCellValueFactory(cell -> new > ReadOnlyObjectWrapper<>(cell.getValue().index())); > indexColumn.setSortable(false); > > TableColumn<Data, Integer> column1 = new TableColumn<>("Col1"); > column1.setCellValueFactory(cell -> new > ReadOnlyObjectWrapper<>(cell.getValue().val1())); > > TableColumn<Data, Integer> column2 = new TableColumn<>("Col2"); > ... @LoayGhreeb I agree with @hjohn and @nlisker. Absent any new information as to why this should be considered a bug in SortedList, this PR is unlikely to move forward. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1519#issuecomment-2400242972
