changeset 4064:e0434ac66df4

RT-31165: Editing is broken in list view and tree view. Tests developed for all relevant controls, although disabled for TreeTableView until an additional issue can be resolved.
author jgiles
date Wed, 26 Jun 2013 07:34:46 +1200
parents 8df61a4fc74c
children 5b75776bb378
files javafx-ui-controls/src/javafx/scene/control/ListCell.java javafx-ui-controls/src/javafx/scene/control/TreeCell.java javafx-ui-controls/src/javafx/scene/control/TreeTableCell.java javafx-ui-controls/test/javafx/scene/control/ListViewTest.java javafx-ui-controls/test/javafx/scene/control/TableViewTest.java javafx-ui-controls/test/javafx/scene/control/TreeTableViewTest.java javafx-ui-controls/test/javafx/scene/control/TreeViewTest.java
diffstat 7 files changed, 188 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/javafx-ui-controls/src/javafx/scene/control/ListCell.java	Mon Jun 24 08:30:06 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/ListCell.java	Wed Jun 26 07:34:46 2013 +1200
@@ -299,13 +299,28 @@
      * Public API                                                              *
      *                                                                         *
      **************************************************************************/
-    
+
+    private int index = -1;
+
     /** {@inheritDoc} */
     @Override void indexChanged() {
+        final int oldIndex = index;
         super.indexChanged();
-        updateItem();
-        updateSelection();
-        updateFocus();
+        index = getIndex();
+
+        if (isEditing() && index == oldIndex) {
+            // no-op
+            // Fix for RT-31165 - if we (needlessly) update the index whilst the
+            // cell is being edited it will no longer be in an editing state.
+            // This means that in certain (common) circumstances that it will
+            // appear that a cell is uneditable as, despite being clicked, it
+            // will not change to the editing state as a layout of VirtualFlow
+            // is immediately invoked, which forces all cells to be updated.
+        } else {
+            updateItem();
+            updateSelection();
+            updateFocus();
+        }
     }
 
     /** {@inheritDoc} */
--- a/javafx-ui-controls/src/javafx/scene/control/TreeCell.java	Mon Jun 24 08:30:06 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/TreeCell.java	Wed Jun 26 07:34:46 2013 +1200
@@ -443,14 +443,25 @@
     private int index = -1;
 
     @Override void indexChanged() {
-        int oldIndex = index;
+        final int oldIndex = index;
+
         index = getIndex();
         
         // when the cell index changes, this may result in the cell
         // changing state to be selected and/or focused.
-        updateItem();
-        updateSelection();
-        updateFocus();
+        if (isEditing() && index == oldIndex) {
+            // no-op
+            // Fix for RT-31165 - if we (needlessly) update the index whilst the
+            // cell is being edited it will no longer be in an editing state.
+            // This means that in certain (common) circumstances that it will
+            // appear that a cell is uneditable as, despite being clicked, it
+            // will not change to the editing state as a layout of VirtualFlow
+            // is immediately invoked, which forces all cells to be updated.
+        } else {
+            updateItem();
+            updateSelection();
+            updateFocus();
+        }
     }
     
     private void updateItem() {
--- a/javafx-ui-controls/src/javafx/scene/control/TreeTableCell.java	Mon Jun 24 08:30:06 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/TreeTableCell.java	Wed Jun 26 07:34:46 2013 +1200
@@ -386,7 +386,9 @@
      * Private Implementation                                                  *
      *                                                                         *
      **************************************************************************/
-    
+
+    private int index = -1;
+
     @Override void indexChanged() {
         super.indexChanged();
         // Ideally we would just use the following two lines of code, rather
@@ -394,9 +396,24 @@
         // RT-22428 where all the columns are collapsed.
         // itemDirty = true;
         // requestLayout();
-        updateItem();
-        updateSelection();
-        updateFocus();
+
+        final int oldIndex = index;
+        super.indexChanged();
+        index = getIndex();
+
+        if (isEditing() && index == oldIndex) {
+            // no-op
+            // Fix for RT-31165 - if we (needlessly) update the index whilst the
+            // cell is being edited it will no longer be in an editing state.
+            // This means that in certain (common) circumstances that it will
+            // appear that a cell is uneditable as, despite being clicked, it
+            // will not change to the editing state as a layout of VirtualFlow
+            // is immediately invoked, which forces all cells to be updated.
+        } else {
+            updateItem();
+            updateSelection();
+            updateFocus();
+        }
     }
     
     private boolean isLastVisibleColumn = false;
--- a/javafx-ui-controls/test/javafx/scene/control/ListViewTest.java	Mon Jun 24 08:30:06 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/ListViewTest.java	Wed Jun 26 07:34:46 2013 +1200
@@ -45,11 +45,14 @@
 import javafx.collections.ObservableList;
 import javafx.geometry.Orientation;
 import javafx.scene.Node;
+import javafx.scene.Scene;
 import javafx.scene.control.cell.CheckBoxListCell;
+import javafx.scene.control.cell.ComboBoxListCell;
 import javafx.scene.layout.StackPane;
 import javafx.scene.layout.VBox;
 import javafx.util.Callback;
 
+import com.sun.javafx.scene.control.infrastructure.MouseEventFirer;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -615,4 +618,32 @@
         final double afterEmptiedWidth = listView.prefWidth(-1);
         assertEquals(initialWidth, afterEmptiedWidth, 0.00);
     }
