Refactor frozen column API (#13334)

Change-Id: I0c9528d2a4b2de2bcd5a6a6e70b1821eb142c4cc

10 files changed, 135 insertions, 162 deletions

diff --git a/client/src/com/vaadin/client/ui/grid/Escalator.java b/client/src/com/vaadin/client/ui/grid/Escalator.java
index a98daf456b..c9df2d5efb 100644
--- a/client/src/com/vaadin/client/ui/grid/Escalator.java
+++ b/client/src/com/vaadin/client/ui/grid/Escalator.java
@@ -3871,7 +3871,7 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
             if (count < 0 || count > getColumnCount()) {
                 throw new IllegalArgumentException(
                         "count must be between 0 and the current number of columns ("
-                                + columns + ")");
+                                + getColumnCount() + ")");
             }
             int oldCount = frozenColumns;
             if (count == oldCount) {
diff --git a/client/src/com/vaadin/client/ui/grid/Grid.java b/client/src/com/vaadin/client/ui/grid/Grid.java
index a7e74bb216..d98927ce18 100644
--- a/client/src/com/vaadin/client/ui/grid/Grid.java
+++ b/client/src/com/vaadin/client/ui/grid/Grid.java
@@ -774,9 +774,10 @@ public class Grid<T> extends ResizeComposite implements
     private Range currentDataAvailable = Range.withLength(0, 0);
 
     /**
-     * The last column frozen counter from the left
+     * The number of frozen columns, 0 freezes the selection column if
+     * displayed, -1 also prevents selection col from freezing.
      */
-    private GridColumn<?, T> lastFrozenColumn;
+    private int frozenColumnCount = 0;
 
     /**
      * Current sort order. The (private) sort() method reads this list to
@@ -1742,10 +1743,6 @@ public class Grid<T> extends ResizeComposite implements
         // Reapply column width
         column.reapplyWidth();
 
-        if (lastFrozenColumn != null && indexOfColumn(lastFrozenColumn) < index) {
-            refreshFrozenColumns();
-        }
-
         // Sink all renderer events
         Set<String> events = new HashSet<String>();
         events.addAll(getConsumedEventsForRenderer(column.getRenderer()));
@@ -1800,11 +1797,7 @@ public class Grid<T> extends ResizeComposite implements
         // Remove from column configuration
         escalator.getColumnConfiguration().removeColumns(columnIndex, 1);
 
-        if (column.equals(lastFrozenColumn)) {
-            setLastFrozenColumn(null);
-        } else {
-            refreshFrozenColumns();
-        }
+        updateFrozenColumns();
 
         header.removeColumn(column);
         footer.removeColumn(column);
@@ -1996,49 +1989,55 @@ public class Grid<T> extends ResizeComposite implements
     }
 
     /**
-     * Sets the rightmost frozen column in the grid.
+     * Sets the number of frozen columns in this grid. Setting the count to 0
+     * means that no data columns will be frozen, but the built-in selection
+     * checkbox column will still be frozen if it's in use. Setting the count to
+     * -1 will also disable the selection column.
      * <p>
-     * All columns up to and including the given column will be frozen in place
-     * when the grid is scrolled sideways.
+     * The default value is 0.
+     * 
+     * @param numberOfColumns
+     *            the number of columns that should be frozen
      * 
-     * @param lastFrozenColumn
-     *            the rightmost column to freeze, or <code>null</code> to not
-     *            have any columns frozen
      * @throws IllegalArgumentException
-     *             if {@code lastFrozenColumn} is not a column from this grid
+     *             if the column count is < -1 or > the number of visible
+     *             columns
      */
-    public void setLastFrozenColumn(GridColumn<?, T> lastFrozenColumn) {
-        this.lastFrozenColumn = lastFrozenColumn;
-        refreshFrozenColumns();
+    public void setFrozenColumnCount(int numberOfColumns) {
+        if (numberOfColumns < -1 || numberOfColumns > getColumnCount()) {
+            throw new IllegalArgumentException(
+                    "count must be between -1 and the current number of columns ("
+                            + getColumnCount() + ")");
+        }
+
+        this.frozenColumnCount = numberOfColumns;
+        updateFrozenColumns();
     }
 
-    private void refreshFrozenColumns() {
-        final int frozenCount;
-        if (lastFrozenColumn != null) {
-            frozenCount = columns.indexOf(lastFrozenColumn) + 1;
-            if (frozenCount == 0) {
-                throw new IllegalArgumentException(
-                        "The given column isn't attached to this grid");
-            }
-        } else {
-            frozenCount = 0;
+    private void updateFrozenColumns() {
+        int numberOfColumns = frozenColumnCount;
+
+        if (numberOfColumns == -1) {
+            numberOfColumns = 0;
+        } else if (selectionColumn != null) {
+            numberOfColumns++;
         }
 
-        escalator.getColumnConfiguration().setFrozenColumnCount(frozenCount);
+        escalator.getColumnConfiguration()
+                .setFrozenColumnCount(numberOfColumns);
+
     }
 
     /**
-     * Gets the rightmost frozen column in the grid.
-     * <p>
-     * <em>Note:</em> Most usually, this method returns the very value set with
-     * {@link #setLastFrozenColumn(GridColumn)}. This value, however, can be
-     * reset to <code>null</code> if the column is removed from this grid.
+     * Gets the number of frozen columns in this grid. 0 means that no data
+     * columns will be frozen, but the built-in selection checkbox column will
+     * still be frozen if it's in use. -1 means that not even the selection
+     * column is frozen.
      * 
-     * @return the rightmost frozen column in the grid, or <code>null</code> if
-     *         no columns are frozen.
+     * @return the number of frozen columns
      */
-    public GridColumn<?, T> getLastFrozenColumn() {
-        return lastFrozenColumn;
+    public int getFrozenColumnCount() {
+        return frozenColumnCount;
     }
 
     public HandlerRegistration addRowVisibilityChangeHandler(
@@ -2672,7 +2671,11 @@ public class Grid<T> extends ResizeComposite implements
         }
 
         if (this.selectColumnRenderer != null) {
-            removeColumnSkipSelectionColumnCheck(selectionColumn);
+            // Clear field so frozen column logic in the remove method knows
+            // what to do
+            GridColumn<?, T> colToRemove = selectionColumn;
+            selectionColumn = null;
+            removeColumnSkipSelectionColumnCheck(colToRemove);
             cellFocusHandler.offsetRangeBy(-1);
         }
 
@@ -2690,6 +2693,8 @@ public class Grid<T> extends ResizeComposite implements
             selectionColumn = null;
             refreshBody();
         }
+
+        updateFrozenColumns();
     }
 
     /**
diff --git a/client/src/com/vaadin/client/ui/grid/GridConnector.java b/client/src/com/vaadin/client/ui/grid/GridConnector.java
index ba674c829e..6aa32abef4 100644
--- a/client/src/com/vaadin/client/ui/grid/GridConnector.java
+++ b/client/src/com/vaadin/client/ui/grid/GridConnector.java
@@ -451,19 +451,6 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                             getState().footer);
                 }
 
-                if (stateChangeEvent.hasPropertyChanged("lastFrozenColumnId")) {
-                    String frozenColId = getState().lastFrozenColumnId;
-                    if (frozenColId != null) {
-                        CustomGridColumn column = columnIdToColumn
-                                .get(frozenColId);
-                        assert column != null : "Column to be frozen could not be found (id:"
-                                + frozenColId + ")";
-                        getWidget().setLastFrozenColumn(column);
-                    } else {
-                        getWidget().setLastFrozenColumn(null);
-                    }
-                }
-
                 if (stateChangeEvent.hasPropertyChanged("editorRowEnabled")) {
                     getWidget().getEditorRow().setEnabled(
                             getState().editorRowEnabled);
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 1453178cf6..34d8a16807 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -1119,11 +1119,12 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
          * 
          * @throws IllegalArgumentException
          *             if the column is no longer attached to any grid
-         * @see Grid#setLastFrozenColumn(Column)
+         * @see Grid#setFrozenColumnCount(int)
          */
         public Column setLastFrozenColumn() {
             checkColumnIsAttached();
-            grid.setLastFrozenColumn(this);
+            grid.setFrozenColumnCount(grid.getState(false).columnOrder
+                    .indexOf(this) + 1);
             return this;
         }
 
@@ -1903,10 +1904,8 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
             }
             datasourceExtension.propertiesAdded(addedPropertyIds);
 
-            Object frozenPropertyId = columnKeys
-                    .get(getState(false).lastFrozenColumnId);
-            if (!columns.containsKey(frozenPropertyId)) {
-                setLastFrozenPropertyId(null);
+            if (getFrozenColumnCount() > columns.size()) {
+                setFrozenColumnCount(columns.size());
             }
 
             // Update sortable columns
@@ -2282,7 +2281,7 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
          * ValueChangeListeners at this point.
          */
 
-        setLastFrozenPropertyId(null);
+        setFrozenColumnCount(0);
 
         if (columns.isEmpty()) {
             // Add columns
@@ -2533,82 +2532,41 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
     }
 
     /**
-     * Sets (or unsets) the rightmost frozen column in the grid.
+     * Sets the number of frozen columns in this grid. Setting the count to 0
+     * means that no data columns will be frozen, but the built-in selection
+     * checkbox column will still be frozen if it's in use. Setting the count to
+     * -1 will also disable the selection column.
      * <p>
-     * All columns up to and including the given column will be frozen in place
-     * when the grid is scrolled sideways.
-     * <p>
-     * Reordering columns in the grid while there is a frozen column will make
-     * all columns frozen that are before the frozen column. ie. If you move the
-     * frozen column to be last, all columns will be frozen.
+     * The default value is 0.
+     * 
+     * @param numberOfColumns
+     *            the number of columns that should be frozen
      * 
-     * @param lastFrozenColumn
-     *            the rightmost column to freeze, or <code>null</code> to not
-     *            have any columns frozen
      * @throws IllegalArgumentException
-     *             if {@code lastFrozenColumn} is not a column from this grid
+     *             if the column count is < 0 or > the number of visible columns
      */
-    void setLastFrozenColumn(Column lastFrozenColumn) {
-        /*
-         * TODO: If and when Grid supports column reordering or insertion of
-         * columns before other columns, make sure to mention that adding
-         * columns before lastFrozenColumn will change the frozen column count
-         */
-
-        if (lastFrozenColumn == null) {
-            getState().lastFrozenColumnId = null;
-        } else if (columns.containsValue(lastFrozenColumn)) {
-            getState().lastFrozenColumnId = lastFrozenColumn.getState().id;
-        } else {
+    public void setFrozenColumnCount(int numberOfColumns) {
+        if (numberOfColumns < -1 || numberOfColumns > columns.size()) {
             throw new IllegalArgumentException(
-                    "The given column isn't attached to this grid");
+                    "count must be between -1 and the current number of columns ("
+                            + columns + ")");
         }
-    }
 
-    /**
-     * Sets (or unsets) the rightmost frozen column in the grid.
-     * <p>
-     * All columns up to and including the indicated property will be frozen in
-     * place when the grid is scrolled sideways.
-     * <p>
-     * <em>Note:</em> If the container used by this grid supports a propertyId
-     * <code>null</code>, it can never be defined as the last frozen column, as
-     * a <code>null</code> parameter will always reset the frozen columns in
-     * Grid.
-     * 
-     * @param propertyId
-     *            the property id corresponding to the column that should be the
-     *            last frozen column, or <code>null</code> to not have any
-     *            columns frozen.
-     * @throws IllegalArgumentException
-     *             if {@code lastFrozenColumn} is not a column from this grid
-     */
-    public void setLastFrozenPropertyId(Object propertyId) {
-        final Column column;
-        if (propertyId == null) {
-            column = null;
-        } else {
-            column = getColumn(propertyId);
-            if (column == null) {
-                throw new IllegalArgumentException(
-                        "property id does not exist.");
-            }
-        }
-        setLastFrozenColumn(column);
+        getState().frozenColumnCount = numberOfColumns;
     }
 
     /**
-     * Gets the rightmost frozen column in the grid.
-     * <p>
-     * <em>Note:</em> Most often, this method returns the very value set with
-     * {@link #setLastFrozenPropertyId(Object)}. This value, however, can be
-     * reset to <code>null</code> if the column is detached from this grid.
+     * Gets the number of frozen columns in this grid. 0 means that no data
+     * columns will be frozen, but the built-in selection checkbox column will
+     * still be frozen if it's in use. -1 means that not even the selection
+     * column is frozen.
+     * 
+     * @see #setFrozenColumnCount(int)
      * 
-     * @return the rightmost frozen column in the grid, or <code>null</code> if
-     *         no columns are frozen.
+     * @return the number of frozen columns
      */
-    public Object getLastFrozenPropertyId() {
-        return columnKeys.get(getState().lastFrozenColumnId);
+    public int getFrozenColumnCount() {
+        return getState(false).frozenColumnCount;
     }
 
     /**
diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java b/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java
index f18eeb42c4..e9b33ba879 100644
--- a/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java
+++ b/server/tests/src/com/vaadin/tests/server/component/grid/GridColumns.java
@@ -180,17 +180,21 @@ public class GridColumns {
     }
 
     @Test
-    public void testFrozenColumnByPropertyId() {
-        assertNull("Grid should not start with a frozen column",
-                grid.getLastFrozenPropertyId());
+    public void testFrozenColumnRemoveColumn() {
+        assertEquals("Grid should not start with a frozen column", 0,
+                grid.getFrozenColumnCount());
+
+        int containerSize = grid.getContainerDataSource()
+                .getContainerPropertyIds().size();
+        grid.setFrozenColumnCount(containerSize);
 
         Object propertyId = grid.getContainerDataSource()
                 .getContainerPropertyIds().iterator().next();
-        grid.setLastFrozenPropertyId(propertyId);
-        assertEquals(propertyId, grid.getLastFrozenPropertyId());
 
         grid.getContainerDataSource().removeContainerProperty(propertyId);
-        assertNull(grid.getLastFrozenPropertyId());
+        assertEquals(
+                "Frozen column count should update when removing last row",
+                containerSize - 1, grid.getFrozenColumnCount());
     }
 
     @Test
diff --git a/shared/src/com/vaadin/shared/ui/grid/GridState.java b/shared/src/com/vaadin/shared/ui/grid/GridState.java
index 621b34a2b4..c30ebae474 100644
--- a/shared/src/com/vaadin/shared/ui/grid/GridState.java
+++ b/shared/src/com/vaadin/shared/ui/grid/GridState.java
@@ -115,12 +115,9 @@ public class GridState extends AbstractComponentState {
 
     public GridStaticSectionState footer = new GridStaticSectionState();
 
-    /**
-     * The id for the last frozen column.
-     * 
-     * @see GridColumnState#id
-     */
-    public String lastFrozenColumnId = null;
+    /** The number of frozen columns */
+    @DelegateToWidget
+    public int frozenColumnCount = 0;
 
     /** The height of the Grid in terms of body rows. */
     @DelegateToWidget
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
index 2e0ac5a5cf..f1eabc57fd 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
@@ -338,6 +338,18 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                         grid.setCellStyleGenerator(generator);
                     }
                 });
+
+        LinkedHashMap<String, Integer> frozenOptions = new LinkedHashMap<String, Integer>();
+        for (int i = -1; i <= COLUMNS; i++) {
+            frozenOptions.put(String.valueOf(i), Integer.valueOf(i));
+        }
+        createSelectAction("Frozen column count", "State", frozenOptions, "0",
+                new Command<Grid, Integer>() {
+                    @Override
+                    public void execute(Grid c, Integer value, Object data) {
+                        c.setFrozenColumnCount(value.intValue());
+                    }
+                });
     }
 
     protected void createHeaderActions() {
@@ -464,6 +476,7 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
         createCategory("Columns", null);
 
         for (int c = 0; c < COLUMNS; c++) {
+            final int index = c;
             createCategory(getColumnProperty(c), "Columns");
 
             createClickAction("Add / Remove", getColumnProperty(c),
@@ -480,15 +493,6 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                         }
                     }, null, c);
 
-            createClickAction("Freeze", getColumnProperty(c),
-                    new Command<Grid, String>() {
-
-                        @Override
-                        public void execute(Grid grid, String value, Object data) {
-                            grid.setLastFrozenPropertyId(getColumnProperty((Integer) data));
-                        }
-                    }, null, c);
-
             createBooleanAction("Sortable", getColumnProperty(c), true,
                     new Command<Grid, Boolean>() {
 
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridClientColumnPropertiesTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridClientColumnPropertiesTest.java
index ea46ee24a5..4aff236f91 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridClientColumnPropertiesTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridClientColumnPropertiesTest.java
@@ -65,10 +65,25 @@ public class GridClientColumnPropertiesTest extends GridBasicClientFeaturesTest
         assertFalse(cellIsFrozen(0, 0));
         assertFalse(cellIsFrozen(0, 1));
 
-        selectMenuPath("Component", "Columns", "Column 0", "Frozen");
+        selectMenuPath("Component", "State", "Frozen column count", "1 columns");
 
         assertTrue(cellIsFrozen(1, 0));
         assertFalse(cellIsFrozen(1, 1));
+
+        selectMenuPath("Component", "State", "Selection mode", "multi");
+
+        assertTrue(cellIsFrozen(1, 1));
+        assertFalse(cellIsFrozen(1, 2));
+
+        selectMenuPath("Component", "State", "Frozen column count", "0 columns");
+
+        assertTrue(cellIsFrozen(1, 0));
+        assertFalse(cellIsFrozen(1, 1));
+
+        selectMenuPath("Component", "State", "Frozen column count",
+                "-1 columns");
+
+        assertFalse(cellIsFrozen(1, 0));
     }
 
     private boolean cellIsFrozen(int row, int col) {
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java
index 9f1d61124e..054c584d28 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStructureTest.java
@@ -106,14 +106,14 @@ public class GridStructureTest extends GridBasicFeaturesTest {
     public void testFreezingColumn() throws Exception {
         openTestURL();
 
-        // Freeze column 2
-        selectMenuPath("Component", "Columns", "Column 2", "Freeze");
+        // Freeze column 1
+        selectMenuPath("Component", "State", "Frozen column count", "1");
 
         WebElement cell = getGridElement().getCell(0, 0);
         assertTrue(cell.getAttribute("class").contains("frozen"));
 
         cell = getGridElement().getCell(0, 1);
-        assertTrue(cell.getAttribute("class").contains("frozen"));
+        assertFalse(cell.getAttribute("class").contains("frozen"));
     }
 
     @Test
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 8a4bbeed09..ce899be8f9 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
@@ -572,6 +572,19 @@ public class GridBasicClientFeaturesWidget extends
                 });
             }
         }, styleGeneratorNamePath);
+
+        for (int i = -1; i <= COLUMNS; i++) {
+            final int index = i;
+            // Including dummy "columns" prefix because TB fails to select item
+            // if it's too narrow
+            addMenuCommand(Integer.toString(index) + " columns",
+                    new ScheduledCommand() {
+                        @Override
+                        public void execute() {
+                            grid.setFrozenColumnCount(index);
+                        }
+                    }, "Component", "State", "Frozen column count");
+        }
     }
 
     private void createColumnsMenu() {
@@ -585,16 +598,6 @@ public class GridBasicClientFeaturesWidget extends
                     column.setSortable(!column.isSortable());
                 }
             }, "Component", "Columns", "Column " + i);
-            addMenuCommand("Frozen", new ScheduledCommand() {
-                @Override
-                public void execute() {
-                    if (column.equals(grid.getLastFrozenColumn())) {
-                        grid.setLastFrozenColumn(null);
-                    } else {
-                        grid.setLastFrozenColumn(column);
-                    }
-                }
-            }, "Component", "Columns", "Column " + i);
 
             addMenuCommand("auto", new ScheduledCommand() {
                 @Override