OSDN Git Service

More GridLayout Fixes
authorTor Norbye <tnorbye@google.com>
Fri, 7 Oct 2011 02:52:03 +0000 (19:52 -0700)
committerTor Norbye <tnorbye@google.com>
Mon, 10 Oct 2011 17:00:05 +0000 (10:00 -0700)
This changeset fixes various issues in the GridLayout support:

(1) Add custom Paste support for GridLayout. The default copy/paste
    handler just treats a paste as a drag into (0,0) followed by a
    drop. That doesn't work well for GridLayout since it ends up
    writing the dragged elements into row/column 0,0. This changeset
    adds a custom override of the paste handler such that it adds the
    pasted elements into successive table cells instead. It still
    needs to adjust the column spans to avoid changing the current
    table structure; that will be done in a followup CL.

(2) Clean up the Layout Actions Bar a bit for GridLayout. Separate out
    the "Show Structure" and "Grid Mode" actions, and make
    grid-operations (Add/Remove Row/Column) only show up in Grid Mode,
    and similarly only show Snap to Grid in Free Mode.

(3) The hidden <Space> widgets should also be hidden when selected via
    Select All. Also fix a remaining issue with the action enablement
    of Select All.

(4) Fix a bug where the preview bounding rectangle was drawn at the
    wrong place when showing a center-horizontal proposal.

(5) Remove an assertion encountered during GridLayout conversion which
    is not always true.

Change-Id: I8c4c0cac5052e59c5943e535b2f790f420303f9d

eclipse/dictionary.txt
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/GridLayoutRule.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/grid/GridLayoutPainter.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/gridmode.png [new file with mode: 0644]
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutCanvas.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionOverlay.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/GridLayoutConverter.java

index 286b5e7..4eb4b94 100644 (file)
@@ -290,3 +290,4 @@ xmlns
 ydpi
 zip
 zipalign
+markup
index baebfad..1eaa6f4 100644 (file)
@@ -17,7 +17,9 @@
 package com.android.ide.common.layout;
 
 import static com.android.ide.common.layout.LayoutConstants.ANDROID_URI;
+import static com.android.ide.common.layout.LayoutConstants.ATTR_LAYOUT_COLUMN;
 import static com.android.ide.common.layout.LayoutConstants.ATTR_LAYOUT_GRAVITY;
+import static com.android.ide.common.layout.LayoutConstants.ATTR_LAYOUT_ROW;
 import static com.android.ide.common.layout.LayoutConstants.ATTR_ORIENTATION;
 import static com.android.ide.common.layout.LayoutConstants.FQCN_SPACE;
 import static com.android.ide.common.layout.LayoutConstants.GRAVITY_VALUE_FILL;
