Add support for Grid column reordering (#13334)

Change-Id: I685cf0455810520e801cccdd46d8af838c8a3917

7 files changed, 201 insertions, 55 deletions

diff --git a/client/src/com/vaadin/client/ui/grid/Grid.java b/client/src/com/vaadin/client/ui/grid/Grid.java
index e307247a81..eb48d417d5 100644
--- a/client/src/com/vaadin/client/ui/grid/Grid.java
+++ b/client/src/com/vaadin/client/ui/grid/Grid.java
@@ -720,7 +720,7 @@ public class Grid<T> extends ResizeComposite implements
     /**
      * List of columns in the grid. Order defines the visible order.
      */
-    private final List<GridColumn<?, T>> columns = new ArrayList<GridColumn<?, T>>();
+    private List<GridColumn<?, T>> columns = new ArrayList<GridColumn<?, T>>();
 
     /**
      * The datasource currently in use. <em>Note:</em> it is <code>null</code>
@@ -1020,6 +1020,10 @@ public class Grid<T> extends ResizeComposite implements
             return width;
         }
 
+        void reapplyWidth() {
+            setWidth(width);
+        }
+
         /**
          * Enables sort indicators for the grid.
          * <p>
@@ -2933,4 +2937,45 @@ public class Grid<T> extends ResizeComposite implements
     public boolean isWorkPending() {
         return escalator.isWorkPending() || dataIsBeingFetched;
     }
+
+    /**
+     * Sets a new column order for the grid. All columns which are not ordered
+     * here will remain in the order they were before as the last columns of
+     * grid.
+     * 
+     * @param orderedColumns
+     *            array of columns in wanted order
+     */
+    public void setColumnOrder(GridColumn<?, T>... orderedColumns) {
+        List<GridColumn<?, T>> newOrder = new ArrayList<GridColumn<?, T>>();
+        if (selectionColumn != null) {
+            newOrder.add(selectionColumn);
+        }
+
+        int i = 0;
+        for (GridColumn<?, T> column : orderedColumns) {
+            if (columns.contains(column)) {
+                newOrder.add(column);
+                ++i;
+            } else {
+                throw new IllegalArgumentException("Given column at index " + i
+                        + " does not exist in Grid");
+            }
+        }
+
+        if (columns.size() != newOrder.size()) {
+            columns.removeAll(newOrder);
+            newOrder.addAll(columns);
+        }
+        columns = newOrder;
+
+        // Update column widths.
+        for (GridColumn<?, T> column : columns) {
+            column.reapplyWidth();
+        }
+
+        refreshHeader();
+        refreshBody();
+        refreshFooter();
+    }
 }
diff --git a/client/src/com/vaadin/client/ui/grid/GridConnector.java b/client/src/com/vaadin/client/ui/grid/GridConnector.java
index ca59745d3f..9f09e00c4d 100644
--- a/client/src/com/vaadin/client/ui/grid/GridConnector.java
+++ b/client/src/com/vaadin/client/ui/grid/GridConnector.java
@@ -242,6 +242,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
     private AbstractRowHandleSelectionModel<JSONObject> selectionModel = createSelectionModel(SharedSelectionMode.NONE);
     private Set<String> selectedKeys = new LinkedHashSet<String>();
