changeset 5825:3c2b98a1509d

RT-26721: [TreeView] focus don't jump up in some cases of nodes collapsing.
author jgiles
date Wed, 27 Nov 2013 14:40:28 +1300
parents 707c8ea0aea2
children 85ada295c274
files modules/controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java 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 5 files changed, 250 insertions(+), 4 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java	Wed Nov 27 14:00:40 2013 +1300
+++ b/modules/controls/src/main/java/javafx/scene/control/MultipleSelectionModelBase.java	Wed Nov 27 14:40:28 2013 +1300
@@ -634,7 +634,14 @@
     }
 
     @Override public boolean isSelected(int index) {
-        if (index >= 0 && index < getItemCount()) {
+        // Note the change in semantics here - we used to check to ensure that
+        // the index is less than the item count, but now simply ensure that
+        // it is less than the length of the selectedIndices bitset. This helps
+        // to resolve issues such as RT-26721, where isSelected(int) was being
+        // called for indices that exceeded the item count, as a TreeItem (e.g.
+        // the root) was being collapsed.
+//        if (index >= 0 && index < getItemCount()) {
+        if (index >= 0 && index < selectedIndices.length()) {
             return selectedIndices.get(index);
         }
 
--- a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Wed Nov 27 14:00:40 2013 +1300
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Wed Nov 27 14:40:28 2013 +1300
@@ -2184,14 +2184,14 @@
                                 final TreeTableColumn<S,?> col = columns.get(column);
                                 if (isSelected(i, col)) {
                                     wasAnyChildSelected = true;
+                                    clearSelection(i, col);
                                 }
-                                clearSelection(i, col);
                             }
                         } else {
                             if (isSelected(i)) {
                                 wasAnyChildSelected = true;
+                                clearSelection(i);
                             }
-                            clearSelection(i);
                         }
                     }
 
--- a/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Wed Nov 27 14:00:40 2013 +1300
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Wed Nov 27 14:40:28 2013 +1300
@@ -1199,8 +1199,8 @@
                     for (int i = startRow + 1; i < startRow + count; i++) {
                         if (isSelected(i)) {
                             wasAnyChildSelected = true;
+                            clearSelection(i);
                         }
-                        clearSelection(i);
                     }
 
                     // put selection onto the newly-collapsed tree item
--- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Wed Nov 27 14:00:40 2013 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Wed Nov 27 14:40:28 2013 +1300
@@ -2786,4 +2786,125 @@
         // other value (based on the width of the content in that column)
         assertEquals(400, last.getWidth(), 0.0);
     }
+
+    @Test public void test_rt26721_collapseParent_firstRootChild() {
+        TreeTableView<String> table = new TreeTableView<>();
+        table.setRoot(new TreeItem("Root"));
+        table.getRoot().setExpanded(true);
+
+        for (int i = 0; i < 4; i++) {
+            TreeItem parent = new TreeItem("item - " + i);
+            table.getRoot().getChildren().add(parent);
+
+            for (int j = 0; j < 4; j++) {
+                TreeItem child = new TreeItem("item - " + i + " " + j);
+                parent.getChildren().add(child);
+            }
+        }
+
+        table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
+        final TreeItem<String> item0 = table.getTreeItem(1);
+        final TreeItem<String> item0child0 = item0.getChildren().get(0);
+        final TreeItem<String> item1 = table.getTreeItem(2);
+
+        assertEquals("item - 0", item0.getValue());
+        assertEquals("item - 1", item1.getValue());
+
+        item0.setExpanded(true);
+        item1.setExpanded(true);
+        Toolkit.getToolkit().firePulse();
+
+        // select the first child of item0
+        table.getSelectionModel().select(item0child0);
+
+        assertEquals(item0child0, table.getSelectionModel().getSelectedItem());
+        assertEquals(item0child0, table.getFocusModel().getFocusedItem());
+
+        // collapse item0 - we expect the selection / focus to move up to item0
+        item0.setExpanded(false);
+        Toolkit.getToolkit().firePulse();
+        assertEquals(item0, table.getSelectionModel().getSelectedItem());
+        assertEquals(item0, table.getFocusModel().getFocusedItem());
+    }
+
+    @Test public void test_rt26721_collapseParent_lastRootChild() {
+        TreeTableView<String> table = new TreeTableView<>();
+        table.setRoot(new TreeItem("Root"));
+        table.getRoot().setExpanded(true);
+
+        for (int i = 0; i < 4; i++) {
+            TreeItem parent = new TreeItem("item - " + i);
+            table.getRoot().getChildren().add(parent);
+
+            for (int j = 0; j < 4; j++) {
+                TreeItem child = new TreeItem("item - " + i + " " + j);
+                parent.getChildren().add(child);
+            }
+        }
+
+        table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
+        final TreeItem<String> item3 = table.getTreeItem(4);
+        final TreeItem<String> item3child0 = item3.getChildren().get(0);
+
+        assertEquals("item - 3", item3.getValue());
+        assertEquals("item - 3 0", item3child0.getValue());
+
+        item3.setExpanded(true);
+        Toolkit.getToolkit().firePulse();
+
+        // select the first child of item0
+        table.getSelectionModel().select(item3child0);
+
+        assertEquals(item3child0, table.getSelectionModel().getSelectedItem());
+        assertEquals(item3child0, table.getFocusModel().getFocusedItem());
+
+        // collapse item3 - we expect the selection / focus to move up to item3
+        item3.setExpanded(false);
+        Toolkit.getToolkit().firePulse();
+        assertEquals(item3, table.getSelectionModel().getSelectedItem());
+        assertEquals(item3, table.getFocusModel().getFocusedItem());
+    }
+
+    @Test public void test_rt26721_collapseGrandParent() {
+        TreeTableView<String> table = new TreeTableView<>();
+        table.setRoot(new TreeItem("Root"));
+        table.getRoot().setExpanded(true);
+
+        for (int i = 0; i < 4; i++) {
+            TreeItem parent = new TreeItem("item - " + i);
+            table.getRoot().getChildren().add(parent);
+
+            for (int j = 0; j < 4; j++) {
+                TreeItem child = new TreeItem("item - " + i + " " + j);
+                parent.getChildren().add(child);
+            }
+        }
+
+        table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
+        final TreeItem<String> item0 = table.getTreeItem(1);
+        final TreeItem<String> item0child0 = item0.getChildren().get(0);
+        final TreeItem<String> item1 = table.getTreeItem(2);
+
+        assertEquals("item - 0", item0.getValue());
+        assertEquals("item - 1", item1.getValue());
+
+        item0.setExpanded(true);
+        item1.setExpanded(true);
+        Toolkit.getToolkit().firePulse();
+
+        // select the first child of item0
+        table.getSelectionModel().select(item0child0);
+
+        assertEquals(item0child0, table.getSelectionModel().getSelectedItem());
+        assertEquals(item0child0, table.getFocusModel().getFocusedItem());
+
+        // collapse root - we expect the selection / focus to move up to root
+        table.getRoot().setExpanded(false);
+        Toolkit.getToolkit().firePulse();
+        assertEquals(table.getRoot(), table.getSelectionModel().getSelectedItem());
+        assertEquals(table.getRoot(), table.getFocusModel().getFocusedItem());
+    }
 }
--- a/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Wed Nov 27 14:00:40 2013 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Wed Nov 27 14:40:28 2013 +1300
@@ -1288,4 +1288,122 @@
         assertEquals(3, treeView.getSelectionModel().getSelectedIndex());
         assertEquals(3, treeView.getFocusModel().getFocusedIndex());
     }
+
+    @Test public void test_rt26721_collapseParent_firstRootChild() {
+        treeView.setRoot(new TreeItem("Root"));
+        treeView.getRoot().setExpanded(true);
+
+        for (int i = 0; i < 4; i++) {
+            TreeItem parent = new TreeItem("item - " + i);
+            treeView.getRoot().getChildren().add(parent);
+
+            for (int j = 0; j < 4; j++) {
+                TreeItem child = new TreeItem("item - " + i + " " + j);
+                parent.getChildren().add(child);
+            }
+        }
+
+        treeView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
+        final TreeItem<String> item0 = treeView.getTreeItem(1);
+        final TreeItem<String> item0child0 = item0.getChildren().get(0);
+        final TreeItem<String> item1 = treeView.getTreeItem(2);
+
+        assertEquals("item - 0", item0.getValue());
+        assertEquals("item - 1", item1.getValue());
+
+        item0.setExpanded(true);
+        item1.setExpanded(true);
+        Toolkit.getToolkit().firePulse();
+
+        // select the first child of item0
+        treeView.getSelectionModel().select(item0child0);
+
+        assertEquals(item0child0, treeView.getSelectionModel().getSelectedItem());
+        assertEquals(item0child0, treeView.getFocusModel().getFocusedItem());
+
+        // collapse item0 - we expect the selection / focus to move up to item0
+        item0.setExpanded(false);
+        Toolkit.getToolkit().firePulse();
+        assertEquals(item0, treeView.getSelectionModel().getSelectedItem());
+        assertEquals(item0, treeView.getFocusModel().getFocusedItem());
+    }
+
+    @Test public void test_rt26721_collapseParent_lastRootChild() {
+        treeView.setRoot(new TreeItem("Root"));
+        treeView.getRoot().setExpanded(true);
+
+        for (int i = 0; i < 4; i++) {
+            TreeItem parent = new TreeItem("item - " + i);
+            treeView.getRoot().getChildren().add(parent);
+
+            for (int j = 0; j < 4; j++) {
+                TreeItem child = new TreeItem("item - " + i + " " + j);
+                parent.getChildren().add(child);
+            }
+        }
+
+        treeView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
+        final TreeItem<String> item3 = treeView.getTreeItem(4);
+        final TreeItem<String> item3child0 = item3.getChildren().get(0);
+
+        assertEquals("item - 3", item3.getValue());
+        assertEquals("item - 3 0", item3child0.getValue());
+
+        item3.setExpanded(true);
+        Toolkit.getToolkit().firePulse();
+
+        // select the first child of item0
+        treeView.getSelectionModel().select(item3child0);
+
+        assertEquals(item3child0, treeView.getSelectionModel().getSelectedItem());
+        assertEquals(item3child0, treeView.getFocusModel().getFocusedItem());
+
+        // collapse item3 - we expect the selection / focus to move up to item3
+        item3.setExpanded(false);
+        Toolkit.getToolkit().firePulse();
+        assertEquals(item3, treeView.getSelectionModel().getSelectedItem());
+        assertEquals(item3, treeView.getFocusModel().getFocusedItem());
+    }
+
+    @Test public void test_rt26721_collapseGrandParent() {
+        treeView.setRoot(new TreeItem("Root"));
+        treeView.getRoot().setExpanded(true);
+
+        for (int i = 0; i < 4; i++) {
+            TreeItem parent = new TreeItem("item - " + i);
+            treeView.getRoot().getChildren().add(parent);
+
+            for (int j = 0; j < 4; j++) {
+                TreeItem child = new TreeItem("item - " + i + " " + j);
+                parent.getChildren().add(child);
+            }
+        }
+
+        treeView.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
+        final TreeItem<String> item0 = treeView.getTreeItem(1);
+        final TreeItem<String> item0child0 = item0.getChildren().get(0);
+        final TreeItem<String> item1 = treeView.getTreeItem(2);
+
+        assertEquals("item - 0", item0.getValue());
+        assertEquals("item - 1", item1.getValue());
+
+        item0.setExpanded(true);
+        item1.setExpanded(true);
+        Toolkit.getToolkit().firePulse();
+
+        // select the first child of item0
+        treeView.getSelectionModel().select(item0child0);
+
+        assertEquals(item0child0, treeView.getSelectionModel().getSelectedItem());
+        assertEquals(item0child0, treeView.getFocusModel().getFocusedItem());
+
+        // collapse root - we expect the selection / focus to move up to root
+        treeView.getRoot().setExpanded(false);
+        Toolkit.getToolkit().firePulse();
+        assertEquals(treeView.getRoot(), treeView.getSelectionModel().getSelectedItem());
+        assertEquals(treeView.getRoot(), treeView.getFocusModel().getFocusedItem());
+    }
 }