+
+    @Test public void test_rt31165() {
+        final ObservableList names = FXCollections.observableArrayList("Adam", "Alex", "Alfred", "Albert");
+        final ObservableList data = FXCollections.observableArrayList();
+        for (int i = 0; i < 18; i++) {
+            data.add(""+i);
+        }
+
+        final ListView listView = new ListView(data);
+        listView.setPrefSize(200, 250);
+        listView.setEditable(true);
+        listView.setCellFactory(ComboBoxListCell.forListView(names));
+
+        IndexedCell cell = VirtualFlowTestUtils.getCell(listView, 1);
+        assertEquals("1", cell.getText());
+        assertFalse(cell.isEditing());
+
+        listView.edit(1);
+
+        assertEquals(1, listView.getEditingIndex());
+        assertTrue(cell.isEditing());
+
+        VirtualFlowTestUtils.getVirtualFlow(listView).requestLayout();
+        Toolkit.getToolkit().firePulse();
+
+        assertEquals(1, listView.getEditingIndex());
+        assertTrue(cell.isEditing());
+    }
 }
--- a/javafx-ui-controls/test/javafx/scene/control/TableViewTest.java	Mon Jun 24 08:30:06 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/TableViewTest.java	Wed Jun 26 07:34:46 2013 +1200
@@ -48,9 +48,12 @@
 import javafx.collections.ObservableList;
 import javafx.event.EventHandler;
 import javafx.scene.control.cell.CheckBoxTableCell;
+import javafx.scene.control.cell.ChoiceBoxTableCell;
+import javafx.scene.control.cell.ComboBoxListCell;
 import javafx.scene.control.cell.PropertyValueFactory;
 import javafx.util.Callback;
 
+import com.sun.javafx.tk.Toolkit;
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
@@ -1325,4 +1328,41 @@
         VirtualFlowTestUtils.assertCellEmpty(VirtualFlowTestUtils.getCell(tableView, 2));
         VirtualFlowTestUtils.assertCellEmpty(VirtualFlowTestUtils.getCell(tableView, 3));
     }
+
+    @Test public void test_rt31165() {
+        final ObservableList names = FXCollections.observableArrayList("Adam", "Alex", "Alfred", "Albert");
+
+        final TableView<Person> tableView = new TableView<Person>();
+        tableView.setEditable(true);
+        tableView.setItems(FXCollections.observableArrayList(
+            new Person("Jacob", "Smith", "jacob.smith@example.com"),
+            new Person("Jim", "Bob", "jim.bob@example.com")
+        ));
+
+        TableColumn firstNameCol = new TableColumn("First Name");
+        firstNameCol.setCellValueFactory(new PropertyValueFactory<Person, String>("firstName"));
+        firstNameCol.setCellFactory(ChoiceBoxTableCell.forTableColumn(names));
+        firstNameCol.setEditable(true);
+
+        tableView.getColumns().add(firstNameCol);
+
+        IndexedCell cell = VirtualFlowTestUtils.getCell(tableView, 1, 0);
+        assertEquals("Jim", cell.getText());
+        assertFalse(cell.isEditing());
+
+        tableView.edit(1, firstNameCol);
+
+        TablePosition editingCell = tableView.getEditingCell();
+        assertEquals(1, editingCell.getRow());
+        assertEquals(firstNameCol, editingCell.getTableColumn());
+        assertTrue(cell.isEditing());
+
+        VirtualFlowTestUtils.getVirtualFlow(tableView).requestLayout();
+        Toolkit.getToolkit().firePulse();
+
+        editingCell = tableView.getEditingCell();
+        assertEquals(1, editingCell.getRow());
+        assertEquals(firstNameCol, editingCell.getTableColumn());
+        assertTrue(cell.isEditing());
+    }
 }
--- a/javafx-ui-controls/test/javafx/scene/control/TreeTableViewTest.java	Mon Jun 24 08:30:06 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/TreeTableViewTest.java	Wed Jun 26 07:34:46 2013 +1200
@@ -59,6 +59,7 @@
 import javafx.scene.control.TreeTableView.TreeTableViewFocusModel;
 import javafx.scene.control.cell.CheckBoxTreeTableCell;
 import javafx.scene.control.cell.PropertyValueFactory;