+    private List<String> columnOrder = new ArrayList<String>();
 
     /**
      * updateFromState is set to true when {@link #updateSelectionFromState()}
@@ -349,27 +350,21 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         // Column updates
         if (stateChangeEvent.hasPropertyChanged("columns")) {
 
-            int totalColumns = getState().columns.size();
-
             // Remove old columns
             purgeRemovedColumns();
 
-            int currentColumns = getWidget().getColumnCount();
-            if (getWidget().getSelectionModel().getSelectionColumnRenderer() != null) {
-                currentColumns--;
-            }
-
             // Add new columns
-            for (int columnIndex = currentColumns; columnIndex < totalColumns; columnIndex++) {
-                addColumnFromStateChangeEvent(columnIndex);
+            for (GridColumnState state : getState().columns) {
+                if (!columnIdToColumn.containsKey(state.id)) {
+                    addColumnFromStateChangeEvent(state);
+                }
+                updateColumnFromState(columnIdToColumn.get(state.id), state);
             }
+        }
 
-            // Update old columns
-            for (int columnIndex = 0; columnIndex < currentColumns; columnIndex++) {
-                // FIXME Currently updating all column header / footers when a
-                // change in made in one column. When the framework supports
-                // quering a specific item in a list then it should do so here.
-                updateColumnFromStateChangeEvent(columnIndex);
+        if (stateChangeEvent.hasPropertyChanged("columnOrder")) {
+            if (orderNeedsUpdate(getState().columnOrder)) {
+                updateColumnOrderFromState(getState().columnOrder);
             }
         }
 
@@ -398,6 +393,29 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         }
     }
 
+    private void updateColumnOrderFromState(List<String> stateColumnOrder) {
+        CustomGridColumn[] columns = new CustomGridColumn[stateColumnOrder
+                .size()];
+        int i = 0;
+        for (String id : stateColumnOrder) {
+            columns[i++] = columnIdToColumn.get(id);
+        }
+        getWidget().setColumnOrder(columns);
+        columnOrder = stateColumnOrder;
+    }
+
+    private boolean orderNeedsUpdate(List<String> stateColumnOrder) {
+        if (stateColumnOrder.size() == columnOrder.size()) {
+            for (int i = 0; i < columnOrder.size(); ++i) {
+                if (!stateColumnOrder.get(i).equals(columnOrder.get(i))) {
+                    return true;
+                }
+            }
+            return false;
+        }
+        return true;
+    }
+
     private void updateSectionFromState(GridStaticSection<?> section,
             GridStaticSectionState state) {
 
@@ -453,27 +471,12 @@ public class GridConnector extends AbstractHasComponentsConnector implements
      * @param columnIndex
      *            The index of the column to update
      */
