changeset 6279:37953435bd98 8u20-b01

RT-23245: TreeView moves its origin position if the root item has a parent
author jgiles
date Mon, 10 Feb 2014 13:59:51 +1300
parents 419eb535475a
children e8caf245c8f7 91c0001a39b0
files modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeCellSkin.java modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableCellSkin.java modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.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 7 files changed, 281 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeCellSkin.java	Mon Feb 10 13:53:32 2014 +1300
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeCellSkin.java	Mon Feb 10 13:59:51 2014 +1300
@@ -215,7 +215,7 @@
         
         Node disclosureNode = getSkinnable().getDisclosureNode();
         
-        int level = TreeView.getNodeLevel(treeItem);
+        int level = tree.getTreeItemLevel(treeItem);
         if (! tree.isShowRoot()) level--;
         double leftMargin = getIndent() * level;
 
@@ -303,7 +303,7 @@
         pw = labelWidth;
 
         // determine the amount of indentation
-        int level = TreeView.getNodeLevel(treeItem);
+        int level = tree.getTreeItemLevel(treeItem);
         if (! tree.isShowRoot()) level--;
         pw += getIndent() * level;
 
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableCellSkin.java	Mon Feb 10 13:53:32 2014 +1300
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableCellSkin.java	Mon Feb 10 13:59:51 2014 +1300
@@ -91,7 +91,7 @@
         TreeItem<S> treeItem = treeTableRow.getTreeItem();
         if (treeItem == null) return leftPadding;
         
-        int nodeLevel = TreeTableView.getNodeLevel(treeItem);
+        int nodeLevel = treeTable.getTreeItemLevel(treeItem);
         if (! treeTable.isShowRoot()) nodeLevel--;
 
         double indentPerLevel = 10;
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java	Mon Feb 10 13:53:32 2014 +1300
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java	Mon Feb 10 13:59:51 2014 +1300
@@ -231,7 +231,7 @@
     }
 
     @Override protected int getIndentationLevel(TreeTableRow<T> control) {
-        return TreeTableView.getNodeLevel(control.getTreeItem());
+        return control.getTreeTableView().getTreeItemLevel(control.getTreeItem());
     }
 
     @Override protected double getIndentationPerLevel() {
--- a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Mon Feb 10 13:53:32 2014 +1300
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java	Mon Feb 10 13:59:51 2014 +1300
@@ -463,18 +463,29 @@
     }
     private static final EventType<?> EDIT_COMMIT_EVENT =
             new EventType(editAnyEvent(), "EDIT_COMMIT");
-    
+
     /**
      * Returns the number of levels of 'indentation' of the given TreeItem, 
-     * based on how many times getParent() can be recursively called. If the 
-     * given TreeItem is the root node, or if the TreeItem does not have any 
-     * parent set, the returned value will be zero. For each time getParent() is 
-     * recursively called, the returned value is incremented by one.
-     * 
+     * based on how many times {@link javafx.scene.control.TreeItem#getParent()}
+     * can be recursively called. If the TreeItem does not have any parent set,
+     * the returned value will be zero. For each time getParent() is recursively
+     * called, the returned value is incremented by one.
+     *
+     * <p><strong>Important note: </strong>This method is deprecated as it does
+     * not consider the root node. This means that this method will iterate
+     * past the root node of the TreeTableView control, if the root node has a parent.
+     * If this is important, call {@link TreeTableView#getTreeItemLevel(TreeItem)}
+     * instead.
+     *
      * @param node The TreeItem for which the level is needed.
      * @return An integer representing the number of parents above the given node,
      *         or -1 if the given TreeItem is null.
+     * @deprecated This method does not correctly calculate the distance from the
+     *          given TreeItem to the root of the TreeTableView. As of JavaFX 8.0_20,
+     *          the proper way to do this is via
+     *          {@link TreeTableView#getTreeItemLevel(TreeItem)}
      */
+    @Deprecated
     public static int getNodeLevel(TreeItem<?> node) {
         return TreeView.getNodeLevel(node);
     }
@@ -1496,6 +1507,39 @@
         treeItemCacheMap.put(_row, new SoftReference<>(treeItem));
         return treeItem;
     }
