Revision: 8348
Author: jlaba...@google.com
Date: Fri Jul 2 05:43:30 2010
Log: Inserting TreeItems into the logical list in the same spot that they
are inserted into the physical DOM. This fixes a bug where keyboard
navigation occurs out of order, and generally makes the logical and
physical structures consistent.
Review at http://gwt-code-reviews.appspot.com/645802
Review by: r...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=8348
Modified:
/trunk/user/src/com/google/gwt/user/client/ui/Tree.java
/trunk/user/src/com/google/gwt/user/client/ui/TreeItem.java
/trunk/user/test/com/google/gwt/user/client/ui/TreeItemTest.java
/trunk/user/test/com/google/gwt/user/client/ui/TreeTest.java
=======================================
--- /trunk/user/src/com/google/gwt/user/client/ui/Tree.java Mon Jun 7
10:39:00 2010
+++ /trunk/user/src/com/google/gwt/user/client/ui/Tree.java Fri Jul 2
05:43:30 2010
@@ -999,60 +999,7 @@
// The 'root' item is invisible and serves only as a container
// for all top-level items.
- root = new TreeItem() {
- @Override
- public void insertItem(int beforeIndex, TreeItem item)
- throws IndexOutOfBoundsException {
- // Check the index.
- int childCount = getChildCount();
- if (beforeIndex < 0 || beforeIndex > childCount) {
- throw new IndexOutOfBoundsException();
- }
-
- // If this element already belongs to a tree or tree item, remove
it.
- if ((item.getParentItem() != null) || (item.getTree() != null)) {
- item.remove();
- }
-
- // Physical attach.
- Element treeElem = Tree.this.getElement();
- if (beforeIndex == childCount) {
- treeElem.appendChild(item.getElement());
- } else {
- Element beforeElem = getChild(beforeIndex).getElement();
- treeElem.insertBefore(item.getElement(), beforeElem);
- }
-
- // Logical attach.
- item.setTree(this.getTree());
-
- // Explicitly set top-level items' parents to null.
- item.setParentItem(null);
- getChildren().add(item);
-
- // Use no margin on top-most items.
- if (LocaleInfo.getCurrentLocale().isRTL()) {
- DOM.setIntStyleAttribute(item.getElement(), "marginRight", 0);
- } else {
- DOM.setIntStyleAttribute(item.getElement(), "marginLeft", 0);
- }
- }
-
- @Override
- public void removeItem(TreeItem item) {
- if (!getChildren().contains(item)) {
- return;
- }
-
- // Update Item state.
- item.setTree(null);
- item.setParentItem(null);
- getChildren().remove(item);
-
- DOM.removeChild(Tree.this.getElement(), item.getElement());
- }
- };
- root.initChildren();
+ root = new TreeItem(true);
root.setTree(this);
setStyleName("gwt-Tree");
=======================================
--- /trunk/user/src/com/google/gwt/user/client/ui/TreeItem.java Mon Jun 7
10:39:00 2010
+++ /trunk/user/src/com/google/gwt/user/client/ui/TreeItem.java Fri Jul 2
05:43:30 2010
@@ -38,6 +38,12 @@
* </p>
*/
public class TreeItem extends UIObject implements HasHTML {
+
+ /**
+ * The margin applied to child items.
+ */
+ private static final double CHILD_MARGIN = 16.0;
+
/**
* Implementation class for {...@link TreeItem}.
*/
@@ -247,6 +253,12 @@
private ArrayList<TreeItem> children;
private Element contentElem, childSpanElem, imageHolder;
+
+ /**
+ * Indicates that this item is a root item in a tree.
+ */
+ private boolean isRoot;
+
private boolean open;
private TreeItem parent;
private boolean selected;
@@ -261,10 +273,7 @@
* Creates an empty tree item.
*/
public TreeItem() {
- Element elem = DOM.clone(BASE_BARE_ELEM, true);
- setElement(elem);
- contentElem = DOM.getFirstChild(elem);
- DOM.setElementAttribute(contentElem, "id", DOM.createUniqueId());
+ this(false);
}
/**
@@ -286,6 +295,24 @@
this();
setWidget(widget);
}
+
+ /**
+ * Creates an empty tree item.
+ *
+ * @param isRoot true if this item is the root of a tree
+ */
+ TreeItem(boolean isRoot) {
+ this.isRoot = isRoot;
+ Element elem = DOM.clone(BASE_BARE_ELEM, true);
+ setElement(elem);
+ contentElem = DOM.getFirstChild(elem);
+ DOM.setElementAttribute(contentElem, "id", DOM.createUniqueId());
+
+ // The root item always has children.
+ if (isRoot) {
+ initChildren();
+ }
+ }
/**
* Adds a child tree item containing the specified text.
@@ -305,6 +332,9 @@
* @param item the item to be added
*/
public void addItem(TreeItem item) {
+ // If this is the item's parent, removing the item will affect the
child
+ // count.
+ maybeRemoveItemFromParent(item);
insertItem(getChildCount(), item);
}
@@ -441,9 +471,7 @@
public void insertItem(int beforeIndex, TreeItem item)
throws IndexOutOfBoundsException {
// Detach item from existing parent.
- if ((item.getParentItem() != null) || (item.getTree() != null)) {
- item.remove();
- }
+ maybeRemoveItemFromParent(item);
// Check the index after detaching in case this item was already the
parent.
int childCount = getChildCount();
@@ -456,28 +484,32 @@
}
// Set the margin.
+ // Use no margin on top-most items.
+ double margin = isRoot ? 0.0 : CHILD_MARGIN;
if (LocaleInfo.getCurrentLocale().isRTL()) {
- item.getElement().getStyle().setMarginRight(16.0, Unit.PX);
+ item.getElement().getStyle().setMarginRight(margin, Unit.PX);
} else {
- item.getElement().getStyle().setMarginLeft(16.0, Unit.PX);
+ item.getElement().getStyle().setMarginLeft(margin, Unit.PX);
}
// Physical attach.
+ Element childContainer = isRoot ? tree.getElement() : childSpanElem;
if (beforeIndex == childCount) {
- childSpanElem.appendChild(item.getElement());
+ childContainer.appendChild(item.getElement());
} else {
Element beforeElem = getChild(beforeIndex).getElement();
- childSpanElem.insertBefore(item.getElement(), beforeElem);
+ childContainer.insertBefore(item.getElement(), beforeElem);
}
// Logical attach.
- item.setParentItem(this);
- children.add(item);
+ // Explicitly set top-level items' parents to null if this is root.
+ item.setParentItem(isRoot ? null : this);
+ children.add(beforeIndex, item);
// Adopt.
item.setTree(tree);
- if (children.size() == 1) {
+ if (!isRoot && children.size() == 1) {
updateState(false, false);
}
}
@@ -534,16 +566,21 @@
}
// Orphan.
+ Tree oldTree = tree;
item.setTree(null);
// Physical detach.
- DOM.removeChild(childSpanElem, item.getElement());
+ if (isRoot) {
+ oldTree.getElement().removeChild(item.getElement());
+ } else {
+ childSpanElem.removeChild(item.getElement());
+ }
// Logical detach.
item.setParentItem(null);
children.remove(item);
- if (children.size() == 0) {
+ if (!isRoot && children.size() == 0) {
updateState(false, false);
}
}
@@ -774,6 +811,17 @@
boolean isFullNode() {
return imageHolder != null;
}
+
+ /**
+ * Remove a tree item from its parent if it has one.
+ *
+ * @param item the tree item to remove from its parent
+ */
+ void maybeRemoveItemFromParent(TreeItem item) {
+ if ((item.getParentItem() != null) || (item.getTree() != null)) {
+ item.remove();
+ }
+ }
void setParentItem(TreeItem parent) {
this.parent = parent;
=======================================
--- /trunk/user/test/com/google/gwt/user/client/ui/TreeItemTest.java Mon
Jun 7 10:39:00 2010
+++ /trunk/user/test/com/google/gwt/user/client/ui/TreeItemTest.java Fri
Jul 2 05:43:30 2010
@@ -26,22 +26,53 @@
public String getModuleName() {
return "com.google.gwt.user.User";
}
+
+ public void testAddIntoSameItem() {
+ TreeItem item = new TreeItem();
+
+ // Add the only child back to its parent.
+ TreeItem a = item.addItem("a");
+ item.addItem(a);
+ assertEquals(1, item.getChildCount());
+ assertEquals(a, item.getChild(0));
+
+ // Add a child back to its parent that has multiple children.
+ TreeItem b = item.addItem("b");
+ item.addItem(a);
+ assertEquals(2, item.getChildCount());
+ assertEquals(b, item.getChild(0));
+ assertEquals(a, item.getChild(1));
+ }
public void testInsert() {
TreeItem item = new TreeItem();
TreeItem b = item.addItem("b");
+ assertEquals(1, item.getChildCount());
+ assertEquals(b, item.getChild(0));
// Insert at zero.
TreeItem a = item.insertItem(0, "a");
+ assertEquals(2, item.getChildCount());
+ assertEquals(a, item.getChild(0));
+ assertEquals(b, item.getChild(1));
assertEquals(a.getElement().getNextSiblingElement(), b.getElement());
// Insert at end.
TreeItem d = item.insertItem(2, new Label("b"));
+ assertEquals(3, item.getChildCount());
+ assertEquals(a, item.getChild(0));
+ assertEquals(b, item.getChild(1));
+ assertEquals(d, item.getChild(2));
assertEquals(b.getElement().getNextSiblingElement(), d.getElement());
// Insert in the middle.
TreeItem c = new TreeItem("c");
item.insertItem(2, c);
+ assertEquals(4, item.getChildCount());
+ assertEquals(a, item.getChild(0));
+ assertEquals(b, item.getChild(1));
+ assertEquals(c, item.getChild(2));
+ assertEquals(d, item.getChild(3));
assertEquals(b.getElement().getNextSiblingElement(), c.getElement());
}
=======================================
--- /trunk/user/test/com/google/gwt/user/client/ui/TreeTest.java Mon Jun 7
10:39:00 2010
+++ /trunk/user/test/com/google/gwt/user/client/ui/TreeTest.java Fri Jul 2
05:43:30 2010
@@ -180,18 +180,32 @@
public void testRootInsert() {
Tree t = new Tree();
TreeItem b = t.addItem("b");
+ assertEquals(1, t.getItemCount());
+ assertEquals(b, t.getItem(0));
// Insert at zero.
TreeItem a = t.insertItem(0, "a");
+ assertEquals(2, t.getItemCount());
+ assertEquals(a, t.getItem(0));
+ assertEquals(b, t.getItem(1));
assertEquals(a.getElement().getNextSiblingElement(), b.getElement());
// Insert at end.
TreeItem d = t.insertItem(2, new Label("d"));
+ assertEquals(3, t.getItemCount());
+ assertEquals(a, t.getItem(0));
+ assertEquals(b, t.getItem(1));
+ assertEquals(d, t.getItem(2));
assertEquals(b.getElement().getNextSiblingElement(), d.getElement());
// Insert in the middle.
TreeItem c = new TreeItem("c");
t.insertItem(2, c);
+ assertEquals(4, t.getItemCount());
+ assertEquals(a, t.getItem(0));
+ assertEquals(b, t.getItem(1));
+ assertEquals(c, t.getItem(2));
+ assertEquals(d, t.getItem(3));
assertEquals(b.getElement().getNextSiblingElement(), c.getElement());
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