@@ -106,7 +108,8 @@ public class GridLayoutRule extends BaseLayoutRule {
     private static final String ACTION_ADD_COL = "_addcol"; //$NON-NLS-1$
     private static final String ACTION_REMOVE_COL = "_removecol"; //$NON-NLS-1$
     private static final String ACTION_ORIENTATION = "_orientation"; //$NON-NLS-1$
-    private static final String ACTION_SHOW_GRID = "_grid"; //$NON-NLS-1$
+    private static final String ACTION_SHOW_STRUCTURE = "_structure"; //$NON-NLS-1$
+    private static final String ACTION_GRID_MODE = "_gridmode"; //$NON-NLS-1$
     private static final String ACTION_SNAP = "_snap"; //$NON-NLS-1$
     private static final String ACTION_DEBUG = "_debug"; //$NON-NLS-1$
 
@@ -116,14 +119,15 @@ public class GridLayoutRule extends BaseLayoutRule {
     private static final URL ICON_REMOVE_ROW = GridLayoutRule.class.getResource("removerow.png"); //$NON-NLS-1$
     private static final URL ICON_ADD_COL = GridLayoutRule.class.getResource("addcol.png"); //$NON-NLS-1$
     private static final URL ICON_REMOVE_COL = GridLayoutRule.class.getResource("removecol.png"); //$NON-NLS-1$
-    private static final URL ICON_SHOW_GRID = GridLayoutRule.class.getResource("showgrid.png"); //$NON-NLS-1$
+    private static final URL ICON_SHOW_STRUCT = GridLayoutRule.class.getResource("showgrid.png"); //$NON-NLS-1$
+    private static final URL ICON_GRID_MODE = GridLayoutRule.class.getResource("gridmode.png"); //$NON-NLS-1$
     private static final URL ICON_SNAP = GridLayoutRule.class.getResource("snap.png"); //$NON-NLS-1$
 
     /**
      * Whether the IDE should show diagnostics for debugging the grid layout - including
      * spacers visibly in the outline, showing row and column numbers, and so on
      */
-    public static boolean sDebugGridLayout = false;
+    public static boolean sDebugGridLayout = CAN_DEBUG;
 
     /** Whether the structure (grid model) should be displayed persistently to the user */
     public static boolean sShowStructure = false;
@@ -171,12 +175,12 @@ public class GridLayoutRule extends BaseLayoutRule {
                 parentNode.editXml("Add/Remove Row/Column", new INodeHandler() {
                     public void handle(INode n) {
                         String id = action.getId();
-                        if (id.equals(ACTION_SHOW_GRID)) {
+                        if (id.equals(ACTION_SHOW_STRUCTURE)) {
                             sShowStructure = !sShowStructure;
-                            // HACK: ToggleButton controls two flags for now - show grid and
-                            // grid mode (handling drags in a grid mode)
+                            mRulesEngine.redraw();
+                            return;
+                        } else if (id.equals(ACTION_GRID_MODE)) {
                             sGridMode = !sGridMode;
-
                             mRulesEngine.redraw();
                             return;
                         } else if (id.equals(ACTION_SNAP)) {
@@ -205,30 +209,39 @@ public class GridLayoutRule extends BaseLayoutRule {
             }
         };
 
-        // Add Row and Add Column
-        actions.add(RuleAction.createSeparator(150));
-        actions.add(RuleAction.createAction(ACTION_ADD_COL, "Add Column", actionCallback,
-                ICON_ADD_COL, 160, false /* supportsMultipleNodes */));
-        actions.add(RuleAction.createAction(ACTION_ADD_ROW, "Add Row", actionCallback,
-                ICON_ADD_ROW, 165, false));
+        actions.add(RuleAction.createSeparator(142));
 
-        // Remove Row and Remove Column (if something is selected)
-        if (children != null && children.size() > 0) {
-            // TODO: Add "Merge Columns" and "Merge Rows" ?
+        actions.add(RuleAction.createToggle(ACTION_GRID_MODE, "Grid Model Mode",
+                sGridMode, actionCallback, ICON_GRID_MODE, 145, false));
 
-            actions.add(RuleAction.createAction(ACTION_REMOVE_COL, "Remove Column",
-                    actionCallback, ICON_REMOVE_COL, 170, false));
-            actions.add(RuleAction.createAction(ACTION_REMOVE_ROW, "Remove Row",
-                    actionCallback, ICON_REMOVE_ROW, 175, false));
-        }
+        // Add and Remove Column actions only apply in Grid Mode
+        if (sGridMode) {
+            // Add Row and Add Column
+            actions.add(RuleAction.createSeparator(150));
+            actions.add(RuleAction.createAction(ACTION_ADD_COL, "Add Column", actionCallback,
+                    ICON_ADD_COL, 160, false /* supportsMultipleNodes */));
+            actions.add(RuleAction.createAction(ACTION_ADD_ROW, "Add Row", actionCallback,
+                    ICON_ADD_ROW, 165, false));
 
-        actions.add(RuleAction.createSeparator(185));
+            // Remove Row and Remove Column (if something is selected)
+            if (children != null && children.size() > 0) {
+                // TODO: Add "Merge Columns" and "Merge Rows" ?
 
-        actions.add(RuleAction.createToggle(ACTION_SNAP, "Snap to Grid",
-                sSnapToGrid, actionCallback, ICON_SNAP, 190, false));
+                actions.add(RuleAction.createAction(ACTION_REMOVE_COL, "Remove Column",
+                        actionCallback, ICON_REMOVE_COL, 170, false));
+                actions.add(RuleAction.createAction(ACTION_REMOVE_ROW, "Remove Row",
+                        actionCallback, ICON_REMOVE_ROW, 175, false));
+            }
 
-        actions.add(RuleAction.createToggle(ACTION_SHOW_GRID, "Show Structure",
-                sShowStructure, actionCallback, ICON_SHOW_GRID, 200, false));
+            actions.add(RuleAction.createSeparator(185));
+        } else {
+            actions.add(RuleAction.createToggle(ACTION_SHOW_STRUCTURE, "Show Structure",
+                    sShowStructure, actionCallback, ICON_SHOW_STRUCT, 190, false));
+
+            // Snap to Grid and Show Structure are only relevant in free form mode
+            actions.add(RuleAction.createToggle(ACTION_SNAP, "Snap to Grid",
+                    sSnapToGrid, actionCallback, ICON_SNAP, 200, false));
+        }
 
         // Temporary: Diagnostics for GridLayout
         if (CAN_DEBUG) {
@@ -491,12 +504,13 @@ public class GridLayoutRule extends BaseLayoutRule {
         if (sShowStructure) {
             // TODO: Cache the grid
             if (view != null) {
-                GridLayoutPainter.paintStructure(view, DrawingStyle.GUIDELINE_DASHED,
-                        parentNode, graphics);
-            } else {
-                GridLayoutPainter.paintStructure(DrawingStyle.GUIDELINE_DASHED,
-                        parentNode, graphics, new GridModel(mRulesEngine, parentNode, view));
+                if (GridLayoutPainter.paintStructure(view, DrawingStyle.GUIDELINE_DASHED,
+                        parentNode, graphics)) {
+                    return;
+                }
             }
+            GridLayoutPainter.paintStructure(DrawingStyle.GUIDELINE_DASHED,
+                        parentNode, graphics, new GridModel(mRulesEngine, parentNode, view));
         } else if (sDebugGridLayout) {
             GridLayoutPainter.paintStructure(DrawingStyle.GRID,
                     parentNode, graphics, new GridModel(mRulesEngine, parentNode, view));
@@ -505,4 +519,66 @@ public class GridLayoutRule extends BaseLayoutRule {
         // TBD: Highlight the cells around the selection, and display easy controls
         // for for example tweaking the rowspan/colspan of a cell? (but only in grid mode)
     }
+
+    /**
+     * Paste into a GridLayout. We have several possible behaviors (and many
+     * more than are listed here):
+     * <ol>
+     * <li> Preserve the current positions of the elements (if pasted from another
+     *      canvas, not just XML markup copied from say a web site) and apply those
+     *      into the current grid. This might mean "overwriting" (sitting on top of)
+     *      existing elements.
+     * <li> Fill available "holes" in the grid.
+     * <li> Lay them out consecutively, row by row, like text.
+     * <li> Some hybrid approach, where I attempt to preserve the <b>relative</b>
+     *      relationships (columns/wrapping, spacing between the pasted views etc)
+     *      but I append them to the bottom of the layout on one or more new rows.
+     * <li> Try to paste at the current mouse position, if known, preserving the
+     *      relative distances between the existing elements there.
+     * </ol>
+     * Attempting to preserve the current position isn't possible right now,
+     * because the clipboard data contains only the textual representation of
+     * the markup. (We'd need to stash position information from a previous
+     * layout render along with the clipboard data).
+     * <p>
+     * Currently, this implementation simply lays out the elements row by row,
+     * approach #3 above.
+     */
+    @Override
+    public void onPaste(INode targetNode, Object targetView, IDragElement[] elements) {
+        DropFeedback feedback = onDropEnter(targetNode, targetView, elements);
+        if (feedback != null) {
+            Rect b = targetNode.getBounds();
+            if (!b.isValid()) {
+                return;
+            }
+
+            Map<String, Pair<String, String>> idMap = getDropIdMap(targetNode, elements,
+                    true /* remap id's */);
+
+            for (IDragElement element : elements) {
+                // Skip <Space> elements and only insert the real elements being
+                // copied
+                if (elements.length > 1 && FQCN_SPACE.equals(element.getFqcn())) {
+                    continue;
+                }
+
+                String fqcn = element.getFqcn();
+                INode newChild = targetNode.appendChild(fqcn);
+                addAttributes(newChild, element, idMap, DEFAULT_ATTR_FILTER);
+
+                // Ensure that we reset any potential row/column attributes from a different
+                // grid layout being copied from
+                newChild.setAttribute(ANDROID_URI, ATTR_LAYOUT_COLUMN, null);
+                newChild.setAttribute(ANDROID_URI, ATTR_LAYOUT_ROW, null);
+
+                // TODO: Set columnSpans to avoid making these widgets completely
+                // break the layout
+                // Alternatively, I could just lay them all out on subsequent lines
+                // with a column span of columnSpan5
+
+                addInnerElements(newChild, element, idMap);
+            }
+        }
+    }
 }
index add8706..70552ce 100644 (file)
@@ -174,7 +174,7 @@ public class GridLayoutPainter {
             if (columnMatch.type == SegmentType.RIGHT) {
                 offsetX -= dragBounds.w;
             } else if (columnMatch.type == SegmentType.CENTER_HORIZONTAL) {
-                offsetX -= dragBounds.centerX();
+                offsetX -= dragBounds.w / 2;
             }
 
             // Draw guidelines for matches
@@ -251,6 +251,7 @@ public class GridLayoutPainter {
             GridModel grid = data.getGrid();
 
             gc.useStyle(DrawingStyle.GUIDELINE);
+            // Paint grid
             for (int row = 1; row < grid.actualRowCount; row++) {
                 int y = grid.getRowY(row);
                 gc.drawLine(b.x, y - radius, b.x2(), y - radius);
@@ -301,14 +302,18 @@ public class GridLayoutPainter {
     }
 
     /**
-     * Paints the structure (the row and column boundaries) of the given GridLayout
+     * Paints the structure (the row and column boundaries) of the given
+     * GridLayout
      *
-     * @param view the instance of the GridLayout whose structure should be painted
+     * @param view the instance of the GridLayout whose structure should be
+     *            painted
      * @param style the drawing style to use for the cell boundaries
      * @param layout the layout element
      * @param gc the graphics context
+     * @return true if the structure was successfully inferred from the view and
+     *         painted
      */
-    public static void paintStructure(Object view, DrawingStyle style, INode layout,
+    public static boolean paintStructure(Object view, DrawingStyle style, INode layout,
             IGraphics gc) {
         Pair<int[],int[]> cellBounds = GridModel.getAxisBounds(view);
         if (cellBounds != null) {
@@ -324,6 +329,10 @@ public class GridLayoutPainter {
                 int x = xs[column] + b.x;
                 gc.drawLine(x, b.y, x, b.y2());
             }
+
+            return true;
+        } else {
+            return false;
         }
     }
 }
diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/gridmode.png b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/gridmode.png
new file mode 100644 (file)
index 0000000..59e0a45
Binary files /dev/null and b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/gridmode.png differ
index e20f61e..17b3f33 100755 (executable)
@@ -1151,6 +1151,7 @@ public class LayoutCanvas extends Canvas {
         mDeleteAction.setEnabled(hasSelection);
         // Select All should *always* be selectable, regardless of whether anything
         // is currently selected.
+        mSelectAllAction.setEnabled(true);
 
         // The paste operation is only available if we can paste our custom type.
         // We do not currently support pasting random text (e.g. XML). Maybe later.
index ff84e79..97d0481 100644 (file)
@@ -187,6 +187,11 @@ public class SelectionOverlay extends Overlay {
     /** Called by the canvas when a view is being selected. */
     private void paintSelection(IGraphics gc, GC swtGc, SelectionItem item,
             boolean isMultipleSelection) {
+        CanvasViewInfo view = item.getViewInfo();
+        if (view.isHidden()) {
+            return;
+        }
+
         NodeProxy selectedNode = item.getNode();
         Rect r = selectedNode.getBounds();
         if (!r.isValid()) {
@@ -211,7 +216,6 @@ public class SelectionOverlay extends Overlay {
         gc.drawRect(x1, y1, x2, y2);
 
         // Paint sibling rectangles, if applicable
-        CanvasViewInfo view = item.getViewInfo();
         List<CanvasViewInfo> siblings = view.getNodeSiblings();
         if (siblings != null) {
             for (CanvasViewInfo sibling : siblings) {
index 6dca1dd..ff9b0c7 100644 (file)
@@ -704,7 +704,11 @@ class GridLayoutConverter {
                 } else {
                     column = index;
                 }
-                assert column >= view.mCol;
+
+                if (column < view.mCol) {
+                    column = view.mCol;
+                }
+
                 view.mColSpan = column - view.mCol + 1;
             }
         }
@@ -756,7 +760,11 @@ class GridLayoutConverter {
                 } else {
                     row = index;
                 }
-                assert row >= view.mRow;
+
+                if (row < view.mRow) {
+                    row = view.mRow;
+                }
+
                 view.mRowSpan = row - view.mRow + 1;
             }
         }
@@ -945,7 +953,8 @@ class GridLayoutConverter {
 
             ElementDescriptor descriptor = child.getUiViewNode().getDescriptor();
             String name = descriptor.getXmlLocalName();
-            if (name.equals(LINEAR_LAYOUT) || name.equals(RELATIVE_LAYOUT)) {
+            if (name.equals(LINEAR_LAYOUT) || name.equals(RELATIVE_LAYOUT)
+                    || name.equals(TABLE_LAYOUT) || name.equals(TABLE_ROW)) {
                 // Don't delete layouts that provide a background image or gradient
                 if (element.hasAttributeNS(ANDROID_URI, ATTR_BACKGROUND)) {
                     AdtPlugin.log(IStatus.WARNING,