+import javafx.scene.control.cell.TextFieldTreeTableCell;
 import javafx.scene.control.cell.TreeItemPropertyValueFactory;
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.StackPane;
@@ -2061,4 +2062,36 @@
         VirtualFlowTestUtils.assertCellEmpty(VirtualFlowTestUtils.getCell(tableView, 2));
         VirtualFlowTestUtils.assertCellEmpty(VirtualFlowTestUtils.getCell(tableView, 3));
     }
+
+    @Ignore("This bug is not yet fixed")
+    @Test public void test_rt31165() {
+        installChildren();
+        treeTableView.setEditable(true);
+
+        TreeTableColumn firstNameCol = new TreeTableColumn("First Name");
+        firstNameCol.setCellValueFactory(new Callback<TreeTableColumn.CellDataFeatures, ObservableValue>() {
+            @Override public ObservableValue call(TreeTableColumn.CellDataFeatures param) {
+                return new ReadOnlyStringWrapper("TEST");
+            }
+        });
+        firstNameCol.setCellFactory(TextFieldTreeTableCell.forTreeTableColumn());
+        firstNameCol.setEditable(true);
+
+        treeTableView.getColumns().add(firstNameCol);
+
+        IndexedCell cell = VirtualFlowTestUtils.getCell(treeTableView, 1, 0);
+        assertEquals("TEST", cell.getText());
+        assertFalse(cell.isEditing());
+
+        treeTableView.edit(child1);
+
+        assertEquals(child1, treeTableView.getEditingItem());
+        assertTrue(cell.isEditing());
+
+        VirtualFlowTestUtils.getVirtualFlow(treeTableView).requestLayout();
+        Toolkit.getToolkit().firePulse();
+
+        assertEquals(child1, treeTableView.getEditingItem());
+        assertTrue(cell.isEditing());
+    }
 }
--- a/javafx-ui-controls/test/javafx/scene/control/TreeViewTest.java	Mon Jun 24 08:30:06 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/TreeViewTest.java	Wed Jun 26 07:34:46 2013 +1200
@@ -56,6 +56,7 @@
 import javafx.scene.Scene;
 import javafx.scene.control.cell.CheckBoxTreeCell;
 import javafx.scene.control.cell.PropertyValueFactory;
+import javafx.scene.control.cell.TextFieldTreeCell;
 import javafx.scene.control.cell.TreeItemPropertyValueFactory;
 import javafx.scene.layout.VBox;
 import javafx.scene.paint.Color;
@@ -1009,11 +1010,13 @@
         treeView.setRoot(rootItem);
         treeView.setMinHeight(100);
         treeView.setPrefHeight(100);
-        treeView.setCellFactory(CheckBoxTreeCell.forTreeView(new Callback<TreeItem<String>, ObservableValue<Boolean>>() {
-            public javafx.beans.value.ObservableValue<Boolean> call(TreeItem<String> param) {
-                return new ReadOnlyBooleanWrapper(true);
-            }
-        }));
+        treeView.setCellFactory(
+                CheckBoxTreeCell.forTreeView(
+                        new Callback<TreeItem<String>, ObservableValue<Boolean>>() {
+                            public javafx.beans.value.ObservableValue<Boolean> call(TreeItem<String> param) {
+                                return new ReadOnlyBooleanWrapper(true);
+                            }
+                        }));
 
         // because only the first row has data, all other rows should be
         // empty (and not contain check boxes - we just check the first four here)
@@ -1023,4 +1026,25 @@
         VirtualFlowTestUtils.assertCellEmpty(VirtualFlowTestUtils.getCell(treeView, 2));
         VirtualFlowTestUtils.assertCellEmpty(VirtualFlowTestUtils.getCell(treeView, 3));
     }
+
+    @Test public void test_rt31165() {
+        installChildren();
+        treeView.setEditable(true);
+        treeView.setCellFactory(TextFieldTreeCell.forTreeView());
+
+        IndexedCell cell = VirtualFlowTestUtils.getCell(treeView, 1);
+        assertEquals(child1.getValue(), cell.getText());
+        assertFalse(cell.isEditing());
+
+        treeView.edit(child1);
+
+        assertEquals(child1, treeView.getEditingItem());
+        assertTrue(cell.isEditing());
+
+        VirtualFlowTestUtils.getVirtualFlow(treeView).requestLayout();
+        Toolkit.getToolkit().firePulse();
+
+        assertEquals(child1, treeView.getEditingItem());
+        assertTrue(cell.isEditing());
+    }
 }