From 3c05a351908353c4c99c14bb5a8fcd3ea9fc7afe Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Thu, 29 Nov 2012 12:02:47 +0200 Subject: Update GridLayout DOM in onConnectorHierarchyChange (#10324, #10097) * Add ChildComponentData that is sent in the state instead of previous UIDL data * Store child data in a map instead of as a linked list * Move Cell instead of moving component form one Cell to the other if moved * Clean up internal state in remove(Widget) Change-Id: I3dabe0165b6dcdf70c0df06a0151b73d080187a5 --- client/src/com/vaadin/client/ui/VGridLayout.java | 127 +++--- .../client/ui/gridlayout/GridLayoutConnector.java | 123 +++--- server/src/com/vaadin/ui/GridLayout.java | 429 ++++++--------------- .../shared/ui/gridlayout/GridLayoutState.java | 17 + .../components/gridlayout/InsertRowInMiddle.html | 31 ++ .../components/gridlayout/InsertRowInMiddle.java | 54 +++ 6 files changed, 326 insertions(+), 455 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.html create mode 100644 uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.java diff --git a/client/src/com/vaadin/client/ui/VGridLayout.java b/client/src/com/vaadin/client/ui/VGridLayout.java index be8353ac6e..8a58c8e1b7 100644 --- a/client/src/com/vaadin/client/ui/VGridLayout.java +++ b/client/src/com/vaadin/client/ui/VGridLayout.java @@ -33,7 +33,6 @@ import com.vaadin.client.ComponentConnector; import com.vaadin.client.ConnectorMap; import com.vaadin.client.LayoutManager; import com.vaadin.client.StyleConstants; -import com.vaadin.client.UIDL; import com.vaadin.client.Util; import com.vaadin.client.VCaption; import com.vaadin.client.ui.gridlayout.GridLayoutConnector; @@ -41,6 +40,7 @@ import com.vaadin.client.ui.layout.ComponentConnectorLayoutSlot; import com.vaadin.client.ui.layout.VLayoutSlot; import com.vaadin.shared.ui.AlignmentInfo; import com.vaadin.shared.ui.MarginInfo; +import com.vaadin.shared.ui.gridlayout.GridLayoutState.ChildComponentData; public class VGridLayout extends ComplexPanel { @@ -499,10 +499,6 @@ public class VGridLayout extends ComplexPanel { this.col = col; } - public boolean hasContent() { - return hasContent; - } - public boolean hasRelativeHeight() { if (slot != null) { return slot.getChild().isRelativeHeight(); @@ -569,66 +565,67 @@ public class VGridLayout extends ComplexPanel { } } - final int row; - final int col; + private int row; + private int col; int colspan = 1; int rowspan = 1; - private boolean hasContent; - private AlignmentInfo alignment; /** For internal use only. May be removed or replaced in the future. */ public ComponentConnectorLayoutSlot slot; - public void updateFromUidl(UIDL cellUidl) { - // Set cell width - colspan = cellUidl.hasAttribute("w") ? cellUidl - .getIntAttribute("w") : 1; - // Set cell height - rowspan = cellUidl.hasAttribute("h") ? cellUidl - .getIntAttribute("h") : 1; - // ensure we will lose reference to old cells, now overlapped by - // this cell - for (int i = 0; i < colspan; i++) { - for (int j = 0; j < rowspan; j++) { - if (i > 0 || j > 0) { - cells[col + i][row + j] = null; - } + public void updateCell(ChildComponentData childComponentData) { + if (row != childComponentData.row1 + || col != childComponentData.column1) { + // cell has been moved, update matrix + if (col < cells.length && cells.length != 0 + && row < cells[0].length && cells[col][row] == this) { + // Remove old reference if still relevant + cells[col][row] = null; } + + row = childComponentData.row1; + col = childComponentData.column1; + + cells[col][row] = this; } - UIDL childUidl = cellUidl.getChildUIDL(0); // we are interested - // about childUidl - hasContent = childUidl != null; - if (hasContent) { - ComponentConnector childConnector = client - .getPaintable(childUidl); - - if (slot == null || slot.getChild() != childConnector) { - slot = new ComponentConnectorLayoutSlot(CLASSNAME, - childConnector, getConnector()); - if (childConnector.isRelativeWidth()) { - slot.getWrapperElement().getStyle() - .setWidth(100, Unit.PCT); - } - Element slotWrapper = slot.getWrapperElement(); - getElement().appendChild(slotWrapper); - - Widget widget = childConnector.getWidget(); - insert(widget, slotWrapper, getWidgetCount(), false); - Cell oldCell = widgetToCell.put(widget, this); - if (oldCell != null) { - oldCell.slot.getWrapperElement().removeFromParent(); - oldCell.slot = null; - } - } + // Set cell width + colspan = 1 + childComponentData.column2 + - childComponentData.column1; + // Set cell height + rowspan = 1 + childComponentData.row2 - childComponentData.row1; + setAlignment(new AlignmentInfo(childComponentData.alignment)); + } + + public void setComponent(ComponentConnector component) { + if (slot == null || slot.getChild() != component) { + slot = new ComponentConnectorLayoutSlot(CLASSNAME, component, + getConnector()); + slot.setAlignment(alignment); + if (component.isRelativeWidth()) { + slot.getWrapperElement().getStyle().setWidth(100, Unit.PCT); + } + Element slotWrapper = slot.getWrapperElement(); + getElement().appendChild(slotWrapper); + + Widget widget = component.getWidget(); + insert(widget, slotWrapper, getWidgetCount(), false); + Cell oldCell = widgetToCell.put(widget, this); + if (oldCell != null) { + oldCell.slot.getWrapperElement().removeFromParent(); + oldCell.slot = null; + } } } public void setAlignment(AlignmentInfo alignmentInfo) { - slot.setAlignment(alignmentInfo); + alignment = alignmentInfo; + if (slot != null) { + slot.setAlignment(alignmentInfo); + } } } @@ -638,8 +635,7 @@ public class VGridLayout extends ComplexPanel { } /** - * Creates a new Cell with the given coordinates. If an existing cell was - * found, returns that one. + * Creates a new Cell with the given coordinates. *

* For internal use only. May be removed or replaced in the future. * @@ -647,12 +643,9 @@ public class VGridLayout extends ComplexPanel { * @param col * @return */ - public Cell createCell(int row, int col) { - Cell cell = getCell(row, col); - if (cell == null) { - cell = new Cell(row, col); - cells[col][row] = cell; - } + public Cell createNewCell(int row, int col) { + Cell cell = new Cell(row, col); + cells[col][row] = cell; return cell; } @@ -734,4 +727,24 @@ public class VGridLayout extends ComplexPanel { } } + @Override + public boolean remove(Widget w) { + boolean removed = super.remove(w); + if (removed) { + Cell cell = widgetToCell.remove(w); + if (cell != null) { + cell.slot.setCaption(null); + cell.slot.getWrapperElement().removeFromParent(); + cell.slot = null; + + if (cells.length < cell.col && cells.length != 0 + && cells[0].length < cell.row + && cells[cell.col][cell.row] == cell) { + cells[cell.col][cell.row] = null; + } + } + } + return removed; + } + } diff --git a/client/src/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java b/client/src/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java index 16125f883e..3284d3e07d 100644 --- a/client/src/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java +++ b/client/src/com/vaadin/client/ui/gridlayout/GridLayoutConnector.java @@ -15,14 +15,13 @@ */ package com.vaadin.client.ui.gridlayout; -import java.util.Iterator; +import java.util.Map.Entry; import com.google.gwt.user.client.Element; import com.google.gwt.user.client.ui.Widget; import com.vaadin.client.ApplicationConnection; import com.vaadin.client.ComponentConnector; import com.vaadin.client.ConnectorHierarchyChangeEvent; -import com.vaadin.client.ConnectorMap; import com.vaadin.client.DirectionalManagedLayout; import com.vaadin.client.Paintable; import com.vaadin.client.UIDL; @@ -33,12 +32,13 @@ import com.vaadin.client.ui.LayoutClickEventHandler; import com.vaadin.client.ui.VGridLayout; import com.vaadin.client.ui.VGridLayout.Cell; import com.vaadin.client.ui.layout.VLayoutSlot; -import com.vaadin.shared.ui.AlignmentInfo; +import com.vaadin.shared.Connector; import com.vaadin.shared.ui.Connect; import com.vaadin.shared.ui.LayoutClickRpc; import com.vaadin.shared.ui.MarginInfo; import com.vaadin.shared.ui.gridlayout.GridLayoutServerRpc; import com.vaadin.shared.ui.gridlayout.GridLayoutState; +import com.vaadin.shared.ui.gridlayout.GridLayoutState.ChildComponentData; import com.vaadin.ui.GridLayout; @Connect(GridLayout.class) @@ -60,11 +60,11 @@ public class GridLayoutConnector extends AbstractComponentContainerConnector }; - private boolean needCaptionUpdate = false; - @Override public void init() { super.init(); + getWidget().client = getConnection(); + getLayoutManager().registerDependency(this, getWidget().spacingMeasureElement); } @@ -98,64 +98,21 @@ public class GridLayoutConnector extends AbstractComponentContainerConnector @Override public void updateFromUIDL(UIDL uidl, ApplicationConnection client) { VGridLayout layout = getWidget(); - layout.client = client; if (!isRealUpdate(uidl)) { return; } - int cols = getState().columns; - int rows = getState().rows; + initSize(); - layout.columnWidths = new int[cols]; - layout.rowHeights = new int[rows]; + for (Entry entry : getState().childData + .entrySet()) { + ComponentConnector child = (ComponentConnector) entry.getKey(); - layout.setSize(rows, cols); + Cell cell = getCell(child); - final int[] alignments = uidl.getIntArrayAttribute("alignments"); - int alignmentIndex = 0; - - for (final Iterator i = uidl.getChildIterator(); i.hasNext();) { - final UIDL r = (UIDL) i.next(); - if ("gr".equals(r.getTag())) { - for (final Iterator j = r.getChildIterator(); j.hasNext();) { - final UIDL cellUidl = (UIDL) j.next(); - if ("gc".equals(cellUidl.getTag())) { - int row = cellUidl.getIntAttribute("y"); - int col = cellUidl.getIntAttribute("x"); - - Widget previousWidget = null; - - Cell cell = layout.getCell(row, col); - if (cell != null && cell.slot != null) { - // This is an update. Track if the widget changes - // and update the caption if that happens. This - // workaround can be removed once the DOM update is - // done in onContainerHierarchyChange - previousWidget = cell.slot.getWidget(); - } - - cell = layout.createCell(row, col); - - cell.updateFromUidl(cellUidl); - - if (cell.hasContent()) { - cell.setAlignment(new AlignmentInfo( - alignments[alignmentIndex++])); - if (cell.slot.getWidget() != previousWidget) { - // Widget changed or widget moved from another - // slot. Update its caption as the widget might - // have called updateCaption when the widget was - // still in its old slot. This workaround can be - // removed once the DOM update - // is done in onContainerHierarchyChange - updateCaption(ConnectorMap.get(getConnection()) - .getConnector(cell.slot.getWidget())); - } - } - } - } - } + ChildComponentData childComponentData = entry.getValue(); + cell.updateCell(childComponentData); } layout.colExpandRatioArray = uidl.getIntArrayAttribute("colExpand"); @@ -165,14 +122,22 @@ public class GridLayoutConnector extends AbstractComponentContainerConnector layout.updateSpacingStyleName(getState().spacing); - if (needCaptionUpdate) { - needCaptionUpdate = false; + getLayoutManager().setNeedsLayout(this); + } + + private Cell getCell(ComponentConnector child) { + VGridLayout layout = getWidget(); + Cell cell = layout.widgetToCell.get(child.getWidget()); - for (ComponentConnector child : getChildComponents()) { - updateCaption(child); - } + if (cell == null) { + ChildComponentData childComponentData = getState().childData + .get(child); + int row = childComponentData.row1; + int col = childComponentData.column1; + + cell = layout.createNewCell(row, col); } - getLayoutManager().setNeedsLayout(this); + return cell; } @Override @@ -187,33 +152,33 @@ public class GridLayoutConnector extends AbstractComponentContainerConnector Widget childWidget = oldChild.getWidget(); layout.remove(childWidget); + } - Cell cell = layout.widgetToCell.remove(childWidget); - cell.slot.setCaption(null); - cell.slot.getWrapperElement().removeFromParent(); - cell.slot = null; + initSize(); + + for (ComponentConnector componentConnector : getChildComponents()) { + Cell cell = getCell(componentConnector); + + cell.setComponent(componentConnector); } } + private void initSize() { + VGridLayout layout = getWidget(); + int cols = getState().columns; + int rows = getState().rows; + + layout.columnWidths = new int[cols]; + layout.rowHeights = new int[rows]; + + layout.setSize(rows, cols); + } + @Override public void updateCaption(ComponentConnector childConnector) { - if (!childConnector.delegateCaptionHandling()) { - // Check not required by interface but by workarounds in this class - // when updateCaption is explicitly called for all children. - return; - } - VGridLayout layout = getWidget(); Cell cell = layout.widgetToCell.get(childConnector.getWidget()); - if (cell == null) { - // workaround before updateFromUidl is removed. We currently update - // the captions at the end of updateFromUidl instead of immediately - // because the DOM has not been set up at this point (as it is done - // in updateFromUidl) - needCaptionUpdate = true; - return; - } if (VCaption.isNeeded(childConnector.getState())) { VLayoutSlot layoutSlot = cell.slot; VCaption caption = layoutSlot.getCaption(); diff --git a/server/src/com/vaadin/ui/GridLayout.java b/server/src/com/vaadin/ui/GridLayout.java index 36f4c40a52..be961964e6 100644 --- a/server/src/com/vaadin/ui/GridLayout.java +++ b/server/src/com/vaadin/ui/GridLayout.java @@ -27,7 +27,6 @@ import java.util.Map.Entry; import com.vaadin.event.LayoutEvents.LayoutClickEvent; import com.vaadin.event.LayoutEvents.LayoutClickListener; import com.vaadin.event.LayoutEvents.LayoutClickNotifier; -import com.vaadin.server.LegacyPaint; import com.vaadin.server.PaintException; import com.vaadin.server.PaintTarget; import com.vaadin.shared.Connector; @@ -36,6 +35,7 @@ import com.vaadin.shared.MouseEventDetails; import com.vaadin.shared.ui.MarginInfo; import com.vaadin.shared.ui.gridlayout.GridLayoutServerRpc; import com.vaadin.shared.ui.gridlayout.GridLayoutState; +import com.vaadin.shared.ui.gridlayout.GridLayoutState.ChildComponentData; /** * A layout where the components are laid out on a grid using cell coordinates. @@ -88,30 +88,8 @@ public class GridLayout extends AbstractLayout implements */ private int cursorY = 0; - /** - * Contains all items that are placed on the grid. These are components with - * grid area definition. - */ - private final LinkedList areas = new LinkedList(); - - /** - * Mapping from components to their respective areas. - */ private final LinkedList components = new LinkedList(); - /** - * Mapping from components to alignments (horizontal + vertical). - */ - private Map componentToAlignment = new HashMap(); - - private static final Alignment ALIGNMENT_DEFAULT = Alignment.TOP_LEFT; - - /** - * Has there been rows inserted or deleted in the middle of the layout since - * the last paint operation. - */ - private boolean structuralChange = false; - private Map columnExpandRatio = new HashMap(); private Map rowExpandRatio = new HashMap(); @@ -176,7 +154,7 @@ public class GridLayout extends AbstractLayout implements *

* * @param component - * the component to be added. + * the component to be added, not null. * @param column1 * the column of the upper left corner of the area c * is supposed to occupy. The leftmost column has index 0. @@ -228,29 +206,30 @@ public class GridLayout extends AbstractLayout implements // Inserts the component to right place at the list // Respect top-down, left-right ordering // component.setParent(this); - final Iterator i = areas.iterator(); + final Iterator i = components.iterator(); + final Map childDataMap = getState().childData; int index = 0; boolean done = false; while (!done && i.hasNext()) { - final Area existingArea = i.next(); + final ChildComponentData existingArea = childDataMap.get(i.next()); if ((existingArea.row1 >= row1 && existingArea.column1 > column1) || existingArea.row1 > row1) { - areas.add(index, area); components.add(index, component); done = true; } index++; } if (!done) { - areas.addLast(area); components.addLast(component); } + childDataMap.put(component, area.childData); + // Attempt to add to super try { super.addComponent(component); } catch (IllegalArgumentException e) { - areas.remove(area); + childDataMap.remove(component); components.remove(component); throw e; } @@ -284,11 +263,12 @@ public class GridLayout extends AbstractLayout implements * if area overlaps with any existing area. */ private void checkExistingOverlaps(Area area) throws OverlapsException { - for (final Iterator i = areas.iterator(); i.hasNext();) { - final Area existingArea = i.next(); - if (existingArea.overlaps(area)) { + for (Entry entry : getState().childData + .entrySet()) { + if (componentsOverlap(entry.getValue(), area.childData)) { // Component not added, overlaps with existing component - throw new OverlapsException(existingArea); + throw new OverlapsException(new Area(entry.getValue(), + (Component) entry.getKey())); } } } @@ -300,7 +280,7 @@ public class GridLayout extends AbstractLayout implements * is 1. * * @param component - * the component to be added. + * the component to be added, not null. * @param column * the column index, starting from 0. * @param row @@ -356,10 +336,13 @@ public class GridLayout extends AbstractLayout implements * grid is automatically extended. * * @param component - * the component to be added. + * the component to be added, not null. */ @Override public void addComponent(Component component) { + if (component == null) { + throw new IllegalArgumentException("Component must not be null"); + } // Finds first available place from the grid Area area; @@ -399,25 +382,10 @@ public class GridLayout extends AbstractLayout implements return; } - Area area = null; - for (final Iterator i = areas.iterator(); area == null - && i.hasNext();) { - final Area a = i.next(); - if (a.getComponent() == component) { - area = a; - } - } - + getState().childData.remove(component); components.remove(component); - if (area != null) { - areas.remove(area); - } - - componentToAlignment.remove(component); super.removeComponent(component); - - markAsDirty(); } /** @@ -431,10 +399,12 @@ public class GridLayout extends AbstractLayout implements public void removeComponent(int column, int row) { // Finds the area - for (final Iterator i = areas.iterator(); i.hasNext();) { - final Area area = i.next(); - if (area.getColumn1() == column && area.getRow1() == row) { - removeComponent(area.getComponent()); + for (final Iterator i = components.iterator(); i.hasNext();) { + final Component component = i.next(); + final ChildComponentData childData = getState().childData + .get(component); + if (childData.column1 == column && childData.row1 == row) { + removeComponent(component); return; } } @@ -477,23 +447,6 @@ public class GridLayout extends AbstractLayout implements */ @Override public void paintContent(PaintTarget target) throws PaintException { - // TODO refactor attribute names in future release. - target.addAttribute("structuralChange", structuralChange); - structuralChange = false; - - // Area iterator - final Iterator areaiterator = areas.iterator(); - - // Current item to be processed (fetch first item) - Area area = areaiterator.hasNext() ? (Area) areaiterator.next() : null; - - // Collects rowspan related information here - final HashMap cellUsed = new HashMap(); - - // Empty cell collector - int emptyCells = 0; - - final String[] alignmentsArray = new String[components.size()]; final Integer[] columnExpandRatioArray = new Integer[getColumns()]; final Integer[] rowExpandRatioArray = new Integer[getRows()]; @@ -517,158 +470,24 @@ public class GridLayout extends AbstractLayout implements } } - boolean equallyDividedRows = false; int realRowExpandRatioSum = 0; float rowSum = getExpandRatioSum(rowExpandRatio); if (rowSum == 0) { // no rows have been expanded - equallyDividedRows = true; float equalSize = 1 / (float) getRows(); int myRatio = Math.round(equalSize * 1000); for (int i = 0; i < getRows(); i++) { rowExpandRatioArray[i] = myRatio; } realRowExpandRatioSum = myRatio * getRows(); - } - - int index = 0; - - // Iterates every applicable row - for (int cury = 0; cury < getRows(); cury++) { - target.startTag("gr"); - - if (!equallyDividedRows) { + } else { + for (int cury = 0; cury < getRows(); cury++) { int myRatio = Math .round((getRowExpandRatio(cury) / rowSum) * 1000); rowExpandRatioArray[cury] = myRatio; realRowExpandRatioSum += myRatio; - } - // Iterates every applicable column - for (int curx = 0; curx < getColumns(); curx++) { - - // Checks if current item is located at curx,cury - if (area != null && (area.row1 == cury) - && (area.column1 == curx)) { - - // First check if empty cell needs to be rendered - if (emptyCells > 0) { - target.startTag("gc"); - target.addAttribute("x", curx - emptyCells); - target.addAttribute("y", cury); - if (emptyCells > 1) { - target.addAttribute("w", emptyCells); - } - target.endTag("gc"); - emptyCells = 0; - } - - // Now proceed rendering current item - final int cols = (area.column2 - area.column1) + 1; - final int rows = (area.row2 - area.row1) + 1; - target.startTag("gc"); - - target.addAttribute("x", curx); - target.addAttribute("y", cury); - - if (cols > 1) { - target.addAttribute("w", cols); - } - if (rows > 1) { - target.addAttribute("h", rows); - } - LegacyPaint.paint(area.getComponent(), target); - - alignmentsArray[index++] = String - .valueOf(getComponentAlignment(area.getComponent()) - .getBitMask()); - - target.endTag("gc"); - - // Fetch next item - if (areaiterator.hasNext()) { - area = areaiterator.next(); - } else { - area = null; - } - - // Updates the cellUsed if rowspan needed - if (rows > 1) { - int spannedx = curx; - for (int j = 1; j <= cols; j++) { - cellUsed.put(new Integer(spannedx), new Integer( - cury + rows - 1)); - spannedx++; - } - } - - // Skips the current item's spanned columns - if (cols > 1) { - curx += cols - 1; - } - - } else { - - // Checks against cellUsed, render space or ignore cell - if (cellUsed.containsKey(new Integer(curx))) { - - // Current column contains already an item, - // check if rowspan affects at current x,y position - final int rowspanDepth = cellUsed - .get(new Integer(curx)).intValue(); - - if (rowspanDepth >= cury) { - - // ignore cell - // Check if empty cell needs to be rendered - if (emptyCells > 0) { - target.startTag("gc"); - target.addAttribute("x", curx - emptyCells); - target.addAttribute("y", cury); - if (emptyCells > 1) { - target.addAttribute("w", emptyCells); - } - target.endTag("gc"); - - emptyCells = 0; - } - } else { - - // empty cell is needed - emptyCells++; - - // Removes the cellUsed key as it has become - // obsolete - cellUsed.remove(Integer.valueOf(curx)); - } - } else { - - // empty cell is needed - emptyCells++; - } - } - - } // iterates every column - - // Last column handled of current row - - // Checks if empty cell needs to be rendered - if (emptyCells > 0) { - target.startTag("gc"); - target.addAttribute("x", getColumns() - emptyCells); - target.addAttribute("y", cury); - if (emptyCells > 1) { - target.addAttribute("w", emptyCells); - } - target.endTag("gc"); - - emptyCells = 0; - } - - target.endTag("gr"); - } // iterates every row - - // Last row handled + } // correct possible rounding error if (rowExpandRatioArray.length > 0) { @@ -681,9 +500,6 @@ public class GridLayout extends AbstractLayout implements target.addAttribute("colExpand", columnExpandRatioArray); target.addAttribute("rowExpand", rowExpandRatioArray); - // Add child component alignment info to layout tag - target.addAttribute("alignments", alignmentsArray); - } private float getExpandRatioSum(Map ratioMap) { @@ -703,11 +519,13 @@ public class GridLayout extends AbstractLayout implements */ @Override public Alignment getComponentAlignment(Component childComponent) { - Alignment alignment = componentToAlignment.get(childComponent); - if (alignment == null) { - return ALIGNMENT_DEFAULT; + ChildComponentData childComponentData = getState().childData + .get(childComponent); + if (childComponentData == null) { + throw new IllegalArgumentException( + "The given component is not a child of this layout"); } else { - return alignment; + return new Alignment(childComponentData.alignment); } } @@ -728,31 +546,8 @@ public class GridLayout extends AbstractLayout implements * @since 3.0 */ public class Area implements Serializable { - - /** - * The column of the upper left corner cell of the area. - */ - private final int column1; - - /** - * The row of the upper left corner cell of the area. - */ - private int row1; - - /** - * The column of the lower right corner cell of the area. - */ - private final int column2; - - /** - * The row of the lower right corner cell of the area. - */ - private int row2; - - /** - * Component painted in the area. - */ - private Component component; + private final ChildComponentData childData; + private final Component component; /** *

@@ -776,10 +571,16 @@ public class GridLayout extends AbstractLayout implements */ public Area(Component component, int column1, int row1, int column2, int row2) { - this.column1 = column1; - this.row1 = row1; - this.column2 = column2; - this.row2 = row2; + this.component = component; + childData = new ChildComponentData(); + childData.column1 = column1; + childData.row1 = row1; + childData.column2 = column2; + childData.row2 = row2; + } + + public Area(ChildComponentData childData, Component component) { + this.childData = childData; this.component = component; } @@ -793,9 +594,7 @@ public class GridLayout extends AbstractLayout implements * this on, false if it does not. */ public boolean overlaps(Area other) { - return column1 <= other.getColumn2() && row1 <= other.getRow2() - && column2 >= other.getColumn1() && row2 >= other.getRow1(); - + return componentsOverlap(childData, other.childData); } /** @@ -807,28 +606,13 @@ public class GridLayout extends AbstractLayout implements return component; } - /** - * Sets the component connected to the area. - * - *

- * This function only sets the value in the data structure and does not - * send any events or set parents. - *

- * - * @param newComponent - * the new connected overriding the existing one. - */ - protected void setComponent(Component newComponent) { - component = newComponent; - } - /** * Gets the column of the top-left corner cell. * * @return the column of the top-left corner cell. */ public int getColumn1() { - return column1; + return childData.column1; } /** @@ -837,7 +621,7 @@ public class GridLayout extends AbstractLayout implements * @return the column of the bottom-right corner cell. */ public int getColumn2() { - return column2; + return childData.column2; } /** @@ -846,7 +630,7 @@ public class GridLayout extends AbstractLayout implements * @return the row of the top-left corner cell. */ public int getRow1() { - return row1; + return childData.row1; } /** @@ -855,11 +639,17 @@ public class GridLayout extends AbstractLayout implements * @return the row of the bottom-right corner cell. */ public int getRow2() { - return row2; + return childData.row2; } } + private static boolean componentsOverlap(ChildComponentData a, + ChildComponentData b) { + return a.column1 <= b.column2 && a.row1 <= b.row2 + && a.column2 >= b.column1 && a.row2 >= b.row1; + } + /** * Gridlayout does not support laying components on top of each other. An * OverlapsException is thrown when a component already exists @@ -895,13 +685,13 @@ public class GridLayout extends AbstractLayout implements } sb.append(")"); sb.append(" is already added to "); - sb.append(existingArea.column1); + sb.append(existingArea.childData.column1); sb.append(","); - sb.append(existingArea.column1); + sb.append(existingArea.childData.column1); sb.append(","); - sb.append(existingArea.row1); + sb.append(existingArea.childData.row1); sb.append(","); - sb.append(existingArea.row2); + sb.append(existingArea.childData.row2); sb.append("(column1, column2, row1, row2)."); return sb.toString(); @@ -970,13 +760,15 @@ public class GridLayout extends AbstractLayout implements // Checks for overlaps if (getColumns() > columns) { - for (final Iterator i = areas.iterator(); i.hasNext();) { - final Area area = i.next(); - if (area.column2 >= columns) { - throw new OutOfBoundsException(area); + for (Entry entry : getState().childData + .entrySet()) { + if (entry.getValue().column2 >= columns) { + throw new OutOfBoundsException(new Area(entry.getValue(), + (Component) entry.getKey())); } } } + // TODO forget expands for removed columns getState().columns = columns; } @@ -1012,13 +804,15 @@ public class GridLayout extends AbstractLayout implements // Checks for overlaps if (getRows() > rows) { - for (final Iterator i = areas.iterator(); i.hasNext();) { - final Area area = i.next(); - if (area.row2 >= rows) { - throw new OutOfBoundsException(area); + for (Entry entry : getState().childData + .entrySet()) { + if (entry.getValue().row2 >= rows) { + throw new OutOfBoundsException(new Area(entry.getValue(), + (Component) entry.getKey())); } } } + // TODO forget expands for removed rows getState().rows = rows; } @@ -1090,30 +884,22 @@ public class GridLayout extends AbstractLayout implements public void replaceComponent(Component oldComponent, Component newComponent) { // Gets the locations - Area oldLocation = null; - Area newLocation = null; - for (final Iterator i = areas.iterator(); i.hasNext();) { - final Area location = i.next(); - final Component component = location.getComponent(); - if (component == oldComponent) { - oldLocation = location; - } - if (component == newComponent) { - newLocation = location; - } - } + ChildComponentData oldLocation = getState().childData.get(oldComponent); + ChildComponentData newLocation = getState().childData.get(newComponent); if (oldLocation == null) { addComponent(newComponent); } else if (newLocation == null) { removeComponent(oldComponent); - addComponent(newComponent, oldLocation.getColumn1(), - oldLocation.getRow1(), oldLocation.getColumn2(), - oldLocation.getRow2()); + addComponent(newComponent, oldLocation.column1, oldLocation.row1, + oldLocation.column2, oldLocation.row2); } else { - oldLocation.setComponent(newComponent); - newLocation.setComponent(oldComponent); - markAsDirty(); + int oldAlignment = oldLocation.alignment; + oldLocation.alignment = newLocation.alignment; + newLocation.alignment = oldAlignment; + + getState().childData.put(newComponent, oldLocation); + getState().childData.put(oldComponent, newLocation); } } @@ -1125,7 +911,6 @@ public class GridLayout extends AbstractLayout implements @Override public void removeAllComponents() { super.removeAllComponents(); - componentToAlignment = new HashMap(); cursorX = 0; cursorY = 0; } @@ -1133,8 +918,19 @@ public class GridLayout extends AbstractLayout implements @Override public void setComponentAlignment(Component childComponent, Alignment alignment) { - componentToAlignment.put(childComponent, alignment); - markAsDirty(); + ChildComponentData childComponentData = getState().childData + .get(childComponent); + if (childComponentData == null) { + throw new IllegalArgumentException( + "Component must be added to layout before using setComponentAlignment()"); + } else { + if (alignment == null) { + childComponentData.alignment = GridLayoutState.ALIGNMENT_DEFAULT + .getBitMask(); + } else { + childComponentData.alignment = alignment.getBitMask(); + } + } } /* @@ -1170,8 +966,7 @@ public class GridLayout extends AbstractLayout implements + " in a gridlayout with height " + getRows()); } - for (Iterator i = areas.iterator(); i.hasNext();) { - Area existingArea = i.next(); + for (ChildComponentData existingArea : getState().childData.values()) { // Areas ending below the row needs to be moved down or stretched if (existingArea.row2 >= row) { existingArea.row2++; @@ -1189,7 +984,6 @@ public class GridLayout extends AbstractLayout implements } setRows(getRows() + 1); - structuralChange = true; markAsDirty(); } @@ -1222,8 +1016,7 @@ public class GridLayout extends AbstractLayout implements } // Shrink or remove areas in the selected row - for (Iterator i = areas.iterator(); i.hasNext();) { - Area existingArea = i.next(); + for (ChildComponentData existingArea : getState().childData.values()) { if (existingArea.row2 >= row) { existingArea.row2--; @@ -1248,7 +1041,6 @@ public class GridLayout extends AbstractLayout implements } } - structuralChange = true; markAsDirty(); } @@ -1339,12 +1131,12 @@ public class GridLayout extends AbstractLayout implements * @return Component in given cell or null if empty */ public Component getComponent(int x, int y) { - for (final Iterator iterator = areas.iterator(); iterator - .hasNext();) { - final Area area = iterator.next(); - if (area.getColumn1() <= x && x <= area.getColumn2() - && area.getRow1() <= y && y <= area.getRow2()) { - return area.getComponent(); + for (Entry entry : getState().childData + .entrySet()) { + ChildComponentData childData = entry.getValue(); + if (childData.column1 <= x && x <= childData.column2 + && childData.row1 <= y && y <= childData.row2) { + return (Component) entry.getKey(); } } return null; @@ -1360,14 +1152,13 @@ public class GridLayout extends AbstractLayout implements * the grid */ public Area getComponentArea(Component component) { - for (final Iterator iterator = areas.iterator(); iterator - .hasNext();) { - final Area area = iterator.next(); - if (area.getComponent() == component) { - return area; - } + ChildComponentData childComponentData = getState().childData + .get(component); + if (childComponentData == null) { + return null; + } else { + return new Area(childComponentData, component); } - return null; } @Override diff --git a/shared/src/com/vaadin/shared/ui/gridlayout/GridLayoutState.java b/shared/src/com/vaadin/shared/ui/gridlayout/GridLayoutState.java index e8a90adcd5..af5ae87895 100644 --- a/shared/src/com/vaadin/shared/ui/gridlayout/GridLayoutState.java +++ b/shared/src/com/vaadin/shared/ui/gridlayout/GridLayoutState.java @@ -15,9 +15,17 @@ */ package com.vaadin.shared.ui.gridlayout; +import java.io.Serializable; +import java.util.HashMap; +import java.util.Map; + +import com.vaadin.shared.Connector; import com.vaadin.shared.ui.AbstractLayoutState; +import com.vaadin.shared.ui.AlignmentInfo; public class GridLayoutState extends AbstractLayoutState { + public static AlignmentInfo ALIGNMENT_DEFAULT = AlignmentInfo.TOP_LEFT; + { primaryStyleName = "v-gridlayout"; } @@ -25,4 +33,13 @@ public class GridLayoutState extends AbstractLayoutState { public int rows = 0; public int columns = 0; public int marginsBitmask = 0; + public Map childData = new HashMap(); + + public static class ChildComponentData implements Serializable { + public int column1; + public int row1; + public int column2; + public int row2; + public int alignment = ALIGNMENT_DEFAULT.getBitMask(); + } } \ No newline at end of file diff --git a/uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.html b/uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.html new file mode 100644 index 0000000000..7de29edd1b --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.html @@ -0,0 +1,31 @@ + + + + + + +New Test + + + + + + + + + + + + + + + + + + + + + +
New Test
open/run/com.vaadin.tests.components.gridlayout.InsertRowInMiddle?restartApplication
clickvaadin=runcomvaadintestscomponentsgridlayoutInsertRowInMiddle::/VVerticalLayout[0]/VOrderedLayout$Slot[1]/VVerticalLayout[0]/VOrderedLayout$Slot[0]/VGridLayout[0]/VButton[0]/domChild[0]/domChild[0]
assertTextvaadin=runcomvaadintestscomponentsgridlayoutInsertRowInMiddle::/VVerticalLayout[0]/VOrderedLayout$Slot[1]/VVerticalLayout[0]/VOrderedLayout$Slot[0]/VGridLayout[0]/VLabel[1]some new row
+ + diff --git a/uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.java b/uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.java new file mode 100644 index 0000000000..0fc764e463 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/gridlayout/InsertRowInMiddle.java @@ -0,0 +1,54 @@ +/* + * Copyright 2012 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package com.vaadin.tests.components.gridlayout; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.GridLayout; +import com.vaadin.ui.Label; + +public class InsertRowInMiddle extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + final GridLayout layout = new GridLayout(1, 2); + layout.addComponent(new Label("some row"), 0, 0); + Button newRowButton = new Button("Insert Row"); + newRowButton.addListener(new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + layout.insertRow(1); + layout.addComponent(new Label("some new row"), 0, 1); + } + }); + layout.addComponent(newRowButton, 0, 1); + addComponent(layout); + } + + @Override + protected String getTestDescription() { + return "A new row added to the middle of a GridLayout should appear without any exception being thrown."; + } + + @Override + protected Integer getTicketNumber() { + return Integer.valueOf(10097); + } + +} -- cgit v1.2.3