From 3ab88b718cdb480058fc9c53506a3dabe4980554 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Tue, 16 Sep 2014 17:01:21 +0300 Subject: [PATCH] Fix GridStaticSection communication to use column ids (#13334) Change-Id: Ic5174543cab912ea8647b92445f33ec3d9fca366 --- .../vaadin/client/ui/grid/GridConnector.java | 23 +--- .../com/vaadin/ui/components/grid/Grid.java | 6 +- .../ui/components/grid/GridStaticSection.java | 119 +++++++++--------- .../component/grid/GridStaticSection.java | 2 +- .../ui/grid/GridStaticSectionState.java | 4 +- 5 files changed, 77 insertions(+), 77 deletions(-) diff --git a/client/src/com/vaadin/client/ui/grid/GridConnector.java b/client/src/com/vaadin/client/ui/grid/GridConnector.java index 26649e6111..d34a57a4b3 100644 --- a/client/src/com/vaadin/client/ui/grid/GridConnector.java +++ b/client/src/com/vaadin/client/ui/grid/GridConnector.java @@ -45,7 +45,6 @@ import com.vaadin.client.ui.grid.renderers.AbstractRendererConnector; import com.vaadin.client.ui.grid.selection.AbstractRowHandleSelectionModel; import com.vaadin.client.ui.grid.selection.SelectionChangeEvent; import com.vaadin.client.ui.grid.selection.SelectionChangeHandler; -import com.vaadin.client.ui.grid.selection.SelectionModel; import com.vaadin.client.ui.grid.selection.SelectionModelMulti; import com.vaadin.client.ui.grid.selection.SelectionModelNone; import com.vaadin.client.ui.grid.selection.SelectionModelSingle; @@ -422,18 +421,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements for (RowState rowState : state.rows) { GridStaticSection.StaticRow row = section.appendRow(); - int selectionOffset = 1; - if (getWidget().getSelectionModel() instanceof SelectionModel.None) { - selectionOffset = 0; - } - - assert rowState.cells.size() == getWidget().getColumnCount() - - selectionOffset; - - int i = 0 + selectionOffset; for (CellState cellState : rowState.cells) { - GridStaticSection.StaticCell cell = row.getCell(getWidget() - .getColumn(i++)); + CustomGridColumn column = columnIdToColumn + .get(cellState.columnId); + GridStaticSection.StaticCell cell = row.getCell(column); switch (cellState.type) { case TEXT: cell.setText(cellState.text); @@ -451,12 +442,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements } } - for (List group : rowState.cellGroups) { + for (List group : rowState.cellGroups) { GridColumn[] columns = new GridColumn[group.size()]; - i = 0; - for (Integer colIndex : group) { - columns[i++] = getWidget().getColumn( - selectionOffset + colIndex); + for (int i = 0; i < group.size(); ++i) { + columns[i] = columnIdToColumn.get(group.get(i)); } row.join(columns); } diff --git a/server/src/com/vaadin/ui/components/grid/Grid.java b/server/src/com/vaadin/ui/components/grid/Grid.java index 81a4598097..e29b1cf196 100644 --- a/server/src/com/vaadin/ui/components/grid/Grid.java +++ b/server/src/com/vaadin/ui/components/grid/Grid.java @@ -590,6 +590,10 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier, GridColumnState columnState = new GridColumnState(); columnState.id = columnKeys.key(datasourcePropertyId); + + GridColumn column = new GridColumn(this, columnState); + columns.put(datasourcePropertyId, column); + getState().columns.add(columnState); for (int i = 0; i < getHeader().getRowCount(); ++i) { @@ -600,8 +604,6 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier, getFooter().getRow(i).addCell(datasourcePropertyId); } - GridColumn column = new GridColumn(this, columnState); - columns.put(datasourcePropertyId, column); column.setHeaderCaption(String.valueOf(datasourcePropertyId)); return column; diff --git a/server/src/com/vaadin/ui/components/grid/GridStaticSection.java b/server/src/com/vaadin/ui/components/grid/GridStaticSection.java index 80a39022e9..398ee6630e 100644 --- a/server/src/com/vaadin/ui/components/grid/GridStaticSection.java +++ b/server/src/com/vaadin/ui/components/grid/GridStaticSection.java @@ -24,6 +24,7 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import com.vaadin.data.Container.Indexed; import com.vaadin.shared.ui.grid.GridStaticCellType; @@ -63,6 +64,7 @@ abstract class GridStaticSection> protected void addCell(Object propertyId) { CELLTYPE cell = createCell(); + cell.setColumnId(section.grid.getColumn(propertyId).getState().id); cells.put(propertyId, cell); rowState.cells.add(cell.getCellState()); } @@ -89,62 +91,6 @@ abstract class GridStaticSection> return cells.get(propertyId); } - /** - * Merges cells in a row - * - * @param cells - * The cells to be merged - * @return The first cell of the merged cells - */ - protected CELLTYPE join(List cells) { - assert cells.size() > 1 : "You cannot merge less than 2 cells together"; - - // Ensure no cell is already grouped - for (CELLTYPE cell : cells) { - if (getCellGroupForCell(cell) != null) { - throw new IllegalStateException("Cell " + cell.getText() - + " is already grouped."); - } - } - - // Ensure continuous range - Iterator cellIterator = this.cells.values().iterator(); - CELLTYPE current = null; - int firstIndex = 0; - - while (cellIterator.hasNext()) { - current = cellIterator.next(); - if (current == cells.get(0)) { - break; - } - firstIndex++; - } - - for (int i = 1; i < cells.size(); ++i) { - current = cellIterator.next(); - - if (current != cells.get(i)) { - throw new IllegalStateException( - "Cell range must be a continous range"); - } - } - - // Create a new group - final ArrayList cellGroup = new ArrayList(cells); - cellGroups.add(cellGroup); - - // Add group to state - List stateGroup = new ArrayList(); - for (int i = 0; i < cells.size(); ++i) { - stateGroup.add(firstIndex + i); - } - rowState.cellGroups.add(stateGroup); - section.markAsDirty(); - - // Returns first cell of group - return cells.get(0); - } - /** * Merges columns cells in a row * @@ -153,6 +99,8 @@ abstract class GridStaticSection> * @return The remaining visible cell after the merge */ public CELLTYPE join(Object... properties) { + assert properties.length > 1 : "You need to merge at least 2 properties"; + List cells = new ArrayList(); for (int i = 0; i < properties.length; ++i) { cells.add(getCell(properties[i])); @@ -169,9 +117,60 @@ abstract class GridStaticSection> * @return The remaining visible cell after the merge */ public CELLTYPE join(CELLTYPE... cells) { + assert cells.length > 1 : "You need to merge at least 2 cells"; + return join(Arrays.asList(cells)); } + protected CELLTYPE join(List cells) { + for (CELLTYPE cell : cells) { + if (getCellGroupForCell(cell) != null) { + throw new IllegalArgumentException("Cell already merged"); + } else if (!this.cells.containsValue(cell)) { + throw new IllegalArgumentException( + "Cell does not exist on this row"); + } + } + + if (cellsInContinuousRange(cells)) { + List columnGroup = new ArrayList(); + for (CELLTYPE cell : cells) { + columnGroup.add(cell.getColumnId()); + } + rowState.cellGroups.add(columnGroup); + cellGroups.add(cells); + return cells.get(0); + } else { + throw new IllegalArgumentException( + "Cells are in invalid order or not in a contiunous range"); + } + } + + private boolean cellsInContinuousRange(List mergeCells) { + Iterator mergeCellIterator = mergeCells.iterator(); + CELLTYPE mergeCell = mergeCellIterator.next(); + boolean firstFound = false; + for (Entry entry : cells.entrySet()) { + // Go through all the cells until first to be merged is found + CELLTYPE currentCell = entry.getValue(); + if (currentCell == mergeCell) { + if (!mergeCellIterator.hasNext()) { + // All the cells to be merged are found and they + // were in continuous range + return true; + } + mergeCell = mergeCellIterator.next(); + firstFound = true; + } else if (firstFound) { + // We found the first cell already, but at least one cell + // was not in a continuous range. + return false; + } + } + + return false; + } + private List getCellGroupForCell(CELLTYPE cell) { for (List group : cellGroups) { if (group.contains(cell)) { @@ -194,6 +193,14 @@ abstract class GridStaticSection> this.row = row; } + private void setColumnId(String id) { + cellState.columnId = id; + } + + private String getColumnId() { + return cellState.columnId; + } + /** * Gets the row where this cell is. * diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java b/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java index e89f6a8c6e..3b00867257 100644 --- a/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java +++ b/server/tests/src/com/vaadin/tests/server/component/grid/GridStaticSection.java @@ -88,7 +88,7 @@ public class GridStaticSection { mergeRow.getCell("zipCode")); } - @Test(expected = IllegalStateException.class) + @Test(expected = IllegalArgumentException.class) public void testJoinHeaderCellsIncorrectly() { final GridHeader section = grid.getHeader(); HeaderRow mergeRow = section.prependRow(); diff --git a/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java b/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java index c3c373b5af..3dde4989b8 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridStaticSectionState.java @@ -37,6 +37,8 @@ public class GridStaticSectionState implements Serializable { public Connector connector = null; public GridStaticCellType type = GridStaticCellType.TEXT; + + public String columnId; } public static class RowState implements Serializable { @@ -44,7 +46,7 @@ public class GridStaticSectionState implements Serializable { public boolean defaultRow = false; - public List> cellGroups = new ArrayList>(); + public List> cellGroups = new ArrayList>(); } public List rows = new ArrayList(); -- 2.39.5