+
+    /**
+     * Returns the number of levels of 'indentation' of the given TreeItem,
+     * based on how many times getParent() can be recursively called. If the
+     * given TreeItem is the root node of this TreeTableView, or if the TreeItem
+     * does not have any parent set, the returned value will be zero. For each
+     * time getParent() is recursively called, the returned value is incremented
+     * by one.
+     *
+     * @param node The TreeItem for which the level is needed.
+     * @return An integer representing the number of parents above the given node,
+     *         or -1 if the given TreeItem is null.
+     */
+    public int getTreeItemLevel(TreeItem<?> node) {
+        final TreeItem<?> root = getRoot();
+
+        if (node == null) return -1;
+        if (node == root) return 0;
+
+        int level = 0;
+        TreeItem<?> parent = node.getParent();
+        while (parent != null) {
+            level++;
+
+            if (parent == root) {
+                break;
+            }
+
+            parent = parent.getParent();
+        }
+
+        return level;
+    }
     
     /**
      * The TreeTableColumns that are part of this TableView. As the user reorders
--- a/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Mon Feb 10 13:53:32 2014 +1300
+++ b/modules/controls/src/main/java/javafx/scene/control/TreeView.java	Mon Feb 10 13:59:51 2014 +1300
@@ -254,15 +254,26 @@
     
     /**
      * Returns the number of levels of 'indentation' of the given TreeItem, 
-     * based on how many times getParent() can be recursively called. If the 
-     * given TreeItem is the root node, or if the TreeItem does not have any 
-     * parent set, the returned value will be zero. For each time getParent() is 
-     * recursively called, the returned value is incremented by one.
+     * based on how many times {@link javafx.scene.control.TreeItem#getParent()}
+     * can be recursively called. If the TreeItem does not have any parent set,
+     * the returned value will be zero. For each time getParent() is recursively
+     * called, the returned value is incremented by one.
+     *
+     * <p><strong>Important note: </strong>This method is deprecated as it does
+     * not consider the root node. This means that this method will iterate
+     * past the root node of the TreeView control, if the root node has a parent.
+     * If this is important, call {@link TreeView#getTreeItemLevel(TreeItem)}
+     * instead.
      * 
      * @param node The TreeItem for which the level is needed.
      * @return An integer representing the number of parents above the given node,
-     *         or -1 if the given TreeItem is null.
+     *          or -1 if the given TreeItem is null.
+     * @deprecated This method does not correctly calculate the distance from the
+     *          given TreeItem to the root of the TreeView. As of JavaFX 8.0_20,
+     *          the proper way to do this is via
+     *          {@link TreeView#getTreeItemLevel(TreeItem)}
      */
+    @Deprecated
     public static int getNodeLevel(TreeItem<?> node) {
         if (node == null) return -1;
 
@@ -276,7 +287,7 @@
         return level;
     }
 
-    
+
     /***************************************************************************
      *                                                                         *
      * Constructors                                                            *
@@ -951,6 +962,38 @@
         return treeItem;
     }
 
+    /**
+     * Returns the number of levels of 'indentation' of the given TreeItem,
+     * based on how many times getParent() can be recursively called. If the
+     * given TreeItem is the root node of this TreeView, or if the TreeItem does
+     * not have any parent set, the returned value will be zero. For each time
+     * getParent() is recursively called, the returned value is incremented by one.
+     *
+     * @param node The TreeItem for which the level is needed.
+     * @return An integer representing the number of parents above the given node,
+     *         or -1 if the given TreeItem is null.
+     */
+    public int getTreeItemLevel(TreeItem<?> node) {
+        final TreeItem<?> root = getRoot();
+
+        if (node == null) return -1;
+        if (node == root) return 0;
+
+        int level = 0;
+        TreeItem<?> parent = node.getParent();
+        while (parent != null) {
+            level++;
+
+            if (parent == root) {
+                break;
+            }
+
+            parent = parent.getParent();
+        }
+
+        return level;
+    }
+
     /** {@inheritDoc} */
     @Override protected Skin<?> createDefaultSkin() {
         return new TreeViewSkin<T>(this);
--- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Mon Feb 10 13:53:32 2014 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java	Mon Feb 10 13:59:51 2014 +1300
@@ -3113,6 +3113,95 @@
         Toolkit.getToolkit().firePulse();
     }
 
+    @Test public void test_rt23245_itemIsInTree() {
+        final TreeTableView<String> view = new TreeTableView<String>();
+        final List<TreeItem<String>> items = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            final TreeItem<String> item = new TreeItem<String>("Item" + i);
+            item.setExpanded(true);
+            items.add(item);
+        }
+
+        // link the items up so that the next item is the child of the current item
+        for (int i = 0; i < 9; i++) {
+            items.get(i).getChildren().add(items.get(i + 1));
+        }
+
+        view.setRoot(items.get(0));
+
+        for (int i = 0; i < 10; i++) {
+            // we expect the level of the tree item at the ith position to be
+            // 0, as every iteration we are setting the ith item as the root.
+            assertEquals(0, view.getTreeItemLevel(items.get(i)));
+
+            // whilst we are testing, we should also ensure that the ith item
+            // is indeed the root item, and that the ith item is indeed the item
+            // at the 0th position
+            assertEquals(items.get(i), view.getRoot());
+            assertEquals(items.get(i), view.getTreeItem(0));
+
+            // shuffle the next item into the root position (keeping its parent
+            // chain intact - which is what exposes this issue in the first place).
+            if (i < 9) {
+                view.setRoot(items.get(i + 1));
+            }
+        }
+    }
+
+    @Test public void test_rt23245_itemIsNotInTree_noRootNode() {
+        final TreeView<String> view = new TreeView<String>();
+        final List<TreeItem<String>> items = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            final TreeItem<String> item = new TreeItem<String>("Item" + i);
+            item.setExpanded(true);
+            items.add(item);
+        }
+
+        // link the items up so that the next item is the child of the current item
+        for (int i = 0; i < 9; i++) {
+            items.get(i).getChildren().add(items.get(i + 1));
+        }
+
+        for (int i = 0; i < 10; i++) {
+            // because we have no root (and we are not changing the root like
+            // the previous test), we expect the tree item level of the item
+            // in the ith position to be i.
+            assertEquals(i, view.getTreeItemLevel(items.get(i)));
+
+            // all items requested from the TreeView should be null, as the
+            // TreeView does not have a root item
+            assertNull(view.getTreeItem(i));
+        }
+    }
+
+    @Test public void test_rt23245_itemIsNotInTree_withUnrelatedRootNode() {
+        final TreeView<String> view = new TreeView<String>();
+        final List<TreeItem<String>> items = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            final TreeItem<String> item = new TreeItem<String>("Item" + i);
+            item.setExpanded(true);
+            items.add(item);
+        }
+
+        // link the items up so that the next item is the child of the current item
+        for (int i = 0; i < 9; i++) {
+            items.get(i).getChildren().add(items.get(i + 1));
+        }
+
+        view.setRoot(new TreeItem("Unrelated root node"));
+
+        for (int i = 0; i < 10; i++) {
+            // because we have no root (and we are not changing the root like
+            // the previous test), we expect the tree item level of the item
+            // in the ith position to be i.
+            assertEquals(i, view.getTreeItemLevel(items.get(i)));
+
+            // all items requested from the TreeView should be null except for
+            // the root node
+            assertNull(view.getTreeItem(i + 1));
+        }
+    }
+
     @Test public void test_rt35039_setRoot() {
         TreeItem<String> root = new TreeItem<>("Root");
         root.setExpanded(true);
--- a/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Mon Feb 10 13:53:32 2014 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java	Mon Feb 10 13:59:51 2014 +1300
@@ -1509,6 +1509,95 @@
         Toolkit.getToolkit().firePulse();
     }
 
