Fix client-side StaticRow to use GridColumn instead of index (#13334)

Change-Id: I82e86c41de400e232fdf153379b8c40167bce438

3 files changed, 107 insertions, 103 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<T> 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<T> extends Composite implements
         public void update(Row row, Iterable<FlyweightCell> cellsToUpdate) {
             GridStaticSection.StaticRow<?> staticRow = section.getRow(row
                     .getRow());
-
-            final List<Integer> columnIndices = getVisibleColumnIndices();
+            final List<GridColumn<?, T>> 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<T> extends Composite implements
         public void postAttach(Row row, Iterable<FlyweightCell> attachedCells) {
             GridStaticSection.StaticRow<?> gridRow = section.getRow(row
                     .getRow());
-            List<Integer> columnIndices = getVisibleColumnIndices();
+            List<GridColumn<?, T>> 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<T> extends Composite implements
             if (section.getRowCount() > row.getRow()) {
                 GridStaticSection.StaticRow<?> gridRow = section.getRow(row
                         .getRow());
-                List<Integer> columnIndices = getVisibleColumnIndices();
+                List<GridColumn<?, T>> 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<T> 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<?, T>) column).setGrid(this);
@@ -1631,8 +1628,8 @@ public class Grid<T> 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<?, T>) column).setGrid(null);
@@ -1686,18 +1683,18 @@ public class Grid<T> 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<Integer> getVisibleColumnIndices() {
-        List<Integer> indices = new ArrayList<Integer>(getColumnCount());
-        for (int i = 0; i < getColumnCount(); i++) {
-            if (getColumn(i).isVisible()) {
-                indices.add(i);
+    protected List<GridColumn<?, T>> getVisibleColumns() {
+        List<GridColumn<?, T>> visible = new ArrayList<GridColumn<?, T>>();
+        for (GridColumn<?, T> 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<ROWTYPE extends GridStaticSection.StaticRow<?>>
      */
     abstract static class StaticRow<CELLTYPE extends StaticCell> {
 
-        private List<CELLTYPE> cells = new ArrayList<CELLTYPE>();
+        private Map<GridColumn<?, ?>, CELLTYPE> cells = new HashMap<GridColumn<?, ?>, CELLTYPE>();
 
         private GridStaticSection<?> section;
 
-        private Collection<List<CELLTYPE>> cellGroups = new HashSet<List<CELLTYPE>>();
+        private Collection<List<GridColumn<?, ?>>> cellGroups = new HashSet<List<GridColumn<?, ?>>>();
 
         /**
-         * 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<CELLTYPE> 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<CELLTYPE>(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<CELLTYPE> cells = new ArrayList<CELLTYPE>();
-            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<ROWTYPE extends GridStaticSection.StaticRow<?>>
          * 
          * @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<CELLTYPE> getCellGroupForCell(CELLTYPE cell) {
-            for (List<CELLTYPE> group : cellGroups) {
-                if (group.contains(cell)) {
+        private List<GridColumn<?, ?>> getCellGroupForColumn(
+                GridColumn<?, ?> column) {
+            for (List<GridColumn<?, ?>> group : cellGroups) {
+                if (group.contains(column)) {
                     return group;
                 }
             }
@@ -284,12 +295,12 @@ abstract class GridStaticSection<ROWTYPE extends GridStaticSection.StaticRow<?>>
         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<CELLTYPE> group : cellGroups) {
+            for (List<GridColumn<?, ?>> group : cellGroups) {
 
                 int firstVisibleColumnInGroup = -1;
                 int lastVisibleColumnInGroup = -1;
@@ -306,11 +317,7 @@ abstract class GridStaticSection<ROWTYPE extends GridStaticSection.StaticRow<?>>
                  * 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<ROWTYPE extends GridStaticSection.StaticRow<?>>
                 /*
                  * 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 extends GridStaticSection.StaticRow<?>>
         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<ROWTYPE extends GridStaticSection.StaticRow<?>>
         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);
         }
     }