-    private void updateColumnFromStateChangeEvent(final int columnIndex) {
-        /*
-         * We use the widget column index here instead of the given column
-         * index. SharedState contains information only about the explicitly
-         * defined columns, while the widget counts the selection column as an
-         * explicit one.
-         */
-        GridColumn<?, JSONObject> column = getWidget().getColumn(
-                getWidgetColumnIndex(columnIndex));
-
-        GridColumnState columnState = getState().columns.get(columnIndex);
+    private void updateColumnFromStateChangeEvent(GridColumnState columnState) {
+        CustomGridColumn column = columnIdToColumn.get(columnState.id);
 
-        assert column instanceof CustomGridColumn : "column at index "
-                + columnIndex + " is not a "
-                + CustomGridColumn.class.getSimpleName() + ", but a "
-                + column.getClass().getSimpleName();
+        updateColumnFromState(column, columnState);
 
-        updateColumnFromState((CustomGridColumn) column, columnState);
-
-        if (columnState.rendererConnector != ((CustomGridColumn) column)
-                .getRendererConnector()) {
+        if (columnState.rendererConnector != column.getRendererConnector()) {
             throw new UnsupportedOperationException(
                     "Changing column renderer after initialization is currently unsupported");
         }
@@ -485,32 +488,17 @@ public class GridConnector extends AbstractHasComponentsConnector implements
      * @param columnIndex
      *            The index of the column, according to how it
      */
-    private void addColumnFromStateChangeEvent(int columnIndex) {
-        GridColumnState state = getState().columns.get(columnIndex);
+    private void addColumnFromStateChangeEvent(GridColumnState state) {
         @SuppressWarnings("unchecked")
         CustomGridColumn column = new CustomGridColumn(state.id,
                 ((AbstractRendererConnector<Object>) state.rendererConnector));
         columnIdToColumn.put(state.id, column);
 
         /*
-         * Adds a column to grid, and registers Grid with the column.
-         * 
-         * We use the widget column index here instead of the given column
-         * index. SharedState contains information only about the explicitly
-         * defined columns, while the widget counts the selection column as an
-         * explicit one.
-         */
-        getWidget().addColumn(column, getWidgetColumnIndex(columnIndex));
-
-        /*
-         * Have to update state _after_ the column has been added to the grid as
-         * then, and only then, the column will call the grid which in turn will
-         * call the escalator's refreshRow methods on header/footer/body and
-         * visually refresh the row. If this is done in the reverse order the
-         * first column state update will be lost as no grid instance is
-         * present.
+         * Add column to grid. Reordering is handled as a separate problem.
          */
-        updateColumnFromState(column, state);
+        getWidget().addColumn(column);
+        columnOrder.add(state.id);
     }
 
     /**
@@ -564,6 +552,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                 CustomGridColumn column = columnIdToColumn.get(id);
                 columnIdIterator.remove();
                 getWidget().removeColumn(column);
+                columnOrder.remove(id);
             }
         }
     }
diff --git a/server/src/com/vaadin/ui/components/grid/Grid.java b/server/src/com/vaadin/ui/components/grid/Grid.java
index 9170b712d0..4285b926c9 100644
--- a/server/src/com/vaadin/ui/components/grid/Grid.java
+++ b/server/src/com/vaadin/ui/components/grid/Grid.java
@@ -616,6 +616,7 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
         columns.put(datasourcePropertyId, column);
 
         getState().columns.add(columnState);
+        getState().columnOrder.add(columnState.id);
         header.addColumn(datasourcePropertyId);
         footer.addColumn(datasourcePropertyId);
 
@@ -625,10 +626,42 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
     }
 
     /**
+     * Sets a new column order for the grid. All columns which are not ordered
+     * here will remain in the order they were before as the last columns of
+     * grid.
+     * 
+     * @param propertyIds
+     *            properties in the order columns should be
+     */
+    public void setColumnOrder(Object... propertyIds) {
+        List<String> columnOrder = new ArrayList<String>();
+        for (Object propertyId : propertyIds) {
+            if (columns.containsKey(propertyId)) {
+                columnOrder.add(columnKeys.key(propertyId));
+            } else {
+                throw new IllegalArgumentException(
+                        "Grid does not contain column for property "
+                                + String.valueOf(propertyId));
+            }
+        }
+
+        List<String> stateColumnOrder = getState().columnOrder;
+        if (stateColumnOrder.size() != columnOrder.size()) {
+            stateColumnOrder.removeAll(columnOrder);
+            columnOrder.addAll(stateColumnOrder);
+        }
+        getState().columnOrder = columnOrder;
+    }
+
+    /**
      * Sets (or unsets) the rightmost frozen column in the grid.
      * <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.
      * 
      * @param lastFrozenColumn
      *            the rightmost column to freeze, or <code>null</code> to not
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 413a31be81..a2e892b715 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
@@ -24,6 +24,8 @@ import static org.junit.Assert.fail;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.util.LinkedHashSet;
+import java.util.Set;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -204,6 +206,35 @@ public class GridColumns {
         assertNull(grid.getLastFrozenPropertyId());
     }
 
+    @Test
+    public void testReorderColumns() {
+        Set<?> containerProperties = new LinkedHashSet<Object>(grid
+                .getContainerDatasource().getContainerPropertyIds());
+        Object[] properties = new Object[] { "column3", "column2", "column6" };
+        grid.setColumnOrder(properties);
+
+        int i = 0;
+        // Test sorted columns are first in order
+        for (Object property : properties) {
+            containerProperties.remove(property);
+            assertEquals(columnIdMapper.key(property),
+                    state.columnOrder.get(i++));
+        }
+
+        // Test remaining columns are in original order
+        for (Object property : containerProperties) {
+            assertEquals(columnIdMapper.key(property),
+                    state.columnOrder.get(i++));
+        }
+
+        try {
+            grid.setColumnOrder("foo", "bar", "baz");
+            fail("Grid allowed sorting with non-existent properties");
+        } catch (IllegalArgumentException e) {
+            // All ok
+        }
+    }
+
     private GridColumnState getColumnState(Object propertyId) {
         String columnId = columnIdMapper.key(propertyId);
         for (GridColumnState columnState : state.columns) {
diff --git a/shared/src/com/vaadin/shared/ui/grid/GridState.java b/shared/src/com/vaadin/shared/ui/grid/GridState.java
index 6a0b5836a7..934ba25884 100644
--- a/shared/src/com/vaadin/shared/ui/grid/GridState.java
+++ b/shared/src/com/vaadin/shared/ui/grid/GridState.java
@@ -88,10 +88,15 @@ public class GridState extends AbstractComponentState {
     public static final String JSONKEY_ROWKEY = "k";
 
     /**
-     * Columns in grid. Column order implicitly deferred from list order.
+     * Columns in grid.
      */
     public List<GridColumnState> columns = new ArrayList<GridColumnState>();
 
+    /**
+     * Column order in grid.
+     */
+    public List<String> columnOrder = new ArrayList<String>();
+
     public GridStaticSectionState header = new GridStaticSectionState();
 
     public GridStaticSectionState footer = new GridStaticSectionState();
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 ae8a0d5eb8..cdf3508bea 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
@@ -18,6 +18,7 @@ package com.vaadin.tests.components.grid.basicfeatures;
 import java.text.DecimalFormat;
 import java.text.DecimalFormatSymbols;
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -255,6 +256,25 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                         grid.setSortOrder(sortOrder);
                     }
                 });
+
+        createBooleanAction("Reverse Grid Columns", "State", false,
+                new Command<Grid, Boolean>() {
+
+                    @Override
+                    public void execute(Grid c, Boolean value, Object data) {
+                        List<Object> ids = new ArrayList<Object>();
+                        ids.addAll(ds.getContainerPropertyIds());
+                        if (!value) {
+                            c.setColumnOrder(ids.toArray());
+                        } else {
+                            Object[] idsArray = new Object[ids.size()];
+                            for (int i = 0; i < ids.size(); ++i) {
+                                idsArray[i] = ids.get((ids.size() - 1) - i);
+                            }
+                            c.setColumnOrder(idsArray);
+                        }
+                    }
+                });
     }
 
     protected void createHeaderActions() {
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 0359a95756..b771185167 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
@@ -29,6 +29,7 @@ import org.openqa.selenium.WebElement;
 
 import com.vaadin.testbench.TestBenchElement;
 import com.vaadin.testbench.elements.NotificationElement;
+import com.vaadin.tests.components.grid.GridElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
 
@@ -242,6 +243,28 @@ public class GridStructureTest extends GridBasicFeaturesTest {
                 isElementPresent(By.xpath("//th[text()='" + columnName + "']")));
     }
 
+    @Test
+    public void testReverseColumns() {
+        openTestURL();
+
+        String[] gridData = new String[GridBasicFeatures.COLUMNS];
+        GridElement grid = getGridElement();
+        for (int i = 0; i < gridData.length; ++i) {
+            gridData[i] = grid.getCell(0, i).getAttribute("innerHTML");
+        }
+
+        selectMenuPath("Component", "State", "Reverse Grid Columns");
+
+        // Compare with reversed order
+        for (int i = 0; i < gridData.length; ++i) {
+            final int column = gridData.length - 1 - i;
+            final String newText = grid.getCell(0, column).getAttribute(
+                    "innerHTML");
+            assertEquals("Grid contained unexpected values. (0, " + column
+                    + ")", gridData[i], newText);
+        }
+    }
+
     private boolean verticalScrollbarIsPresent() {
         return "scroll".equals(getGridVerticalScrollbar().getCssValue(
                 "overflow-y"));