changeset 6885:143503528295

RT-36885: [TreeView, TreeTableView] Focus doesn't follow the selection after adding children
author jgiles
date Tue, 29 Apr 2014 12:13:17 +1200
parents da036c46b124
children c4d8457b7064
files modules/controls/src/main/java/javafx/scene/control/TreeTableView.java modules/controls/src/main/java/javafx/scene/control/TreeView.java modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java
diffstat 4 files changed, 129 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Tue Apr 29 10:13:39 2014 +1200
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Tue Apr 29 12:13:17 2014 +1200
@@ -3104,13 +3104,18 @@
                         shift = - e.getTreeItem().previousExpandedDescendentCount + 1;
                     }
                 } else if (e.wasAdded()) {
-                    for (int i = 0; i < e.getAddedChildren().size(); i++) {
-                        TreeItem<S> item = e.getAddedChildren().get(i);
-                        row = treeTableView.getRow(item);
-                        
-                        if (item != null && row <= getFocusedIndex()) {
-//                            shift = e.getTreeItem().isExpanded() ? e.getAddedSize() : 0;
-                            shift += item.getExpandedDescendentCount(false);
+                    // get the TreeItem the event occurred on - we only need to
+                    // shift if the tree item is expanded
+                    TreeItem<S> eventTreeItem = e.getTreeItem();
+                    if (eventTreeItem.isExpanded()) {
+                        for (int i = 0; i < e.getAddedChildren().size(); i++) {
+                            // get the added item and determine the row it is in
+                            TreeItem<S> item = e.getAddedChildren().get(i);
+                            row = treeTableView.getRow(item);
+
+                            if (item != null && row <= getFocusedIndex()) {
+                                shift += item.getExpandedDescendentCount(false);
+                            }
                         }
                     }
                 } else if (e.wasRemoved()) {
--- a/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Tue Apr 29 10:13:39 2014 +1200
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Tue Apr 29 12:13:17 2014 +1200
@@ -1521,13 +1521,18 @@
                         shift = - e.getTreeItem().previousExpandedDescendentCount + 1;
                     }
                 } else if (e.wasAdded()) {
-                    for (int i = 0; i < e.getAddedChildren().size(); i++) {
-                        TreeItem<T> item = e.getAddedChildren().get(i);
-                        row = treeView.getRow(item);
-                        
-                        if (item != null && row <= getFocusedIndex()) {
-//                            shift = e.getTreeItem().isExpanded() ? e.getAddedSize() : 0;
-                            shift += item.getExpandedDescendentCount(false);
+                    // get the TreeItem the event occurred on - we only need to
+                    // shift if the tree item is expanded
+                    TreeItem<T> eventTreeItem = e.getTreeItem();
+                    if (eventTreeItem.isExpanded()) {
+                        for (int i = 0; i < e.getAddedChildren().size(); i++) {
+                            // get the added item and determine the row it is in
+                            TreeItem<T> item = e.getAddedChildren().get(i);
+                            row = treeView.getRow(item);
+
+                            if (item != null && row <= getFocusedIndex()) {
+                                shift += item.getExpandedDescendentCount(false);
+                            }
                         }
                     }
                 } else if (e.wasRemoved()) {
--- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Tue Apr 29 10:13:39 2014 +1200
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Tue Apr 29 12:13:17 2014 +1200
@@ -3346,4 +3346,56 @@
 
         sl.dispose();
     }
+
+    @Test public void test_rt36885() {
+        test_rt36885(false);
+    }
+
+    @Test public void test_rt36885_addChildAfterSelection() {
+        test_rt36885(true);
+    }
+
+    private void test_rt36885(boolean addChildToAAfterSelection) {
+        TreeItem<String> root = new TreeItem<>("Root");         // 0
+                TreeItem<String> a = new TreeItem<>("a");       // 1
+                    TreeItem<String> a1 = new TreeItem<>("a1"); // a expanded = 2, a collapsed = -1
+            TreeItem<String> b = new TreeItem<>("b");           // a expanded = 3, a collapsed = 2
+                TreeItem<String> b1 = new TreeItem<>("b1");     // a expanded = 4, a collapsed = 3
+                TreeItem<String> b2 = new TreeItem<>("b2");     // a expanded = 5, a collapsed = 4
+
+        root.setExpanded(true);
+        root.getChildren().setAll(a, b);
+
+        a.setExpanded(false);
+        if (!addChildToAAfterSelection) {
+            a.getChildren().add(a1);
+        }
+
+        b.setExpanded(true);
+        b.getChildren().addAll(b1, b2);
+
+        final TreeTableView<String> treeView = new TreeTableView<>(root);
+        TreeTableColumn<String, String> tableColumn = new TreeTableColumn<>();
+        tableColumn.setCellValueFactory(rowValue -> new SimpleStringProperty(rowValue.getValue().getValue()));
+        treeView.getColumns().add(tableColumn);
+
+        TreeTableView.TreeTableViewSelectionModel<String> sm = treeView.getSelectionModel();
+        FocusModel<TreeItem<String>> fm = treeView.getFocusModel();
+
+        sm.select(b1);
+        assertEquals(3, sm.getSelectedIndex());
+        assertEquals(b1, sm.getSelectedItem());
+        assertEquals(3, fm.getFocusedIndex());
+        assertEquals(b1, fm.getFocusedItem());
+
+        if (addChildToAAfterSelection) {
+            a.getChildren().add(a1);
+        }
+
+        a.setExpanded(true);
+        assertEquals(4, sm.getSelectedIndex());
+        assertEquals(b1, sm.getSelectedItem());
+        assertEquals(4, fm.getFocusedIndex());
+        assertEquals(b1, fm.getFocusedItem());
+    }
 }
--- a/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Tue Apr 29 10:13:39 2014 +1200
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Tue Apr 29 12:13:17 2014 +1200
@@ -1749,4 +1749,57 @@
 
         sl.dispose();
     }
+
+    @Test public void test_rt36885_addChildBeforeSelection() {
+        test_rt36885(false);
+    }
+
+    @Test public void test_rt36885_addChildAfterSelection() {
+        test_rt36885(true);
+    }
+
+    private void test_rt36885(boolean addChildToAAfterSelection) {
+        TreeItem<String> root = new TreeItem<>("Root");     // 0
+            TreeItem<String> a = new TreeItem<>("a");       // 1
+                TreeItem<String> a1 = new TreeItem<>("a1"); // a expanded = 2, a collapsed = -1
+            TreeItem<String> b = new TreeItem<>("b");       // a expanded = 3, a collapsed = 2
+                TreeItem<String> b1 = new TreeItem<>("b1"); // a expanded = 4, a collapsed = 3
+                TreeItem<String> b2 = new TreeItem<>("b2"); // a expanded = 5, a collapsed = 4
+
+        root.setExpanded(true);
+        root.getChildren().setAll(a, b);
+
+        a.setExpanded(false);
+        if (!addChildToAAfterSelection) {
+            a.getChildren().add(a1);
+        }
+
+        b.setExpanded(true);
+        b.getChildren().addAll(b1, b2);
+
+        final TreeView<String> treeView = new TreeView<String>(root);
+
+        treeView.getFocusModel().focusedIndexProperty().addListener((observable, oldValue, newValue) -> {
+            System.out.println("focusedIndex: " + oldValue + " to " + newValue);
+        });
+
+        MultipleSelectionModel<TreeItem<String>> sm = treeView.getSelectionModel();
+        FocusModel<TreeItem<String>> fm = treeView.getFocusModel();
+
+        sm.select(b1);
+        assertEquals(3, sm.getSelectedIndex());
+        assertEquals(b1, sm.getSelectedItem());
+        assertEquals(3, fm.getFocusedIndex());
+        assertEquals(b1, fm.getFocusedItem());
+
+        if (addChildToAAfterSelection) {
+            a.getChildren().add(a1);
+        }
+
+        a.setExpanded(true);
+        assertEquals(4, sm.getSelectedIndex());
+        assertEquals(b1, sm.getSelectedItem());
+        assertEquals(4, fm.getFocusedIndex());
+        assertEquals(b1, fm.getFocusedItem());
+    }
 }