Allow changing renderers after column creation (vaadin#8841)

Closes vaadin#8250

Author: tsuoanttila

client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java

@@ -73,7 +73,6 @@ public Object getValue(JsonObject row) {
                 return null;
             }
         };
-        column.setRenderer(getRendererConnector().getRenderer());
         getParent().addColumn(column, getState().internalId);
     }
 
@@ -92,6 +91,11 @@ void updateSortable() {
         column.setSortable(getState().sortable);
     }
 
+    @OnStateChange("renderer")
+    void updateRenderer() {
+        column.setRenderer(getRendererConnector().getRenderer());
+    }
+
     @OnStateChange("hidingToggleCaption")
     void updateHidingToggleCaption() {
         column.setHidingToggleCaption(getState().hidingToggleCaption);

client/src/main/java/com/vaadin/client/connectors/treegrid/TreeGridConnector.java

@@ -18,12 +18,13 @@
 import java.util.Collection;
 import java.util.logging.Logger;
 
+import com.google.gwt.core.client.Scheduler;
 import com.google.gwt.dom.client.BrowserEvents;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.event.dom.client.KeyCodes;
 import com.google.gwt.user.client.Event;
 import com.google.web.bindery.event.shared.HandlerRegistration;
-import com.vaadin.client.communication.StateChangeEvent;
+import com.vaadin.client.annotations.OnStateChange;
 import com.vaadin.client.connectors.grid.GridConnector;
 import com.vaadin.client.renderers.ClickableRenderer;
 import com.vaadin.client.renderers.HierarchyRenderer;
@@ -43,7 +44,7 @@
 
 /**
  * A connector class for the TreeGrid component.
- * 
+ *
  * @author Vaadin Ltd
  * @since 8.1
  */
@@ -67,13 +68,18 @@ public TreeGridState getState() {
         return (TreeGridState) super.getState();
     }
 
-    @Override
-    public void onStateChanged(StateChangeEvent stateChangeEvent) {
-        super.onStateChanged(stateChangeEvent);
-
-        if (stateChangeEvent.hasPropertyChanged("hierarchyColumnId")
-                || stateChangeEvent.hasPropertyChanged("columns")) {
-
+    /**
+     * This method has been scheduled finally to avoid possible race conditions
+     * between state change handling for the Grid and its columns. The renderer
+     * of the column is set in a state change handler, and might not be
+     * available when this method is executed.
+     * <p>
+     * TODO: This might need some clean up if we decide to allow setting a new
+     * renderer for hierarchy columns.
+     */
+    @OnStateChange("hierarchyColumnId")
+    void updateHierarchyColumn() {
+        Scheduler.get().scheduleFinally(() -> {
             // Id of old hierarchy column
             String oldHierarchyColumnId = this.hierarchyColumnId;
 
@@ -111,7 +117,7 @@ public void onStateChanged(StateChangeEvent stateChangeEvent) {
                 Logger.getLogger(TreeGridConnector.class.getName()).warning(
                         "Couldn't find column: " + newHierarchyColumnId);
             }
-        }
+        });
     }
 
     private HierarchyRenderer getHierarchyRenderer() {
@@ -159,18 +165,21 @@ public void onUnregister() {
     }
 
     private native void replaceCellFocusEventHandler(Grid<?> grid,
-            GridEventHandler<?> eventHandler)/*-{
+            GridEventHandler<?> eventHandler)
+    /*-{
         var browserEventHandlers = [email protected]::browserEventHandlers;
-        
+    
         // FocusEventHandler is initially 5th in the list of browser event handlers
         [email protected]::set(*)(5, eventHandler);
     }-*/;
 
-    private native void replaceClickEvent(Grid<?> grid, GridClickEvent event)/*-{
+    private native void replaceClickEvent(Grid<?> grid, GridClickEvent event)
+    /*-{
         [email protected]::clickEvent = event;
     }-*/;
 
-    private native EventCellReference<?> getEventCell(Grid<?> grid)/*-{
+    private native EventCellReference<?> getEventCell(Grid<?> grid)
+    /*-{
         return [email protected]::eventCell;
     }-*/;
 
@@ -213,13 +222,15 @@ public void onEvent(Grid.GridEvent<JsonObject> event) {
             }
         }
 
-        private native Collection<String> getNavigationEvents(Grid<?> grid)/*-{
-           return [email protected]::cellFocusHandler
-           [email protected]::getNavigationEvents()();
+        private native Collection<String> getNavigationEvents(Grid<?> grid)
+        /*-{
+            return [email protected]::cellFocusHandler
+            [email protected]::getNavigationEvents()();
         }-*/;
 
         private native void handleNavigationEvent(Grid<?> grid,
-                Grid.GridEvent<JsonObject> event)/*-{
+                Grid.GridEvent<JsonObject> event)
+        /*-{
             [email protected]::cellFocusHandler
             [email protected]::handleNavigationEvent(*)(
             [email protected]::getDomEvent()(),

documentation/components/components-grid.asciidoc

@@ -403,9 +403,23 @@ You set the column renderer in the [classname]#Grid.Column# object as follows:
 [source, java]
 ----
 // the type of birthYear is a number
-Column<Integer> bornColumn = grid.addColumn(Person:getBirthYear,
+Column<Person, Integer> bornColumn = grid.addColumn(Person:getBirthYear,
       new NumberRenderer("born in %d AD"));
 ----
+
+Changing the renderer during runtime is also possible, but for type safety
+you should store the column reference with data types for doing this.
+When you change the renderer, the content of Grid is refreshed.
+
+[source, java]
+----
+Column<Person, Integer> ageColumn = grid.addColumn(Person::getBirthYear);
+// The default renderer is TextRenderer
+addComponent(new Button("Change renderer", 
+	clickEvent -> ageColumn.setRenderer(new NumberRenderer())
+));
+----
+
 The following renderers are available, as defined in the server-side
 [package]#com.vaadin.ui.renderers# package:
 

server/src/main/java/com/vaadin/ui/Grid.java

@@ -73,6 +73,7 @@
 import com.vaadin.server.SerializableSupplier;
 import com.vaadin.server.Setter;
 import com.vaadin.server.VaadinServiceClassLoaderUtil;
+import com.vaadin.shared.Connector;
 import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.Registration;
 import com.vaadin.shared.data.DataCommunicatorConstants;
@@ -1813,6 +1814,34 @@ public <F, C extends HasValue<F> & Component> Column<T, V> setEditorComponent(
             return setEditorBinding(binding);
         }
 
+        /**
+         * Sets the Renderer for this Column. Setting the renderer will cause
+         * all currently available row data to be recreated and sent to the
+         * client.
+         *
+         * @param renderer
+         *            the new renderer
+         * @return this column
+         */
+        public Column<T, V> setRenderer(Renderer<? super V> renderer) {
+            Objects.requireNonNull(renderer, "Renderer can't be null");
+
+            // Remove old renderer
+            Connector oldRenderer = getState().renderer;
+            if (oldRenderer != null && oldRenderer instanceof Extension) {
+                removeExtension((Extension) oldRenderer);
+            }
+
+            // Set new renderer
+            getState().renderer = renderer;
+            addExtension(renderer);
+
+            // Trigger redraw
+            getParent().getDataCommunicator().reset();
+
+            return this;
+        }
+
         /**
          * Gets the grid that this column belongs to.
          *
@@ -2341,11 +2370,25 @@ public <V> Column<T, V> addColumn(ValueProvider<T, V> valueProvider) {
     public <V> Column<T, V> addColumn(ValueProvider<T, V> valueProvider,
             AbstractRenderer<? super T, ? super V> renderer) {
         String generatedIdentifier = getGeneratedIdentifier();
-        Column<T, V> column = new Column<>(valueProvider, renderer);
+        Column<T, V> column = createColumn(valueProvider, renderer);
         addColumn(generatedIdentifier, column);
         return column;
     }
 
+    /**
+     * Creates a column instance from a value provider and a renderer.
+     *
+     * @param valueProvider
+     *            the value provider
+     * @param renderer
+     *            the renderer
+     * @return a new column instance
+     */
+    protected <V> Column<T, V> createColumn(ValueProvider<T, V> valueProvider,
+            AbstractRenderer<? super T, ? super V> renderer) {
+        return new Column<>(valueProvider, renderer);
+    }
+
     private void addColumn(String identifier, Column<T, ?> column) {
         if (getColumns().contains(column)) {
             return;

server/src/main/java/com/vaadin/ui/TreeGrid.java

@@ -19,21 +19,24 @@
 import java.util.Objects;
 import java.util.stream.Stream;
 
+import com.vaadin.data.ValueProvider;
 import com.vaadin.data.provider.DataProvider;
 import com.vaadin.data.provider.HierarchicalDataProvider;
 import com.vaadin.shared.ui.treegrid.NodeCollapseRpc;
 import com.vaadin.shared.ui.treegrid.TreeGridCommunicationConstants;
 import com.vaadin.shared.ui.treegrid.TreeGridState;
+import com.vaadin.ui.renderers.AbstractRenderer;
+import com.vaadin.ui.renderers.Renderer;
 
 import elemental.json.Json;
 import elemental.json.JsonObject;
 
 /**
  * A grid component for displaying hierarchical tabular data.
- * 
+ *
  * @author Vaadin Ltd
  * @since 8.1
- * 
+ *
  * @param <T>
  *            the grid bean type
  */
@@ -106,9 +109,12 @@ public void setDataProvider(DataProvider<T, ?> dataProvider) {
      * <p>
      * Setting a hierarchy column by calling this method also sets the column to
      * be visible and not hidable.
-     * 
+     * <p>
+     * <strong>Note:</strong> Changing the Renderer of the hierarchy column is
+     * not supported.
+     *
      * @see Column#setId(String)
-     * 
+     *
      * @param id
      *            id of the column to use for displaying hierarchy
      */
@@ -137,7 +143,7 @@ protected TreeGridState getState(boolean markAsDirty) {
      * expanded, it will be collapsed.
      * <p>
      * Toggling expansion on a leaf item in the hierarchy will have no effect.
-     * 
+     *
      * @param item
      *            the item to toggle expansion for
      */
@@ -161,4 +167,24 @@ public void toggleCollapse(T item) {
         }
         return (HierarchicalDataProvider<T, ?>) dataProvider;
     }
+
+    @Override
+    protected <V> Column<T, V> createColumn(ValueProvider<T, V> valueProvider,
+            AbstractRenderer<? super T, ? super V> renderer) {
+        return new Column<T, V>(valueProvider, renderer) {
+
+            @Override
+            public com.vaadin.ui.Grid.Column<T, V> setRenderer(
+                    Renderer<? super V> renderer) {
+                // Disallow changing renderer for the hierarchy column
+                if (getInternalIdForColumn(this).equals(
+                        TreeGrid.this.getState(false).hierarchyColumnId)) {
+                    throw new IllegalStateException(
+                            "Changing the renderer of the hierarchy column is not allowed.");
+                }
+
+                return super.setRenderer(renderer);
+            }
+        };
+    }
 }

server/src/test/java/com/vaadin/tests/components/treegrid/TreeGridColumnTest.java

@@ -0,0 +1,19 @@
+package com.vaadin.tests.components.treegrid;
+
+import org.junit.Test;
+
+import com.vaadin.ui.TreeGrid;
+import com.vaadin.ui.renderers.TextRenderer;
+
+public class TreeGridColumnTest {
+
+    private TreeGrid<String> treeGrid = new TreeGrid<>();
+
+    @Test(expected = IllegalStateException.class)
+    public void testChangeRendererOfHierarchyColumn() {
+        treeGrid.addColumn(Object::toString).setId("foo");
+        treeGrid.setHierarchyColumn("foo");
+        // This should not be allowed.
+        treeGrid.getColumn("foo").setRenderer(new TextRenderer());
+    }
+}

uitest/src/main/java/com/vaadin/tests/components/grid/GridRendererSwitch.java

@@ -0,0 +1,53 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.stream.IntStream;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.Column;
+import com.vaadin.ui.Notification;
+import com.vaadin.ui.renderers.ButtonRenderer;
+import com.vaadin.ui.renderers.TextRenderer;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class GridRendererSwitch extends AbstractTestUI {
+
+    private boolean textRenderer = true;
+    private boolean reverse = false;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Grid<Integer> grid = new Grid<>();
+        Column<Integer, String> column = grid.addColumn(i -> "Foo " + i)
+                .setCaption("Foo");
+        Column<Integer, String> secondColumn = grid.addColumn(i -> "Bar " + i)
+                .setCaption("Bar");
+
+        addComponent(grid);
+        addComponent(new Button("Switch", e -> {
+            if (textRenderer) {
+                ButtonRenderer<Integer> renderer = new ButtonRenderer<>();
+                renderer.addClickListener(event -> Notification
+                        .show("Click on row: " + event.getItem()));
+                column.setRenderer(renderer);
+            } else {
+                column.setRenderer(new TextRenderer());
+            }
+            textRenderer = !textRenderer;
+        }));
+        addComponent(new Button("Reverse", e -> {
+            if (reverse) {
+                grid.setColumnOrder(column, secondColumn);
+            } else {
+                grid.setColumnOrder(secondColumn, column);
+            }
+            reverse = !reverse;
+        }));
+
+        grid.setItems(IntStream.range(0, 10).boxed());
+    }
+
+}