Don't assign ids to all newly dropped widgets

This changeset makes the layout editor no longer assign default id's
to all newly dropped widgets. In particular, it no longer assigns id's
to *Layout widgets, such as LinearLayout and RelativeLayout. These
id's are typically unused, and in the case where you do want an id to
manipulate the widget you probably want to change it to some more
logical name than "linearLayout1" anyway. (It also stops assigning
id's to <include> tags, <merge> tags and <Space> widgets.)

It also changes the label of the "Edit ID..." context menu action to
"Assign ID..." for widgets that do not currently have an id.

Change-Id: Ibf0ec25a6a687e34d3eebf828251d196cadb9d54

3 files changed, 54 insertions, 10 deletions

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 cb75b0c..e7facf5 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
@@ -363,7 +363,9 @@ public class BaseViewRule extends AbstractViewRule {
                     null, 10, true));
         }
 
-        actions.add(RuleAction.createAction(ATTR_ID, "Edit ID...", onChange, null, 20, true));
+        String editIdLabel = selectedNode.getStringAttr(ANDROID_URI, ATTR_ID) != null ?
+                "Edit ID..." : "Assign ID...";
+        actions.add(RuleAction.createAction(ATTR_ID, editIdLabel, onChange, null, 20, true));
 
         addCommonPropertyActions(actions, selectedNode, onChange, 21);
 
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtils.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtils.java
index d4b0a67..4693a1e 100644
--- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtils.java
+++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtils.java
@@ -36,6 +36,9 @@ import static com.android.ide.common.layout.LayoutConstants.SPACE;
 import static com.android.ide.common.layout.LayoutConstants.VALUE_FILL_PARENT;
 import static com.android.ide.common.layout.LayoutConstants.VALUE_WRAP_CONTENT;
 import static com.android.ide.eclipse.adt.internal.editors.layout.descriptors.LayoutDescriptors.REQUEST_FOCUS;
+import static com.android.ide.eclipse.adt.internal.editors.layout.descriptors.LayoutDescriptors.VIEW_FRAGMENT;
+import static com.android.ide.eclipse.adt.internal.editors.layout.descriptors.LayoutDescriptors.VIEW_INCLUDE;
+import static com.android.ide.eclipse.adt.internal.editors.layout.descriptors.LayoutDescriptors.VIEW_MERGE;
 
 import com.android.ide.common.api.IAttributeInfo.Format;
 import com.android.ide.common.resources.platform.AttributeInfo;
@@ -687,8 +690,8 @@ public final class DescriptorsUtils {
         ElementDescriptor descriptor = node.getDescriptor();
 
         String name = descriptor.getXmlLocalName();
-        if (name.equals(REQUEST_FOCUS) || name.equals(SPACE)) {
-            // Don't add ids etc to <requestFocus>, or to grid spacers
+        if (name.equals(REQUEST_FOCUS)) {
+            // Don't add ids, widths and heights etc to <requestFocus>
             return;
         }
 
@@ -709,13 +712,15 @@ public final class DescriptorsUtils {
                     false /* override */);
         }
 
-        String freeId = getFreeWidgetId(node);
-        if (freeId != null) {
-            node.setAttributeValue(
-                    ATTR_ID,
-                    SdkConstants.NS_RESOURCES,
-                    freeId,
-                    false /* override */);
+        if (needsDefaultId(node.getDescriptor())) {
+            String freeId = getFreeWidgetId(node);
+            if (freeId != null) {
+                node.setAttributeValue(
+                        ATTR_ID,
+                        SdkConstants.NS_RESOURCES,
+                        freeId,
+                        false /* override */);
+            }
         }
 
         // Set a text attribute on textual widgets -- but only on those that define a text
@@ -753,6 +758,28 @@ public final class DescriptorsUtils {
     }
 
     /**
+     * Determines whether new views of the given type should be assigned a
+     * default id.
+     *
+     * @param descriptor a descriptor describing the view to look up
+     * @return true if new views of the given type should be assigned a default
+     *         id
+     */
+    public static boolean needsDefaultId(ElementDescriptor descriptor) {
+        // By default, layouts do not need ids.
+        String tag = descriptor.getXmlLocalName();
+        if (tag.endsWith("Layout")  //$NON-NLS-1$
+                || tag.equals(VIEW_FRAGMENT)
+                || tag.equals(VIEW_INCLUDE)
+                || tag.equals(VIEW_MERGE)
+                || tag.equals(SPACE)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
      * Given a UI node, returns the first available id that matches the
      * pattern "prefix%d".
      * <p/>TabWidget is a special case and the method will always return "@android:id/tabs".
diff --git a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtilsTest.java b/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtilsTest.java
index 3fa1cf9..51781be 100644
--- a/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtilsTest.java
+++ b/eclipse/plugins/com.android.ide.eclipse.tests/unittests/com/android/ide/eclipse/adt/internal/editors/descriptors/DescriptorsUtilsTest.java
@@ -26,6 +26,7 @@ import junit.framework.TestCase;
 /**
  * Unit tests for DescriptorsUtils in the editors plugin
  */
+@SuppressWarnings("javadoc")
 public class DescriptorsUtilsTest extends TestCase {
 
     @Override
@@ -129,6 +130,20 @@ public class DescriptorsUtilsTest extends TestCase {
                 DescriptorsUtils.getFreeWidgetId(uiRoot, "LinearLayout"));
     }
 
+    public void testNeedsDefaultId() throws Exception {
+        assertTrue(DescriptorsUtils.needsDefaultId(new ElementDescriptor("Button")));
+        assertTrue(DescriptorsUtils.needsDefaultId(new ElementDescriptor("EditText")));
+        assertTrue(DescriptorsUtils.needsDefaultId(new ElementDescriptor("TextView")));
+
+        assertFalse(DescriptorsUtils.needsDefaultId(new ElementDescriptor("LinearLayout")));
+        assertFalse(DescriptorsUtils.needsDefaultId(new ElementDescriptor("GridLayout")));
+        assertFalse(DescriptorsUtils.needsDefaultId(new ElementDescriptor("RelativeLayout")));
+        assertFalse(DescriptorsUtils.needsDefaultId(new ElementDescriptor("include")));
+        assertFalse(DescriptorsUtils.needsDefaultId(new ElementDescriptor("merge")));
+        assertFalse(DescriptorsUtils.needsDefaultId(new ElementDescriptor("fragment")));
+        assertFalse(DescriptorsUtils.needsDefaultId(new ElementDescriptor("Space")));
+    }
+
     private static ViewElementDescriptor createDesc(String name, String fqn, boolean hasChildren) {
         if (hasChildren) {
             return new ViewElementDescriptor(name, name, fqn, "", "", new AttributeDescriptor[0],