From f19ed54992d4f3ae7354ba7843b0a0ade0b593bd Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Tue, 9 Sep 2014 14:39:09 +0300 Subject: [PATCH] Fix client-side StaticRow to use GridColumn instead of index (#13334) Change-Id: I82e86c41de400e232fdf153379b8c40167bce438 --- .../src/com/vaadin/client/ui/grid/Grid.java | 47 +++--- .../vaadin/client/ui/grid/GridConnector.java | 7 +- .../client/ui/grid/GridStaticSection.java | 156 +++++++++--------- .../grid/GridBasicClientFeaturesWidget.java | 48 +++--- .../GridClientColumnRendererConnector.java | 25 ++- 5 files changed, 143 insertions(+), 140 deletions(-) diff --git a/client/src/com/vaadin/client/ui/grid/Grid.java b/client/src/com/vaadin/client/ui/grid/Grid.java index 5221284c3c..b0f97413ff 100644 --- a/client/src/com/vaadin/client/ui/grid/Grid.java +++ b/client/src/com/vaadin/client/ui/grid/Grid.java @@ -403,8 +403,7 @@ public class Grid extends Composite implements --newRow; break; case KeyCodes.KEY_RIGHT: - if (activeCellRange.getEnd() >= getVisibleColumnIndices() - .size()) { + if (activeCellRange.getEnd() >= getVisibleColumns().size()) { return; } ++newColumn; @@ -1163,13 +1162,11 @@ public class Grid extends Composite implements public void update(Row row, Iterable cellsToUpdate) { GridStaticSection.StaticRow staticRow = section.getRow(row .getRow()); - - final List columnIndices = getVisibleColumnIndices(); + final List> columns = getVisibleColumns(); for (FlyweightCell cell : cellsToUpdate) { - - int index = columnIndices.get(cell.getColumn()); - final StaticCell metadata = staticRow.getCell(index); + final StaticCell metadata = staticRow.getCell(columns.get(cell + .getColumn())); // Decorate default row with sorting indicators if (staticRow instanceof HeaderRow) { @@ -1256,11 +1253,11 @@ public class Grid extends Composite implements public void postAttach(Row row, Iterable attachedCells) { GridStaticSection.StaticRow gridRow = section.getRow(row .getRow()); - List columnIndices = getVisibleColumnIndices(); + List> columns = getVisibleColumns(); for (FlyweightCell cell : attachedCells) { - int index = columnIndices.get(cell.getColumn()); - StaticCell metadata = gridRow.getCell(index); + StaticCell metadata = gridRow.getCell(columns.get(cell + .getColumn())); /* * If the cell contains widgets that are not currently attach * then attach them now. @@ -1286,10 +1283,10 @@ public class Grid extends Composite implements if (section.getRowCount() > row.getRow()) { GridStaticSection.StaticRow gridRow = section.getRow(row .getRow()); - List columnIndices = getVisibleColumnIndices(); + List> columns = getVisibleColumns(); for (FlyweightCell cell : cellsToDetach) { - int index = columnIndices.get(cell.getColumn()); - StaticCell metadata = gridRow.getCell(index); + StaticCell metadata = gridRow.getCell(columns.get(cell + .getColumn())); if (GridStaticCellType.WIDGET.equals(metadata.getType()) && metadata.getWidget().isAttached()) { @@ -1529,8 +1526,8 @@ public class Grid extends Composite implements // Register column with grid columns.add(index, column); - header.addColumn(column, index); - footer.addColumn(column, index); + header.addColumn(column); + footer.addColumn(column); // Register this grid instance with the column ((AbstractGridColumn) column).setGrid(this); @@ -1631,8 +1628,8 @@ public class Grid extends Composite implements int visibleIndex = findVisibleColumnIndex(column); columns.remove(columnIndex); - header.removeColumn(columnIndex); - footer.removeColumn(columnIndex); + header.removeColumn(column); + footer.removeColumn(column); // de-register column with grid ((AbstractGridColumn) column).setGrid(null); @@ -1686,18 +1683,18 @@ public class Grid extends Composite implements } /** - * Returns a list of column indices that are currently visible. + * Returns a list of columns that are currently visible. * - * @return a list of indices + * @return a list of columns */ - private List getVisibleColumnIndices() { - List indices = new ArrayList(getColumnCount()); - for (int i = 0; i < getColumnCount(); i++) { - if (getColumn(i).isVisible()) { - indices.add(i); + protected List> getVisibleColumns() { + List> visible = new ArrayList>(); + for (GridColumn column : getColumns()) { + if (column.isVisible()) { + visible.add(column); } } - return indices; + return visible; } /** diff --git a/client/src/com/vaadin/client/ui/grid/GridConnector.java b/client/src/com/vaadin/client/ui/grid/GridConnector.java index 8153a68f9e..d9e6463d32 100644 --- a/client/src/com/vaadin/client/ui/grid/GridConnector.java +++ b/client/src/com/vaadin/client/ui/grid/GridConnector.java @@ -40,8 +40,6 @@ import com.vaadin.client.data.RpcDataSourceConnector.RpcDataSource; import com.vaadin.client.ui.AbstractFieldConnector; import com.vaadin.client.ui.AbstractHasComponentsConnector; import com.vaadin.client.ui.grid.GridHeader.HeaderRow; -import com.vaadin.client.ui.grid.GridStaticSection.StaticCell; -import com.vaadin.client.ui.grid.GridStaticSection.StaticRow; import com.vaadin.client.ui.grid.renderers.AbstractRendererConnector; import com.vaadin.client.ui.grid.selection.AbstractRowHandleSelectionModel; import com.vaadin.client.ui.grid.selection.SelectionChangeEvent; @@ -333,7 +331,7 @@ public class GridConnector extends AbstractHasComponentsConnector { } for (RowState rowState : state.rows) { - StaticRow row = section.appendRow(); + GridStaticSection.StaticRow row = section.appendRow(); int selectionOffset = 1; if (getWidget().getSelectionModel() instanceof SelectionModel.None) { @@ -345,7 +343,8 @@ public class GridConnector extends AbstractHasComponentsConnector { int i = 0 + selectionOffset; for (CellState cellState : rowState.cells) { - StaticCell cell = row.getCell(i++); + GridStaticSection.StaticCell cell = row.getCell(getWidget() + .getColumn(i++)); switch (cellState.type) { case TEXT: cell.setText(cellState.text); diff --git a/client/src/com/vaadin/client/ui/grid/GridStaticSection.java b/client/src/com/vaadin/client/ui/grid/GridStaticSection.java index 8c9ada46d0..05b809e156 100644 --- a/client/src/com/vaadin/client/ui/grid/GridStaticSection.java +++ b/client/src/com/vaadin/client/ui/grid/GridStaticSection.java @@ -18,8 +18,10 @@ package com.vaadin.client.ui.grid; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import com.google.gwt.user.client.ui.Widget; import com.vaadin.shared.ui.grid.GridStaticCellType; @@ -185,80 +187,63 @@ abstract class GridStaticSection> */ abstract static class StaticRow { - private List cells = new ArrayList(); + private Map, CELLTYPE> cells = new HashMap, CELLTYPE>(); private GridStaticSection section; - private Collection> cellGroups = new HashSet>(); + private Collection>> cellGroups = new HashSet>>(); /** - * Returns the cell at the given position in this row. + * Returns the cell on given GridColumn. * - * @param index - * the position of the cell - * @return the cell at the index - * @throws IndexOutOfBoundsException - * if the index is out of bounds + * @param column + * the column in grid + * @return the cell on given column, null if not found */ - public CELLTYPE getCell(int index) { - return cells.get(index); + public CELLTYPE getCell(GridColumn column) { + return cells.get(column); } /** - * Merges cells in a row + * Merges columns cells in a row * - * @param cells - * The cells to be merged - * @return The first cell of the merged cells + * @param columns + * the columns which header should be merged + * @return the remaining visible cell after the merge, or the cell on + * first column if all are hidden */ - 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."); - } + public CELLTYPE join(GridColumn... columns) { + if (columns.length <= 1) { + throw new IllegalArgumentException( + "You can't merge less than 2 columns together."); } - // Ensure continuous range - int firstCellIndex = this.cells.indexOf(cells.get(0)); - for (int i = 0; i < cells.size(); i++) { - if (this.cells.get(firstCellIndex + i) != cells.get(i)) { + final List columnList = section.grid.getColumns(); + int firstIndex = columnList.indexOf(columns[0]); + int i = 0; + for (GridColumn column : columns) { + if (!cells.containsKey(column)) { + throw new IllegalArgumentException( + "Given column does not exists on row " + column); + } else if (getCellGroupForColumn(column) != null) { + throw new IllegalStateException( + "Column is already in a group."); + } else if (!column.equals(columnList.get(firstIndex + (i++)))) { throw new IllegalStateException( - "Cell range must be a continous range"); + "Columns are in invalid order or not in a continuous range"); } } - // Create a new group - cellGroups.add(new ArrayList(cells)); + cellGroups.add(Arrays.asList(columns)); - // Calculates colspans, triggers refresh on section implicitly calculateColspans(); - // Returns first cell of group - return cells.get(0); - } - - /** - * Merges columns cells in a row - * - * @param columns - * The columns which header should be merged - * @return The remaining visible cell after the merge - */ - public CELLTYPE join(GridColumn... columns) { - assert columns.length > 1 : "You cannot merge less than 2 columns together"; - - // Convert columns to cells - List cells = new ArrayList(); - for (GridColumn c : columns) { - int index = getSection().getGrid().getColumns().indexOf(c); - cells.add(this.cells.get(index)); + for (i = 0; i < columns.length; ++i) { + if (columns[i].isVisible()) { + return getCell(columns[i]); + } } - - return join(cells); + return getCell(columns[0]); } /** @@ -266,15 +251,41 @@ abstract class GridStaticSection> * * @param cells * The cells to merge. Must be from the same row. - * @return The remaining visible cell after the merge + * @return The remaining visible cell after the merge, or the first cell + * if all columns are hidden */ public CELLTYPE join(CELLTYPE... cells) { - return join(Arrays.asList(cells)); + if (cells.length <= 1) { + throw new IllegalArgumentException( + "You can't merge less than 2 cells together."); + } + + GridColumn[] columns = new GridColumn[cells.length]; + + int j = 0; + for (GridColumn column : this.cells.keySet()) { + CELLTYPE cell = this.cells.get(column); + if (!this.cells.containsValue(cells[j])) { + throw new IllegalArgumentException( + "Given cell does not exists on row"); + } else if (cell.equals(cells[j])) { + columns[j++] = column; + if (j == cells.length) { + break; + } + } else if (j > 0) { + throw new IllegalStateException( + "Cells are in invalid order or not in a continuous range."); + } + } + + return join(columns); } - private List getCellGroupForCell(CELLTYPE cell) { - for (List group : cellGroups) { - if (group.contains(cell)) { + private List> getCellGroupForColumn( + GridColumn column) { + for (List> group : cellGroups) { + if (group.contains(column)) { return group; } } @@ -284,12 +295,12 @@ abstract class GridStaticSection> void calculateColspans() { // Reset all cells - for (CELLTYPE cell : cells) { + for (CELLTYPE cell : this.cells.values()) { cell.setColspan(1); } // Set colspan for grouped cells - for (List group : cellGroups) { + for (List> group : cellGroups) { int firstVisibleColumnInGroup = -1; int lastVisibleColumnInGroup = -1; @@ -306,11 +317,7 @@ abstract class GridStaticSection> * visible cell and how many cells are hidden in between. */ for (int i = 0; i < group.size(); i++) { - CELLTYPE cell = group.get(i); - int cellIndex = this.cells.indexOf(cell); - boolean columnVisible = getSection().getGrid() - .getColumn(cellIndex).isVisible(); - if (columnVisible) { + if (group.get(i).isVisible()) { lastVisibleColumnInGroup = i; if (firstVisibleColumnInGroup == -1) { firstVisibleColumnInGroup = i; @@ -330,22 +337,23 @@ abstract class GridStaticSection> /* * Assign colspan to first cell in group. */ - CELLTYPE firstVisibleCell = group + GridColumn firstVisibleColumn = group .get(firstVisibleColumnInGroup); + CELLTYPE firstVisibleCell = getCell(firstVisibleColumn); firstVisibleCell.setColspan(lastVisibleColumnInGroup - firstVisibleColumnInGroup - hiddenInsideGroup + 1); } } - protected void addCell(int index) { + protected void addCell(GridColumn column) { CELLTYPE cell = createCell(); cell.setSection(getSection()); - cells.add(index, cell); + cells.put(column, cell); } - protected void removeCell(int index) { - cells.remove(index); + protected void removeCell(GridColumn column) { + cells.remove(column); } protected abstract CELLTYPE createCell(); @@ -415,7 +423,7 @@ abstract class GridStaticSection> ROWTYPE row = createRow(); row.setSection(this); for (int i = 0; i < getGrid().getColumnCount(); ++i) { - row.addCell(i); + row.addCell(grid.getColumn(i)); } rows.add(index, row); @@ -509,15 +517,15 @@ abstract class GridStaticSection> return isVisible() ? getRowCount() : 0; } - protected void addColumn(GridColumn column, int index) { + protected void addColumn(GridColumn column) { for (ROWTYPE row : rows) { - row.addCell(index); + row.addCell(column); } } - protected void removeColumn(int index) { + protected void removeColumn(GridColumn column) { for (ROWTYPE row : rows) { - row.removeCell(index); + row.removeCell(column); } } diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java index fcf1723db0..0da8c1fc67 100644 --- a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java +++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java @@ -388,24 +388,22 @@ public class GridBasicClientFeaturesWidget extends for (int i = 0; i < COLUMNS; i++) { final int index = i; + final GridColumn> column = grid.getColumn(index); addMenuCommand("Visible", new ScheduledCommand() { @Override public void execute() { - grid.getColumn(index).setVisible( - !grid.getColumn(index).isVisible()); + column.setVisible(!column.isVisible()); } }, "Component", "Columns", "Column " + i); addMenuCommand("Sortable", new ScheduledCommand() { @Override public void execute() { - grid.getColumn(index).setSortable( - !grid.getColumn(index).isSortable()); + column.setSortable(!column.isSortable()); } }, "Component", "Columns", "Column " + i); addMenuCommand("Frozen", new ScheduledCommand() { @Override public void execute() { - GridColumn> column = grid.getColumn(index); if (column.equals(grid.getLastFrozenColumn())) { grid.setLastFrozenColumn(null); } else { @@ -417,19 +415,19 @@ public class GridBasicClientFeaturesWidget extends addMenuCommand("auto", new ScheduledCommand() { @Override public void execute() { - grid.getColumn(index).setWidth(-1); + column.setWidth(-1); } }, "Component", "Columns", "Column " + i, "Width"); addMenuCommand("50px", new ScheduledCommand() { @Override public void execute() { - grid.getColumn(index).setWidth(50); + column.setWidth(50); } }, "Component", "Columns", "Column " + i, "Width"); addMenuCommand("200px", new ScheduledCommand() { @Override public void execute() { - grid.getColumn(index).setWidth(200); + column.setWidth(200); } }, "Component", "Columns", "Column " + i, "Width"); @@ -437,14 +435,14 @@ public class GridBasicClientFeaturesWidget extends addMenuCommand("Text Header", new ScheduledCommand() { @Override public void execute() { - grid.getHeader().getRow(0).getCell(index) + grid.getHeader().getRow(0).getCell(column) .setText("Text Header"); } }, "Component", "Columns", "Column " + i, "Header Type"); addMenuCommand("HTML Header", new ScheduledCommand() { @Override public void execute() { - grid.getHeader().getRow(0).getCell(index) + grid.getHeader().getRow(0).getCell(column) .setHtml("HTML Header"); } }, "Component", "Columns", "Column " + i, "Header Type"); @@ -459,7 +457,8 @@ public class GridBasicClientFeaturesWidget extends button.setText("Clicked"); } }); - grid.getHeader().getRow(0).getCell(index).setWidget(button); + grid.getHeader().getRow(0).getCell(column) + .setWidget(button); } }, "Component", "Columns", "Column " + i, "Header Type"); @@ -467,14 +466,14 @@ public class GridBasicClientFeaturesWidget extends addMenuCommand("Text Footer", new ScheduledCommand() { @Override public void execute() { - grid.getFooter().getRow(0).getCell(index) + grid.getFooter().getRow(0).getCell(column) .setText("Text Footer"); } }, "Component", "Columns", "Column " + i, "Footer Type"); addMenuCommand("HTML Footer", new ScheduledCommand() { @Override public void execute() { - grid.getFooter().getRow(0).getCell(index) + grid.getFooter().getRow(0).getCell(column) .setHtml("HTML Footer"); } }, "Component", "Columns", "Column " + i, "Footer Type"); @@ -489,7 +488,8 @@ public class GridBasicClientFeaturesWidget extends button.setText("Clicked"); } }); - grid.getFooter().getRow(0).getCell(index).setWidget(button); + grid.getFooter().getRow(0).getCell(column) + .setWidget(button); } }, "Component", "Columns", "Column " + i, "Footer Type"); } @@ -504,11 +504,12 @@ public class GridBasicClientFeaturesWidget extends // Lets use some different cell types if (i % 3 == 0) { - row.getCell(i).setText(caption); + row.getCell(grid.getColumn(i)).setText(caption); } else if (i % 2 == 0) { - row.getCell(i).setHtml("" + caption + ""); + row.getCell(grid.getColumn(i)) + .setHtml("" + caption + ""); } else { - row.getCell(i).setWidget(new HTML(caption)); + row.getCell(grid.getColumn(i)).setWidget(new HTML(caption)); } } headerCounter++; @@ -520,11 +521,12 @@ public class GridBasicClientFeaturesWidget extends // Lets use some different cell types if (i % 3 == 0) { - row.getCell(i).setText(caption); + row.getCell(grid.getColumn(i)).setText(caption); } else if (i % 2 == 0) { - row.getCell(i).setHtml("" + caption + ""); + row.getCell(grid.getColumn(i)) + .setHtml("" + caption + ""); } else { - row.getCell(i).setWidget(new HTML(caption)); + row.getCell(grid.getColumn(i)).setWidget(new HTML(caption)); } } footerCounter++; @@ -597,7 +599,8 @@ public class GridBasicClientFeaturesWidget extends @Override public void execute() { - row.join(row.getCell(0), row.getCell(1)); + row.join(row.getCell(grid.getColumn(0)), + row.getCell(grid.getColumn(1))); } }, menuPath); @@ -712,7 +715,8 @@ public class GridBasicClientFeaturesWidget extends @Override public void execute() { - row.join(row.getCell(0), row.getCell(1)); + row.join(row.getCell(grid.getColumn(0)), + row.getCell(grid.getColumn(1))); } }, menuPath); diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java index c5571394bd..f4ca1d0344 100644 --- a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java +++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java @@ -140,12 +140,12 @@ public class GridClientColumnRendererConnector extends // Add a column to display the data in GridColumn c = createColumnWithRenderer(Renderers.TEXT_RENDERER); grid.addColumn(c); - grid.getHeader().getDefaultRow().getCell(0).setText("Column 1"); + grid.getHeader().getDefaultRow().getCell(c).setText("Column 1"); // Add another column with a custom complex renderer c = createColumnWithRenderer(Renderers.CPLX_RENDERER); grid.addColumn(c); - grid.getHeader().getDefaultRow().getCell(1).setText("Column 2"); + grid.getHeader().getDefaultRow().getCell(c).setText("Column 2"); // Add method for testing sort event firing grid.addSortHandler(new SortEventHandler() { @@ -156,10 +156,9 @@ public class GridClientColumnRendererConnector extends String text = "Client-side sort event received
" + "Columns: " + event.getOrder().size() + ", order: "; for (SortOrder order : event.getOrder()) { - int colIdx = getWidget().getColumns().indexOf( - order.getColumn()); String columnHeader = getWidget().getHeader() - .getDefaultRow().getCell(colIdx).getText(); + .getDefaultRow().getCell(order.getColumn()) + .getText(); text += columnHeader + ": " + order.getDirection().toString(); } @@ -174,24 +173,20 @@ public class GridClientColumnRendererConnector extends @Override public void addColumn(Renderers renderer) { + GridColumn column; if (renderer == Renderers.NUMBER_RENDERER) { - GridColumn numberColumn = createNumberColumnWithRenderer(renderer); - getWidget().addColumn(numberColumn); - + column = createNumberColumnWithRenderer(renderer); } else if (renderer == Renderers.DATE_RENDERER) { - GridColumn dateColumn = createDateColumnWithRenderer(renderer); - getWidget().addColumn(dateColumn); - + column = createDateColumnWithRenderer(renderer); } else { - GridColumn column = createColumnWithRenderer(renderer); - getWidget().addColumn(column); + column = createColumnWithRenderer(renderer); } + getWidget().addColumn(column); - int idx = getWidget().getColumnCount() - 1; getWidget() .getHeader() .getDefaultRow() - .getCell(idx) + .getCell(column) .setText( "Column " + String.valueOf(getWidget() -- 2.39.5