+    @Test public void test_rt23245_itemIsInTree() {
+        final TreeView<String> view = new TreeView<String>();
+        final List<TreeItem<String>> items = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            final TreeItem<String> item = new TreeItem<String>("Item" + i);
+            item.setExpanded(true);
+            items.add(item);
+        }
+
+        // link the items up so that the next item is the child of the current item
+        for (int i = 0; i < 9; i++) {
+            items.get(i).getChildren().add(items.get(i + 1));
+        }
+
+        view.setRoot(items.get(0));
+
+        for (int i = 0; i < 10; i++) {
+            // we expect the level of the tree item at the ith position to be
+            // 0, as every iteration we are setting the ith item as the root.
+            assertEquals(0, view.getTreeItemLevel(items.get(i)));
+
+            // whilst we are testing, we should also ensure that the ith item
+            // is indeed the root item, and that the ith item is indeed the item
+            // at the 0th position
+            assertEquals(items.get(i), view.getRoot());
+            assertEquals(items.get(i), view.getTreeItem(0));
+
+            // shuffle the next item into the root position (keeping its parent
+            // chain intact - which is what exposes this issue in the first place).
+            if (i < 9) {
+                view.setRoot(items.get(i + 1));
+            }
+        }
+    }
+
+    @Test public void test_rt23245_itemIsNotInTree_noRootNode() {
+        final TreeView<String> view = new TreeView<String>();
+        final List<TreeItem<String>> items = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            final TreeItem<String> item = new TreeItem<String>("Item" + i);
+            item.setExpanded(true);
+            items.add(item);
+        }
+
+        // link the items up so that the next item is the child of the current item
+        for (int i = 0; i < 9; i++) {
+            items.get(i).getChildren().add(items.get(i + 1));
+        }
+
+        for (int i = 0; i < 10; i++) {
+            // because we have no root (and we are not changing the root like
+            // the previous test), we expect the tree item level of the item
+            // in the ith position to be i.
+            assertEquals(i, view.getTreeItemLevel(items.get(i)));
+
+            // all items requested from the TreeView should be null, as the
+            // TreeView does not have a root item
+            assertNull(view.getTreeItem(i));
+        }
+    }
+
+    @Test public void test_rt23245_itemIsNotInTree_withUnrelatedRootNode() {
+        final TreeView<String> view = new TreeView<String>();
+        final List<TreeItem<String>> items = new ArrayList<>();
+        for (int i = 0; i < 10; i++) {
+            final TreeItem<String> item = new TreeItem<String>("Item" + i);
+            item.setExpanded(true);
+            items.add(item);
+        }
+
+        // link the items up so that the next item is the child of the current item
+        for (int i = 0; i < 9; i++) {
+            items.get(i).getChildren().add(items.get(i + 1));
+        }
+
+        view.setRoot(new TreeItem("Unrelated root node"));
+
+        for (int i = 0; i < 10; i++) {
+            // because we have no root (and we are not changing the root like
+            // the previous test), we expect the tree item level of the item
+            // in the ith position to be i.
+            assertEquals(i, view.getTreeItemLevel(items.get(i)));
+
+            // all items requested from the TreeView should be null except for
+            // the root node
+            assertNull(view.getTreeItem(i + 1));
+        }
+    }
+
     @Test public void test_rt35039_setRoot() {
         TreeItem<String> root = new TreeItem<>("Root");
         root.setExpanded(true);