Grid column hiding info from client to server #(17023)

Fixes mismatched client-server API regarding ColumnVisibilityChangeEvent
Adds and removes the column hiding toggle as needed when columns added / removed.
Known bug when a hidable column added, column toggle won't get the caption of column.

Change-Id: I708e19432dc822f713bf11f5b8e6eadb528a3961

8 files changed, 278 insertions, 23 deletions

diff --git a/WebContent/VAADIN/themes/base/grid/grid.scss b/WebContent/VAADIN/themes/base/grid/grid.scss
index 87b936a1b9..ccb7043c50 100644
--- a/WebContent/VAADIN/themes/base/grid/grid.scss
+++ b/WebContent/VAADIN/themes/base/grid/grid.scss
@@ -84,16 +84,17 @@ $v-grid-editor-background-color: $v-grid-row-background-color !default;
   .#{$primaryStyleName}-sidebar {
     position: absolute;
     top: 1px;
-    right : 0;
-    
+    right : 1px;
+
     background-color: $v-grid-header-background-color;
     border-left: $v-grid-header-border;
     border-bottom: $v-grid-header-border;    
     z-index: 5;
-    
+
     .#{$primaryStyleName}-sidebar-button {
       height: $v-grid-header-row-height;
-      
+      text-align: right;
+
       &:after {
         content: "\f0c9";
         font-family: FontAwesome, sans-serif;
@@ -102,6 +103,17 @@ $v-grid-editor-background-color: $v-grid-row-background-color !default;
         padding: 0 $v-grid-cell-padding-horizontal;
       }
     }
+
+    .#{$primaryStyleName}-sidebar-content {
+      
+      .column-hiding-panel {
+        display: block;
+        .column-hiding-toggle {
+          display: block;
+          padding: 3px 12px;
+        }
+      }      
+    }
   }
 
   // Common cell styles
diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
index 0807690023..7c568e02e5 100644
--- a/client/src/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -56,6 +56,8 @@ import com.vaadin.client.widget.grid.events.BodyClickHandler;
 import com.vaadin.client.widget.grid.events.BodyDoubleClickHandler;
 import com.vaadin.client.widget.grid.events.ColumnReorderEvent;
 import com.vaadin.client.widget.grid.events.ColumnReorderHandler;
+import com.vaadin.client.widget.grid.events.ColumnVisibilityChangeEvent;
+import com.vaadin.client.widget.grid.events.ColumnVisibilityChangeHandler;
 import com.vaadin.client.widget.grid.events.GridClickEvent;
 import com.vaadin.client.widget.grid.events.GridDoubleClickEvent;
 import com.vaadin.client.widget.grid.events.SelectAllEvent;
@@ -388,6 +390,33 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         }
     };
 
+    private ColumnVisibilityChangeHandler<JsonObject> columnVisibilityChangeHandler = new ColumnVisibilityChangeHandler<JsonObject>() {
+
+        @Override
+        public void onVisibilityChange(
+                ColumnVisibilityChangeEvent<JsonObject> event) {
+            if (!columnsUpdatedFromState) {
+                Column<?, JsonObject> column = event.getColumn();
+                if (column instanceof CustomGridColumn) {
+                    getRpcProxy(GridServerRpc.class).columnVisibilityChanged(
+                            ((CustomGridColumn) column).id, column.isHidden(),
+                            event.isUserOriginated());
+                    for (GridColumnState state : getState().columns) {
+                        if (state.id.equals(((CustomGridColumn) column).id)) {
+                            state.hidden = event.isHidden();
+                            break;
+                        }
+                    }
+                } else {
+                    getLogger().warning(
+                            "Visibility changed for a unknown column type in Grid: "
+                                    + column.toString() + ", type "
+                                    + column.getClass());
+                }
+            }
+        }
+    };
+
     private static class CustomDetailsGenerator implements DetailsGenerator {
 
         private final Map<Integer, ComponentConnector> indexToDetailsMap = new HashMap<Integer, ComponentConnector>();
@@ -713,6 +742,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
         getWidget().setEditorHandler(new CustomEditorHandler());
         getWidget().addColumnReorderHandler(columnReorderHandler);
+        getWidget().addColumnVisibilityChangeHandler(
+                columnVisibilityChangeHandler);
         getWidget().setDetailsGenerator(customDetailsGenerator);
         getLayoutManager().registerDependency(this, getWidget().getElement());
 
@@ -734,7 +765,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                 if (!columnIdToColumn.containsKey(state.id)) {
                     addColumnFromStateChangeEvent(state);
                 }
-                updateColumnFromState(columnIdToColumn.get(state.id), state);
+                updateColumnFromStateChangeEvent(state);
             }
         }
 
