Merge "Don't assign ids to all newly dropped widgets"

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],