Merge "Usability fix for the layout actions bar"

9 files changed, 130 insertions, 18 deletions

diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/api/IClientRulesEngine.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/api/IClientRulesEngine.java
index e849340..6aa4776 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/api/IClientRulesEngine.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/api/IClientRulesEngine.java
@@ -19,6 +19,8 @@ package com.android.ide.common.api;
 
 import com.android.annotations.Nullable;
 
+import java.util.Collection;
+
 /**
  * A Client Rules Engine is a set of methods that {@link IViewRule}s can use to
  * access the client public API of the Rules Engine. Rules can access it via
@@ -143,5 +145,12 @@ public interface IClientRulesEngine {
      * @return the layout resource to include
      */
     String displayIncludeSourceInput();
+
+    /**
+     * Select the given nodes
+     *
+     * @param nodes the nodes to be selected, never null
+     */
+    void select(Collection<INode> nodes);
 }
 
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/BaseViewRule.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/BaseViewRule.java
index 1571e03..96e8d7f 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/BaseViewRule.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/BaseViewRule.java
@@ -659,7 +659,7 @@ public class BaseViewRule implements IViewRule {
     public void onChildInserted(INode node, INode parent, InsertType insertType) {
     }
 
-    private static String stripIdPrefix(String id) {
+    public static String stripIdPrefix(String id) {
         if (id.startsWith(NEW_ID_PREFIX)) {
             id = id.substring(NEW_ID_PREFIX.length());
         } else if (id.startsWith(ID_PREFIX)) {
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/FrameLayoutRule.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/FrameLayoutRule.java
index af001c6..83a93be 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/FrameLayoutRule.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/FrameLayoutRule.java
@@ -161,7 +161,7 @@ public class FrameLayoutRule extends BaseLayoutRule {
         super.addLayoutActions(actions, parentNode, children);
         actions.add(MenuAction.createSeparator(25));
         actions.add(createMarginAction(parentNode, children));
-        if (children.size() > 0) {
+        if (children != null && children.size() > 0) {
             actions.add(createGravityAction(children, ATTR_LAYOUT_GRAVITY));
         }
     }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableLayoutRule.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableLayoutRule.java
index c3b3bfa..cc67d3a 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableLayoutRule.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableLayoutRule.java
@@ -17,6 +17,7 @@ package com.android.ide.common.layout;
 
 import static com.android.ide.common.layout.LayoutConstants.FQCN_TABLE_ROW;
 
+import com.android.ide.common.api.IClientRulesEngine;
 import com.android.ide.common.api.IMenuCallback;
 import com.android.ide.common.api.INode;
 import com.android.ide.common.api.INodeHandler;
@@ -25,6 +26,7 @@ import com.android.ide.common.api.InsertType;
 import com.android.ide.common.api.MenuAction;
 
 import java.net.URL;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -69,8 +71,8 @@ public class TableLayoutRule extends LinearLayoutRule {
         IMenuCallback addTab = new IMenuCallback() {
             public void action(MenuAction action, final String valueId, Boolean newValue) {
                 final INode node = selectedNode;
-                node.appendChild(FQCN_TABLE_ROW);
-
+                INode newRow = node.appendChild(FQCN_TABLE_ROW);
+                mRulesEngine.select(Collections.singletonList(newRow));
             }
         };
         return concatenate(super.getContextMenu(selectedNode),
@@ -82,13 +84,14 @@ public class TableLayoutRule extends LinearLayoutRule {
     public void addLayoutActions(List<MenuAction> actions, final INode parentNode,
             final List<? extends INode> children) {
         super.addLayoutActions(actions, parentNode, children);
-        addTableLayoutActions(actions, parentNode, children);
+        addTableLayoutActions(mRulesEngine, actions, parentNode, children);
     }
 
     /**
      * Adds layout actions to add and remove toolbar items
      */
-    static void addTableLayoutActions(List<MenuAction> actions, final INode parentNode,
+    static void addTableLayoutActions(final IClientRulesEngine rulesEngine,
+            List<MenuAction> actions, final INode parentNode,
             final List<? extends INode> children) {
         IMenuCallback actionCallback = new IMenuCallback() {
             public void action(final MenuAction action, final String valueId,
@@ -96,7 +99,36 @@ public class TableLayoutRule extends LinearLayoutRule {
                 parentNode.editXml("Add/Remove Table Row", new INodeHandler() {
                     public void handle(INode n) {
                         if (action.getId().equals(ACTION_ADD_ROW)) {
-                            parentNode.appendChild(FQCN_TABLE_ROW);
+                            // Determine the index of the selection, if any; if there is
+                            // a selection, insert the row before the current row, otherwise
+                            // append it to the table.
+                            int index = -1;
+                            INode[] rows = parentNode.getChildren();
+                            if (children != null) {
+                                findTableIndex:
+                                for (INode child : children) {
+                                    // Find direct child of table layout
+                                    while (child != null && child.getParent() != parentNode) {
+                                        child = child.getParent();
+                                    }
+                                    if (child != null) {
+                                        // Compute index of direct child of table layout
+                                        for (int i = 0; i < rows.length; i++) {
+                                            if (rows[i] == child) {
+                                                index = i;
+                                                break findTableIndex;
+                                            }
+                                        }
+                                    }
+                                }
+                            }
+                            INode newRow;
+                            if (index == -1) {
+                                newRow = parentNode.appendChild(FQCN_TABLE_ROW);
+                            } else {
+                                newRow = parentNode.insertChildAt(FQCN_TABLE_ROW, index);
+                            }
+                            rulesEngine.select(Collections.singletonList(newRow));
                         } else if (action.getId().equals(ACTION_REMOVE_ROW)) {
                             // Find the direct children of the TableLayout to delete;
                             // this is necessary since TableRow might also use
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableRowRule.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableRowRule.java
index 13e648e..ac03653 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableRowRule.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/TableRowRule.java
@@ -53,9 +53,12 @@ public class TableRowRule extends LinearLayoutRule {
         // Also apply table-specific actions on the table row such that you can
         // select something in a table row and still get offered actions on the surrounding
         // table.
-        INode grandParent = parentNode.getParent();
-        if (grandParent != null && grandParent.getFqcn().equals(FQCN_TABLE_LAYOUT)) {
-            TableLayoutRule.addTableLayoutActions(actions, grandParent, children);
+        if (children != null) {
+            INode grandParent = parentNode.getParent();
+            if (grandParent != null && grandParent.getFqcn().equals(FQCN_TABLE_LAYOUT)) {
+                TableLayoutRule.addTableLayoutActions(mRulesEngine, actions, grandParent,
+                        children);
+            }
         }
     }
 }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutActionBar.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutActionBar.java
index 313064a..0dcd83e 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutActionBar.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutActionBar.java
@@ -15,10 +15,14 @@
  */
 package com.android.ide.eclipse.adt.internal.editors.layout.gle2;
 
+import static com.android.ide.common.layout.LayoutConstants.ANDROID_URI;
+import static com.android.ide.common.layout.LayoutConstants.ATTR_ID;
+
 import com.android.ide.common.api.MenuAction;
 import com.android.ide.common.api.MenuAction.OrderedChoices;
 import com.android.ide.common.api.MenuAction.Separator;
 import com.android.ide.common.api.MenuAction.Toggle;
+import com.android.ide.common.layout.BaseViewRule;
 import com.android.ide.eclipse.adt.AdtPlugin;
 import com.android.ide.eclipse.adt.internal.editors.IconFactory;
 import com.android.ide.eclipse.adt.internal.editors.layout.configuration.ConfigurationComposite;
@@ -116,25 +120,58 @@ public class LayoutActionBar extends Composite {
         }
         List<MenuAction> actions = new ArrayList<MenuAction>();
         engine.callAddLayoutActions(actions, parent, selectedNodes);
-        addActions(actions);
+
+        // Place actions in the correct order (the actions may come from different
+        // rules and should be merged properly via sorting keys)
+        Collections.sort(actions);
+
+        // Add in actions for the child as well, if there is exactly one.
+        // These are not merged into the parent list of actions; they are appended
+        // at the end.
+        int index = -1;
+        String label = null;
+        if (selectedNodes.size() == 1) {
+            List<MenuAction> itemActions = new ArrayList<MenuAction>();
+            NodeProxy selectedNode = selectedNodes.get(0);
+            engine.callAddLayoutActions(itemActions, selectedNode, null);
+            if (itemActions.size() > 0) {
+                Collections.sort(itemActions);
+
+                if (!(itemActions.get(0) instanceof MenuAction.Separator)) {
+                    actions.add(MenuAction.createSeparator(0));
+                }
+                label = selectedNode.getStringAttr(ANDROID_URI, ATTR_ID);
+                if (label != null) {
+                    label = BaseViewRule.stripIdPrefix(label);
+                    index = actions.size();
+                }
+                actions.addAll(itemActions);
+            }
+        }
+
+        addActions(actions, index, label);
 
         mLayoutToolBar.pack();
         mLayoutToolBar.layout();
     }
 
-    private void addActions(List<MenuAction> actions) {
+    private void addActions(List<MenuAction> actions, int labelIndex, String label) {
         if (actions.size() > 0) {
-            // Place actions in the correct order (the actions may come from different
-            // rules and should be merged properly via sorting keys)
-            Collections.sort(actions);
-
             // Flag used to indicate that if there are any actions -after- this, it
             // should be separated from this current action (we don't unconditionally
             // add a separator at the end of these groups in case there are no more
             // actions at the end so that we don't have a trailing separator)
             boolean needSeparator = false;
 
-            for (final MenuAction action : actions) {
+            int index = 0;
+            for (MenuAction action : actions) {
+                if (index == labelIndex) {
+                    final ToolItem button = new ToolItem(mLayoutToolBar, SWT.PUSH);
+                    button.setText(label);
+                    needSeparator = false;
+                }
+                index++;
+
                 if (action instanceof Separator) {
                     addSeparator(mLayoutToolBar);
                     needSeparator = false;
@@ -145,7 +182,7 @@ public class LayoutActionBar extends Composite {
                 }
 
                 if (action instanceof MenuAction.OrderedChoices) {
-                    final MenuAction.OrderedChoices choices = (OrderedChoices) action;
+                    MenuAction.OrderedChoices choices = (OrderedChoices) action;
                     if (!choices.isRadio()) {
                         addDropdown(choices);
                     } else {
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionManager.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionManager.java
index bf95ee6..bcc433d 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionManager.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionManager.java
@@ -477,6 +477,17 @@ public class SelectionManager implements ISelectionProvider {
         redraw();
     }
 
+    public void select(Collection<INode> nodes) {
+        List<CanvasViewInfo> infos = new ArrayList<CanvasViewInfo>(nodes.size());
+        for (INode node : nodes) {
+            CanvasViewInfo info = mCanvas.getViewHierarchy().findViewInfoFor(node);
+            if (info != null) {
+                infos.add(info);
+            }
+        }
+        selectMultiple(infos);
+    }
+
     /**
      * Selects the visual element corresponding to the given XML node
      * @param xmlNode The Node whose element we want to select.
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gre/RulesEngine.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gre/RulesEngine.java
index 8c2051f..7b51c3a 100755
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gre/RulesEngine.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gre/RulesEngine.java
@@ -38,6 +38,8 @@ import com.android.ide.eclipse.adt.internal.editors.descriptors.ElementDescripto
 import com.android.ide.eclipse.adt.internal.editors.layout.descriptors.ViewElementDescriptor;
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.GraphicalEditorPart;
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.IncludeFinder;
+import com.android.ide.eclipse.adt.internal.editors.layout.gle2.LayoutCanvas;
+import com.android.ide.eclipse.adt.internal.editors.layout.gle2.SelectionManager;
 import com.android.ide.eclipse.adt.internal.editors.layout.gle2.SimpleElement;
 import com.android.ide.eclipse.adt.internal.editors.layout.uimodel.UiViewElementNode;
 import com.android.ide.eclipse.adt.internal.resources.manager.ResourceManager;
@@ -959,5 +961,19 @@ public class RulesEngine {
 
             return null;
         }
+
+        public void select(final Collection<INode> nodes) {
+            LayoutCanvas layoutCanvas = mEditor.getCanvasControl();
+            final SelectionManager selectionManager = layoutCanvas.getSelectionManager();
+            selectionManager.select(nodes);
+            // ALSO run an async select since immediately after nodes are created they
+            // may not be selectable. We can't ONLY run an async exec since
+            // code may depend on operating on the selection.
+            layoutCanvas.getDisplay().asyncExec(new Runnable() {
+                public void run() {
+                    selectionManager.select(nodes);
+                }
+            });
+        }
     }
 }
diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/common/layout/LayoutTestBase.java b/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/common/layout/LayoutTestBase.java
index a7aae04..18d985e 100644
--- a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/common/layout/LayoutTestBase.java
+++ b/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/common/layout/LayoutTestBase.java
@@ -249,6 +249,10 @@ public class LayoutTestBase extends TestCase {
             fail("Not supported in tests yet");
             return null;
         }
+
+        public void select(Collection<INode> nodes) {
+            fail("Not supported in tests yet");
+        }
     }
 
     public void testDummy() {