On Sat, 10 Aug 2024 11:53:00 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>>> 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... > >> 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. > > Why is that the expected order? It is "a" possible, but valid order. When > only sorting on `Col1` then in my opinion [1, 0, 2, 3], [1, 0, 3, 2], [1, 2, > 0, 3], etc.. are all valid sort orders. > > If we want to distinguish between equal elements to give them a fixed > sub-ordering (and the inverse one I suppose when the sort is descending) then > what should the distinguishing factor be? Its position in the underlying > unsorted list? The time it was added to the underlying list? They need not > be the same. Using the position in the underlying list could be quite random > if the source of the list doesn't guarantee a fixed position (from an > unsorted database query for example). > > What if the underlying list is another sorted list or a filtered list, can we > even rely on the indices those provide? @hjohn I was confused as well, but the [`TableView` docs](https://openjfx.io/javadoc/22/javafx.controls/javafx/scene/control/TableView.html#sorting-heading) say: > Starting with JavaFX 8.0 (and the introduction of > [SortedList](https://openjfx.io/javadoc/22/javafx.base/javafx/collections/transformation/SortedList.html)), > it is now possible to have the collection return to the unsorted state when > there are no columns as part of the TableView [sort > order](https://openjfx.io/javadoc/22/javafx.controls/javafx/scene/control/TableView.html#getSortOrder()). > To do this, you must create a SortedList instance, and bind its > [comparator](https://openjfx.io/javadoc/22/javafx.base/javafx/collections/transformation/SortedList.html#comparatorProperty()) > property to the TableView > [comparator](https://openjfx.io/javadoc/22/javafx.controls/javafx/scene/control/TableView.html#comparatorProperty()) > property So I think that the confusion stems from the fuzzy guarantee that removing the column-based sorting will return the table to *the* unsorted state before that sorting (and not to *a* unsorted state). That might mean that `TableView` will have to maintain the initial sorting somewhere. Or... it's a bad documentation :) I still don't see how this is a bug in `SortedList`. It could be a bug in `TableView`. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1519#issuecomment-2284512503