@@ -947,7 +978,9 @@ public class GridConnector extends AbstractHasComponentsConnector implements
     private void updateColumnFromStateChangeEvent(GridColumnState columnState) {
         CustomGridColumn column = columnIdToColumn.get(columnState.id);
 
+        columnsUpdatedFromState = true;
         updateColumnFromState(column, columnState);
+        columnsUpdatedFromState = false;
 
         if (columnState.rendererConnector != column.getRendererConnector()) {
             throw new UnsupportedOperationException(
diff --git a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java
index 10bfbfad68..4c25f7a61b 100644
--- a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java
+++ b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java
@@ -60,7 +60,7 @@ public class ColumnVisibilityChangeEvent<T> extends
     }
 
     /**
-     * Is the column hidden or visible.
+     * Was the column set hidden or visible.
      * 
      * @return <code>true</code> if the column was hidden <code>false</code> if
      *         it was set visible
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 3e201277a0..3b26c8be57 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -68,7 +68,6 @@ import com.google.gwt.user.client.ui.HasEnabled;
 import com.google.gwt.user.client.ui.HasWidgets;
 import com.google.gwt.user.client.ui.ResizeComposite;
 import com.google.gwt.user.client.ui.ToggleButton;
-import com.google.gwt.user.client.ui.VerticalPanel;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.BrowserInfo;
 import com.vaadin.client.DeferredWorker;
@@ -1926,7 +1925,8 @@ public class Grid<T> extends ResizeComposite implements
          */
         private void setCellFocus(int rowIndex, int columnIndexDOM,
                 RowContainer container) {
-            if (rowIndex == rowWithFocus && cellFocusRange.contains(columnIndexDOM)
+            if (rowIndex == rowWithFocus
+                    && cellFocusRange.contains(columnIndexDOM)
                     && container == this.containerWithFocus) {
                 refreshRow(rowWithFocus);
                 return;
@@ -1955,10 +1955,12 @@ public class Grid<T> extends ResizeComposite implements
                     ++i;
                 } while (cell != null);
             }
-            int columnIndex = getColumns().indexOf(getVisibleColumn(columnIndexDOM));
+            int columnIndex = getColumns().indexOf(
+                    getVisibleColumn(columnIndexDOM));
             if (columnIndex >= escalator.getColumnConfiguration()
                     .getFrozenColumnCount()) {
-                escalator.scrollToColumn(columnIndexDOM, ScrollDestination.ANY, 10);
+                escalator.scrollToColumn(columnIndexDOM, ScrollDestination.ANY,
+                        10);
             }
 
             if (this.containerWithFocus == container) {
@@ -3048,7 +3050,7 @@ public class Grid<T> extends ResizeComposite implements
      * UI and functionality related to hiding columns with toggles in the
      * sidebar.
      */
-    private final class ColumnHider extends VerticalPanel {
+    private final class ColumnHider extends FlowPanel {
 
         ColumnHider() {
             setStyleName("column-hiding-panel");
@@ -3073,7 +3075,7 @@ public class Grid<T> extends ResizeComposite implements
         }
 
         private ToggleButton createToggle(final Column<?, T> column) {
-            ToggleButton toggle = new ToggleButton(column.headerCaption);
+            ToggleButton toggle = new ToggleButton();
             toggle.addStyleName("column-hiding-toggle");
             toggle.addValueChangeHandler(new ValueChangeHandler<Boolean>() {
 
@@ -3082,6 +3084,7 @@ public class Grid<T> extends ResizeComposite implements
                     column.setHidden(!event.getValue(), true);
                 }
             });
+            updateColumnHidingToggleCaption(column, toggle);
             columnToHidingToggleMap.put(column, toggle);
             return toggle;
         }
@@ -3119,7 +3122,19 @@ public class Grid<T> extends ResizeComposite implements
         }
 
         private void updateColumnHidingToggleCaption(Column<?, T> column) {
-            columnToHidingToggleMap.get(column).setText(column.headerCaption);
+            updateColumnHidingToggleCaption(column,
+                    columnToHidingToggleMap.get(column));
+        }
+
+        private void updateColumnHidingToggleCaption(Column<?, T> column,
+                ToggleButton toggle) {
+            String caption = column.headerCaption;
+            if (caption == null || caption.isEmpty()) {
+                // TODO what if the content is a widget?
+                HeaderCell cell = getDefaultHeaderRow().getCell(column);
+                caption = cell.getText();
+            }
+            toggle.setText(caption);
         }
 
     }
@@ -5001,6 +5016,10 @@ public class Grid<T> extends ResizeComposite implements
         Set<String> events = new HashSet<String>();
         events.addAll(getConsumedEventsForRenderer(column.getRenderer()));
 
+        if (column.isHidable()) {
+            columnHider.updateColumnHidable(column);
+        }
+
         sinkEvents(events);
     }
 
@@ -5061,6 +5080,10 @@ public class Grid<T> extends ResizeComposite implements
         ((Column<?, T>) column).setGrid(null);
 
         columns.remove(columnIndex);
+
+        if (column.isHidable()) {
+            columnHider.updateColumnHidable(column);
+        }
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 4a52dba173..e093a99159 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -182,7 +182,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
     }
 
     /**
-     * An event that is fired when a column becomes hidden or unhidden.
+     * An event that is fired when a column's visibility changes.
      * 
      * @since
      */
@@ -190,6 +190,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
 
         private final Column column;
         private final boolean userOriginated;
+        private final boolean hidden;
 
         /**
          * Constructor for a column visibility change event.
@@ -198,21 +199,25 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          *            the grid from which this event originates
          * @param column
          *            the column that changed its visibility
+         * @param hidden
+         *            <code>true</code> if the column was hidden,
+         *            <code>false</code> if it became visible
          * @param isUserOriginated
          *            <code>true</code> iff the event was triggered by an UI
          *            interaction
          */
         public ColumnVisibilityChangeEvent(Grid source, Column column,
-                boolean isUserOriginated) {
+                boolean hidden, boolean isUserOriginated) {
             super(source);
             this.column = column;
+            this.hidden = hidden;
             userOriginated = isUserOriginated;
         }
 
         /**
-         * Gets the column that became hidden or unhidden.
+         * Gets the column that became hidden or visible.
          * 
-         * @return the column that became hidden or unhidden.
+         * @return the column that became hidden or visible.
          * @see Column#isHidden()
          */
         public Column getColumn() {
@@ -220,6 +225,16 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
         }
 
         /**
+         * Was the column set hidden or visible.
+         * 
+         * @return <code>true</code> if the column was hidden <code>false</code>
+         *         if it was set visible
+         */
+        public boolean isHidden() {
+            return hidden;
+        }
+
+        /**
          * Returns <code>true</code> if the column reorder was done by the user,
          * <code>false</code> if not and it was triggered by server side code.
          * 
@@ -2828,7 +2843,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             if (hidden != getState().hidden) {
                 getState().hidden = hidden;
                 grid.markAsDirty();
-                grid.fireColumnVisibilityChangeEvent(this, false);
+                grid.fireColumnVisibilityChangeEvent(this, hidden, false);
             }
         }
 
@@ -3359,6 +3374,42 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             }
 
             @Override
+            public void columnVisibilityChanged(String id, boolean hidden,
+                    boolean userOriginated) {
+                final Column column = getColumnByColumnId(id);
+                final GridColumnState columnState = column.getState();
+
+                if (columnState.hidden != hidden) {
+                    columnState.hidden = hidden;
+
+                    final String diffStateKey = "columns";
+                    ConnectorTracker connectorTracker = getUI()
+                            .getConnectorTracker();
+                    JsonObject diffState = connectorTracker
+                            .getDiffState(Grid.this);
+
+                    assert diffState.hasKey(diffStateKey) : "Field name has changed";
+                    Type type = null;
+                    try {
+                        type = (getState(false).getClass().getDeclaredField(
+                                diffStateKey).getGenericType());
+                    } catch (NoSuchFieldException e) {
+                        e.printStackTrace();
+                    } catch (SecurityException e) {
+                        e.printStackTrace();
+                    }
+                    EncodeResult encodeResult = JsonCodec.encode(
+                            getState(false).columns, diffState, type,
+                            connectorTracker);
+
+                    diffState.put(diffStateKey, encodeResult.getEncodedValue());
+
+                    fireColumnVisibilityChangeEvent(column, hidden,
+                            userOriginated);
+                }
+            }
+
+            @Override
             public void sendDetailsComponents(int fetchId) {
                 getRpcProxy(GridClientRpc.class).setDetailsConnectorChanges(
                         detailComponentManager.getAndResetConnectorChanges(),
@@ -5455,9 +5506,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 COLUMN_VISIBILITY_METHOD);
     }
 
-    private void fireColumnVisibilityChangeEvent(Column column,
+    private void fireColumnVisibilityChangeEvent(Column column, boolean hidden,
             boolean isUserOriginated) {
-        fireEvent(new ColumnVisibilityChangeEvent(this, column,
+        fireEvent(new ColumnVisibilityChangeEvent(this, column, hidden,
                 isUserOriginated));
     }
 
diff --git a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java
index 28f59ea93a..2b2308fe84 100644
--- a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java
+++ b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java
@@ -76,4 +76,19 @@ public interface GridServerRpc extends ServerRpc {
      * @see com.vaadin.ui.Grid#setDetailsVisible(Object, boolean)
      */
     void sendDetailsComponents(int fetchId);
+
+    /**
+     * Informs the server that the column's visibility has been changed.
+     * 
+     * @since
+     * @param id
+     *            the id of the column
+     * @param hidden
+     *            <code>true</code> if hidden, <code>false</code> if unhidden
+     * @param userOriginated
+     *            <code>true</code> if triggered by user, <code>false</code> if
+     *            by code
+     */
+    void columnVisibilityChanged(String id, boolean hidden,
+            boolean userOriginated);
 }
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 c8c0e54123..d3b1237cf9 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
@@ -767,12 +767,18 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
             createClickAction("Add / Remove", getColumnProperty(c),
                     new Command<Grid, String>() {
 
+                        boolean wasHidable;
+
                         @Override
                         public void execute(Grid grid, String value, Object data) {
                             String columnProperty = getColumnProperty((Integer) data);
                             if (grid.getColumn(columnProperty) == null) {
                                 grid.addColumn(columnProperty);
+                                grid.getColumn(columnProperty).setHidable(
+                                        wasHidable);
                             } else {
+                                wasHidable = grid.getColumn(columnProperty)
+                                        .isHidable();
                                 grid.removeColumn(columnProperty);
                             }
                         }
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java
index 8fb733dfa0..22a08d6748 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java
@@ -17,18 +17,22 @@ package com.vaadin.tests.components.grid.basicfeatures.server;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
+import java.util.List;
+
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
 
-import com.vaadin.testbench.annotations.RunLocally;
-import com.vaadin.testbench.parallel.Browser;
 import com.vaadin.testbench.parallel.TestCategory;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
 
 @TestCategory("grid")
-@RunLocally(Browser.PHANTOMJS)
 public class GridColumnVisibilityTest extends GridBasicFeaturesTest {
 
     private static final String[] TOGGLE_LISTENER = new String[] { "Component",
@@ -40,6 +44,8 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest {
             + "changed: propertyId: Column 0, isHidden: true";
     private static final String COLUMN_0_BECAME_UNHIDDEN_MSG = "Visibility "
             + "changed: propertyId: Column 0, isHidden: false";
+    private static final String USER_ORIGINATED_TRUE = "userOriginated: true";
+    private static final String USER_ORIGINATED_FALSE = "userOriginated: false";
 
     @Before
     public void setUp() {
@@ -72,9 +78,11 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest {
 
         selectMenuPath(TOGGLE_HIDE_COLUMN_0);
         assertTrue(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG));
+        assertTrue(logContainsText(USER_ORIGINATED_FALSE));
 
         selectMenuPath(TOGGLE_HIDE_COLUMN_0);
         assertTrue(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG));
+        assertTrue(logContainsText(USER_ORIGINATED_FALSE));
     }
 
     @Test
@@ -86,4 +94,111 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest {
         selectMenuPath(TOGGLE_HIDE_COLUMN_0);
         assertFalse(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG));
     }
+
+    @Test
+    public void testColumnHiding_userOriginated_correctParams() {
+        selectMenuPath(TOGGLE_LISTENER);
+        toggleColumnHidable(0);
+        assertColumnHeaderOrder(0, 1, 2, 3);
+
+        getSidebarOpenButton().click();
+        getColumnHidingToggle(0).click();
+        getSidebarOpenButton().click();
+
+        assertColumnHeaderOrder(1, 2, 3);
+        assertTrue(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG));
+        assertTrue(logContainsText(USER_ORIGINATED_TRUE));
+
+        getSidebarOpenButton().click();
+        getColumnHidingToggle(0).click();
+        getSidebarOpenButton().click();
+
+        assertColumnHeaderOrder(0, 1, 2, 3);
+        assertTrue(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG));
+        assertTrue(logContainsText(USER_ORIGINATED_TRUE));
+
+        getSidebarOpenButton().click();
+        getColumnHidingToggle(0).click();
+        getSidebarOpenButton().click();
+
+        assertColumnHeaderOrder(1, 2, 3);
+        assertTrue(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG));
+        assertTrue(logContainsText(USER_ORIGINATED_TRUE));
+    }
+
+    @Test
+    public void testColumnHiding_whenHidableColumnRemoved_toggleRemoved() {
+        toggleColumnHidable(0);
+        toggleColumnHidable(1);
+        getSidebarOpenButton().click();
+        assertNotNull(getColumnHidingToggle(0));
+
+        addRemoveColumn(0);
+
+        assertNull(getColumnHidingToggle(0));
+    }
+
+    @Test
+    @Ignore
+    // known issue, column caption not passed to toggle when added again
+    public void testColumnHiding_whenHidableColumnAdded_toggleAdded() {
+        selectMenuPath("Component", "Size", "Width", "100%");
+        toggleColumnHidable(0);
+        toggleColumnHidable(1);
+        addRemoveColumn(0);
+        addRemoveColumn(4);
+        addRemoveColumn(5);
+        addRemoveColumn(6);
+        addRemoveColumn(7);
+        addRemoveColumn(8);
+        addRemoveColumn(9);
+        addRemoveColumn(10);
+        assertColumnHeaderOrder(1, 2, 3, 11);
+
+        getSidebarOpenButton().click();
+        assertNull(getColumnHidingToggle(0));
+        getSidebarOpenButton().click();
+
+        addRemoveColumn(0);
+        assertColumnHeaderOrder(1, 2, 3, 11, 0);
+
+        getSidebarOpenButton().click();
+        assertNotNull(getColumnHidingToggle(0));
+    }
+
+    private void toggleColumnHidable(int index) {
+        selectMenuPath("Component", "Columns", "Column " + index, "Hidable");
+    }
+
+    private void addRemoveColumn(int index) {
+        selectMenuPath("Component", "Columns", "Column " + index,
+                "Add / Remove");
+    }
+
+    private WebElement getSidebar() {
+        List<WebElement> elements = findElements(By.className("v-grid-sidebar"));
+        return elements.isEmpty() ? null : elements.get(0);
+    }
+
+    private WebElement getSidebarOpenButton() {
+        List<WebElement> elements = findElements(By
+                .className("v-grid-sidebar-button"));
+        return elements.isEmpty() ? null : elements.get(0);
+    }
+
+    /**
+     * Returns the toggle inside the sidebar for hiding the column at the given
+     * index, or null if not found.
+     */
+    private WebElement getColumnHidingToggle(int columnIndex) {
+        WebElement sidebar = getSidebar();
+        List<WebElement> elements = sidebar.findElements(By
+                .className("column-hiding-toggle"));
+        for (WebElement e : elements) {
+            if (("Column " + columnIndex).equalsIgnoreCase(e.getText())) {
+                return e;
+            }
+        }
+        return null;
+    }
 }