From e632293a36d9a334d8e87c8baa1f4196f6fb917e Mon Sep 17 00:00:00 2001 From: Tor Norbye Date: Thu, 6 Oct 2011 19:52:03 -0700 Subject: [PATCH] More GridLayout Fixes 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 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 | 1 + .../android/ide/common/layout/GridLayoutRule.java | 138 ++++++++++++++++----- .../ide/common/layout/grid/GridLayoutPainter.java | 17 ++- .../src/com/android/ide/common/layout/gridmode.png | Bin 0 -> 519 bytes .../internal/editors/layout/gle2/LayoutCanvas.java | 1 + .../editors/layout/gle2/SelectionOverlay.java | 6 +- .../layout/refactoring/GridLayoutConverter.java | 15 ++- 7 files changed, 139 insertions(+), 39 deletions(-) create mode 100644 eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/gridmode.png diff --git a/eclipse/dictionary.txt b/eclipse/dictionary.txt index 286b5e778..4eb4b94e5 100644 --- a/eclipse/dictionary.txt +++ b/eclipse/dictionary.txt @@ -290,3 +290,4 @@ xmlns ydpi zip zipalign +markup diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/GridLayoutRule.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/GridLayoutRule.java index baebfad3f..1eaa6f46a 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/GridLayoutRule.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/GridLayoutRule.java @@ -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): + *
    + *
  1. 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. + *
  2. Fill available "holes" in the grid. + *
  3. Lay them out consecutively, row by row, like text. + *
  4. Some hybrid approach, where I attempt to preserve the relative + * 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. + *
  5. Try to paste at the current mouse position, if known, preserving the + * relative distances between the existing elements there. + *
+ * 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). + *

+ * 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> idMap = getDropIdMap(targetNode, elements, + true /* remap id's */); + + for (IDragElement element : elements) { + // Skip 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); + } + } + } } diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/grid/GridLayoutPainter.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/grid/GridLayoutPainter.java index add870666..70552ce79 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/grid/GridLayoutPainter.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/common/layout/grid/GridLayoutPainter.java @@ -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 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 index 0000000000000000000000000000000000000000..59e0a45110df0b8574f72fe4fbdb03bdf5e7856a GIT binary patch literal 519 zcmeAS@N?(olHy`uVBq!ia0vp^0wB!61|;P_|4#%`jKx9jP7LeL$-D$|*pj^6T^Rm@ z;DWu&Cj&(|3p^r=85p>QL70(Y)*K0-AbW|YuPgg)E;dnd%h_{tOn^eNnIRD+&iT2y zsd*(pE(3#eQEFmIYKlU6W=V#EyQgnJie4%^(7HHJ7sn8b(@Q5CdNC&owBB!9v3H(> zPR^Pwg1JsA50oUhi`i`*znDDIG;4LeaH^rJV{R_rLhH}-j{ezwQtj4@lK#KXAD^9l zQ|El~!~Rea*K2GI58rP1xX}1r!W~IwA&rIA_m4imDgK1Twd2Uo1O|qTaNq2;TiK4> zRpbnk*m6lhFoj`N)_HT*SbOF*EUFz37cmy7`>=T?FkP@U4RL5TW{6^uu`QX-WL0-Y zy6SJ<;)TUej2$-GAJPiTI=$DEfAQYHGdd5SJY~6jIJ3xO!l9>UkMcMeS6pjaV5Lyb zuF2s!+r5};(@e%rF`Gjxor;}LRK!GCK2KmQ_*JuOisS#a_b){INAFKJu$^)3nnqIb zrq2Gm{acI@f0s(Xw|@U}Led%SnDY$J!pu+4{Fe7Jhhf6s@A*gSUrW2C2ea<@=bHkG0#8>zmvv4F FO#n05&W`{9 literal 0 HcmV?d00001 diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutCanvas.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutCanvas.java index e20f61e33..17b3f332c 100755 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutCanvas.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/LayoutCanvas.java @@ -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. diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionOverlay.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionOverlay.java index ff84e7940..97d048108 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionOverlay.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionOverlay.java @@ -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 siblings = view.getNodeSiblings(); if (siblings != null) { for (CanvasViewInfo sibling : siblings) { diff --git a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/GridLayoutConverter.java b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/GridLayoutConverter.java index 6dca1ddce..ff9b0c73a 100644 --- a/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/GridLayoutConverter.java +++ b/eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/refactoring/GridLayoutConverter.java @@ -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, -- 2.11.0