Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8092759

[TableView] Table content does not sort again after changing the data set

    Details

      Description

      1. Fill a table with random sortable data
      2. Click a header of the table/column to sort them
      3. Fill the table again using the setItems() method
      4. The header keeps displaying an arrow indicating the way the data are supposed to be ordered but they aren't.

      Code to reproduce the issue:

      import javafx.application.Application;
      import javafx.beans.property.SimpleStringProperty;
      import javafx.beans.value.ObservableValue;
      import javafx.collections.FXCollections;
      import javafx.event.ActionEvent;
      import javafx.event.EventHandler;
      import javafx.geometry.Insets;
      import javafx.scene.Scene;
      import javafx.scene.control.Button;
      import javafx.scene.control.TableColumn;
      import javafx.scene.control.TableView;
      import javafx.scene.control.TableColumn.CellDataFeatures;
      import javafx.scene.layout.VBox;
      import javafx.stage.Stage;
      import javafx.util.Callback;

      public class Test3 extends Application
      {
      public static void main(String[] args)
      {
      launch(args);
      }

      @Override
      public void start(Stage primaryStage) throws Exception
      {
      final TableColumn<String, String> column = new TableColumn<String, String>("Click to sort");
      column.setCellValueFactory(new Callback<TableColumn.CellDataFeatures<String,String>, ObservableValue<String>>()
      {
      @Override
      public ObservableValue<String> call(CellDataFeatures<String, String> param)
      {
      return new SimpleStringProperty(param.getValue());
      }
      });

      final TableView<String> table = new TableView<String>();
      table.setColumnResizePolicy(TableView.CONSTRAINED_RESIZE_POLICY);
      table.getColumns().add(column);

      final Button button = new Button("Fill table");

      final VBox vbox = new VBox(10.0, table, button);
      vbox.setPadding(new Insets(10.0));

      primaryStage.setScene(new Scene(vbox));
      primaryStage.show();

      button.onActionProperty().set(new EventHandler<ActionEvent>()
      {
      @Override
      public void handle(ActionEvent event)
      {
      table.setItems(FXCollections.observableArrayList("s", "o", "r", "t", "m", "e"));
      }
      });
      }
      }

        Activity

        Hide
        jgiles Jonathan Giles added a comment -
        This is a fair complaint. The basic problem is that the sort is applied on the list itself - so when the list changes the TableView does not (currently) double-check whether the sort criteria applies to the new items list.

        There are two options when the items list changes when the TableView has sort criteria specified:

        1) Reset the sort criteria so that the list remains in the order specified. This means all columns go back to being unsorted in the UI.
        2) Sort the list actively as the items change

        From a gut feel point of view, 2) seems like the better option, but I'm inclined to go with 1). My reasoning is the following:

        1) It feels wrong to me to change the order of a list given to me by a developer without their express permission. It feels even weirder if we take this to the logical conclusion and say we must always maintain the order of the items list whenever it changes (e.g. when a new item is added or removed, rather than the entire list being replaced as in the bug report above).

        2) When designing TableView we decided to defer to SortedList as our 'smart' way of handling sorting. Unfortunately, SortedList could not ship with 2.0, but has subsequently shipped in a release. With SortedList you can bind the SortedList.comparatorProperty() to the TableView.comparatorProperty(), such that the SortedList remains sorted according to the TableView sort order. This allows for the behavior expected, both in the bug report and in the 'logical conclusion' reached in my previous point.
        Show
        jgiles Jonathan Giles added a comment - This is a fair complaint. The basic problem is that the sort is applied on the list itself - so when the list changes the TableView does not (currently) double-check whether the sort criteria applies to the new items list. There are two options when the items list changes when the TableView has sort criteria specified: 1) Reset the sort criteria so that the list remains in the order specified. This means all columns go back to being unsorted in the UI. 2) Sort the list actively as the items change From a gut feel point of view, 2) seems like the better option, but I'm inclined to go with 1). My reasoning is the following: 1) It feels wrong to me to change the order of a list given to me by a developer without their express permission. It feels even weirder if we take this to the logical conclusion and say we must always maintain the order of the items list whenever it changes (e.g. when a new item is added or removed, rather than the entire list being replaced as in the bug report above). 2) When designing TableView we decided to defer to SortedList as our 'smart' way of handling sorting. Unfortunately, SortedList could not ship with 2.0, but has subsequently shipped in a release. With SortedList you can bind the SortedList.comparatorProperty() to the TableView.comparatorProperty(), such that the SortedList remains sorted according to the TableView sort order. This allows for the behavior expected, both in the bug report and in the 'logical conclusion' reached in my previous point.
        Hide
        jsmucrjfx Jan Smucr (Inactive) added a comment -
        Thank you for your response. I'd go with the first option as well.
        Show
        jsmucrjfx Jan Smucr (Inactive) added a comment - Thank you for your response. I'd go with the first option as well.
        Hide
        jgiles Jonathan Giles added a comment -
        I should add that to handle this correctly we can't just clear out the TableView.sortOrder list whenever the items list changes - we may need to special case for situations where the incoming list is a SortedList (or take some alternative approach, but this is the first option that sticks out to me). It's a shame to have special-case code for SortedList, but we already do that for other reasons.
        Show
        jgiles Jonathan Giles added a comment - I should add that to handle this correctly we can't just clear out the TableView.sortOrder list whenever the items list changes - we may need to special case for situations where the incoming list is a SortedList (or take some alternative approach, but this is the first option that sticks out to me). It's a shame to have special-case code for SortedList, but we already do that for other reasons.
        Hide
        jgiles Jonathan Giles added a comment -
        Unit tests:
        javafx.scene.control.TableViewTest.test_rt35763_observableList()
        javafx.scene.control.TableViewTest.test_rt35763_sortedList()
        javafx.scene.control.TreeTableViewTest.test_rt35763()

        Test application:
        Attached to jira

        Changeset: 91c0001a39b0
        Author: jgiles
        Date: Tue Feb 11 10:23:44 2014 +1300
        URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/91c0001a39b0
        Show
        jgiles Jonathan Giles added a comment - Unit tests: javafx.scene.control.TableViewTest.test_rt35763_observableList() javafx.scene.control.TableViewTest.test_rt35763_sortedList() javafx.scene.control.TreeTableViewTest.test_rt35763() Test application: Attached to jira Changeset: 91c0001a39b0 Author: jgiles Date: Tue Feb 11 10:23:44 2014 +1300 URL: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/91c0001a39b0
        Hide
        pkeckjfx Philipp Keck (Inactive) added a comment -
        Show
        pkeckjfx Philipp Keck (Inactive) added a comment - Probably regression: https://javafx-jira.kenai.com/browse/RT-36425

          People

          • Assignee:
            jgiles Jonathan Giles
            Reporter:
            jsmucrjfx Jan Smucr (Inactive)
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Imported: