changeset 5033:9d21ed934fd0

RT-32963: [TreeTableView] Weird selection issue with root TreeItem Reviewed-by: psomashe
author jgiles
date Tue, 17 Sep 2013 10:21:34 +1200
parents f6155a8057f3
children 6e2462521fe1
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/TreeTableViewMouseInputTest.java modules/controls/src/test/java/javafx/scene/control/TreeViewMouseInputTest.java
diffstat 4 files changed, 76 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Mon Sep 16 16:57:27 2013 +1200
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Tue Sep 17 10:21:34 2013 +1200
@@ -2167,6 +2167,14 @@
                 } else if (e.wasAdded()) {
                     // shuffle selection by the number of added items
                     shift = treeItem.isExpanded() ? e.getAddedSize() : 0;
+
+                    // RT-32963: We were taking the startRow from the TreeItem
+                    // in which the children were added, rather than from the
+                    // actual position of the new child. This led to selection
+                    // being moved off the parent TreeItem by mistake.
+                    if (e.getAddedSize() == 1) {
+                        startRow = treeTableView.getRow(e.getAddedChildren().get(0));
+                    }
                 } else if (e.wasRemoved()) {
                     // shuffle selection by the number of removed items
                     shift = treeItem.isExpanded() ? -e.getRemovedSize() : 0;
--- a/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Mon Sep 16 16:57:27 2013 +1200
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Tue Sep 17 10:21:34 2013 +1200
@@ -1118,6 +1118,14 @@
                 } else if (e.wasAdded()) {
                     // shuffle selection by the number of added items
                     shift = treeItem.isExpanded() ? e.getAddedSize() : 0;
+
+                    // RT-32963: We were taking the startRow from the TreeItem
+                    // in which the children were added, rather than from the
+                    // actual position of the new child. This led to selection
+                    // being moved off the parent TreeItem by mistake.
+                    if (e.getAddedSize() == 1) {
+                        startRow = treeView.getRow(e.getAddedChildren().get(0));
+                    }
                 } else if (e.wasRemoved()) {
                     // shuffle selection by the number of removed items
                     shift = treeItem.isExpanded() ? -e.getRemovedSize() : 0;
--- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewMouseInputTest.java	Mon Sep 16 16:57:27 2013 +1200
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewMouseInputTest.java	Tue Sep 17 10:21:34 2013 +1200
@@ -335,7 +335,7 @@
         VirtualFlowTestUtils.clickOnRow(tableView, selectRow - 1, KeyModifier.SHIFT);
         assertEquals(2, sm.getSelectedItems().size());
         assertEquals("Row 2", sm.getSelectedItem().getValue());
-        assertEquals("Row 2", sm.getSelectedItems().get(1).getValue());
+        assertEquals("Row 2", sm.getSelectedItems().get(0).getValue());
     }
 
     @Test public void test_rt21444_down_cell() {
@@ -379,7 +379,7 @@
         VirtualFlowTestUtils.clickOnRow(tableView, selectRow - 1, true, KeyModifier.SHIFT);
         assertEquals(2, sm.getSelectedItems().size());
         assertEquals("Row 2", sm.getSelectedItem().getValue());
-        assertEquals("Row 2", sm.getSelectedItems().get(1).getValue());
+        assertEquals("Row 2", sm.getSelectedItems().get(0).getValue());
     }
 
     @Test public void test_rt21444_down_row() {
@@ -402,4 +402,33 @@
         VirtualFlowTestUtils.clickOnRow(tableView, selectRow + 1, true, KeyModifier.SHIFT);
         assertEquals("Row 4", sm.getSelectedItem().getValue());
     }
+
+    @Test public void test_rt_32963() {
+        final int items = 8;
+        root.getChildren().clear();
+        root.setExpanded(true);
+        for (int i = 0; i < items; i++) {
+            root.getChildren().add(new TreeItem<>("Row " + i));
+        }
+        tableView.setRoot(root);
+
+        tableView.setShowRoot(true);
+        final MultipleSelectionModel sm = tableView.getSelectionModel();
+        sm.setSelectionMode(SelectionMode.MULTIPLE);
+        sm.clearAndSelect(0);
+
+        assertEquals(9, tableView.getExpandedItemCount());
+
+        assertEquals(0, sm.getSelectedIndex());
+        assertEquals(0, fm.getFocusedIndex());
+        assertEquals(root, sm.getSelectedItem());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        VirtualFlowTestUtils.clickOnRow(tableView, 5, KeyModifier.SHIFT);
+        assertEquals("Actual selected index: " + sm.getSelectedIndex(), 5, sm.getSelectedIndex());
+        assertEquals("Actual focused index: " + fm.getFocusedIndex(), 5, fm.getFocusedIndex());
+        assertTrue("Selected indices: " + sm.getSelectedIndices(), sm.getSelectedIndices().contains(0));
+        assertTrue("Selected items: " + sm.getSelectedItems(), sm.getSelectedItems().contains(root));
+        assertEquals(6, sm.getSelectedItems().size());
+    }
 }
--- a/modules/controls/src/test/java/javafx/scene/control/TreeViewMouseInputTest.java	Mon Sep 16 16:57:27 2013 +1200
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewMouseInputTest.java	Tue Sep 17 10:21:34 2013 +1200
@@ -312,4 +312,33 @@
         assertEquals("Row 4", sm.getSelectedItem().getValue());
         assertEquals("Row 4", sm.getSelectedItems().get(1).getValue());
     }
+
+    @Test public void test_rt_32963() {
+        final int items = 8;
+        root.getChildren().clear();
+        root.setExpanded(true);
+        for (int i = 0; i < items; i++) {
+            root.getChildren().add(new TreeItem<>("Row " + i));
+        }
+        treeView.setRoot(root);
+
+        treeView.setShowRoot(true);
+        final MultipleSelectionModel sm = treeView.getSelectionModel();
+        sm.setSelectionMode(SelectionMode.MULTIPLE);
+        sm.clearAndSelect(0);
+
+        assertEquals(9, treeView.getExpandedItemCount());
+
+        assertEquals(0, sm.getSelectedIndex());
+        assertEquals(0, fm.getFocusedIndex());
+        assertEquals(root, sm.getSelectedItem());
+        assertEquals(1, sm.getSelectedItems().size());
+
+        VirtualFlowTestUtils.clickOnRow(treeView, 5, KeyModifier.SHIFT);
+        assertEquals("Actual selected index: " + sm.getSelectedIndex(), 5, sm.getSelectedIndex());
+        assertEquals("Actual focused index: " + fm.getFocusedIndex(), 5, fm.getFocusedIndex());
+        assertTrue("Selected indices: " + sm.getSelectedIndices(), sm.getSelectedIndices().contains(0));
+        assertTrue("Selected items: " + sm.getSelectedItems(), sm.getSelectedItems().contains(root));
+        assertEquals(6, sm.getSelectedItems().size());
+    }
 }