Fix ClickableRenderers to use CellReferences (#13334)

This patch also uses row keys and column ids in communication rather
than indices.

Change-Id: Ie3bb554bd5c45700f1681160eef08bef8beeaaee

9 files changed, 106 insertions, 33 deletions

diff --git a/client/src/com/vaadin/client/connectors/AbstractRendererConnector.java b/client/src/com/vaadin/client/connectors/AbstractRendererConnector.java
index 781de78d06..b258980fe1 100644
--- a/client/src/com/vaadin/client/connectors/AbstractRendererConnector.java
+++ b/client/src/com/vaadin/client/connectors/AbstractRendererConnector.java
@@ -24,7 +24,9 @@ import com.vaadin.client.metadata.Type;
 import com.vaadin.client.metadata.TypeData;
 import com.vaadin.client.metadata.TypeDataStore;
 import com.vaadin.client.renderers.Renderer;
+import com.vaadin.client.widgets.Grid.Column;
 
+import elemental.json.JsonObject;
 import elemental.json.JsonValue;
 
 /**
@@ -137,23 +139,45 @@ public abstract class AbstractRendererConnector<T> extends
     }
 
     /**
-     * Gets the row key for a row index.
+     * Gets the row key for a row object.
      * <p>
      * In case this renderer wants be able to identify a row in such a way that
      * the server also understands it, the row key is used for that. Rows are
      * identified by unified keys between the client and the server.
      * 
-     * @param index
-     *            the row index for which to get the row key
-     * @return the row key for the row at {@code index}
+     * @param row
+     *            the row object
+     * @return the row key for the given row
      */
-    protected String getRowKey(int index) {
+    protected String getRowKey(JsonObject row) {
         final ServerConnector parent = getParent();
         if (parent instanceof GridConnector) {
-            return ((GridConnector) parent).getRowKey(index);
+            return ((GridConnector) parent).getRowKey(row);
         } else {
             throw new IllegalStateException("Renderers can only be used "
                     + "with a Grid.");
         }
     }
+
+    /**
+     * Gets the column id for a column.
+     * <p>
+     * In case this renderer wants be able to identify a column in such a way
+     * that the server also understands it, the column id is used for that.
+     * Columns are identified by unified ids between the client and the server.
+     * 
+     * @param column
+     *            the column object
+     * @return the column id for the given column
+     */
+    protected String getColumnId(Column<?, JsonObject> column) {
+        final ServerConnector parent = getParent();
+        if (parent instanceof GridConnector) {
+            return ((GridConnector) parent).getColumnId(column);
+        } else {
+            throw new IllegalStateException("Renderers can only be used "
+                    + "with a Grid.");
+        }
+    }
+
 }
diff --git a/client/src/com/vaadin/client/connectors/ClickableRendererConnector.java b/client/src/com/vaadin/client/connectors/ClickableRendererConnector.java
index ee995384f3..90aaad7032 100644
--- a/client/src/com/vaadin/client/connectors/ClickableRendererConnector.java
+++ b/client/src/com/vaadin/client/connectors/ClickableRendererConnector.java
@@ -43,8 +43,8 @@ public abstract class ClickableRendererConnector<T> extends
             @Override
             public void onClick(RendererClickEvent<JsonObject> event) {
                 getRpcProxy(RendererClickRpc.class).click(
-                        event.getCell().getRow(),
-                        event.getCell().getColumn(),
+                        getRowKey(event.getCell().getRow()),
+                        getColumnId(event.getCell().getColumn()),
                         MouseEventDetailsBuilder.buildMouseEventDetails(event
                                 .getNativeEvent()));
             }
diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
index b5e5adba8b..d3954f2366 100644
--- a/client/src/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -861,14 +861,13 @@ public class GridConnector extends AbstractHasComponentsConnector implements
     }-*/;
 
     /**
-     * Gets the row key for a row by index.
+     * Gets the row key for a row object.
      * 
-     * @param index
-     *            the index of the row for which to get the key
-     * @return the key for the row at {@code index}
+     * @param row
+     *            the row object
+     * @return the key for the given row
      */
-    public String getRowKey(int index) {
-        final JsonObject row = dataSource.getRow(index);
+    public String getRowKey(JsonObject row) {
         final Object key = dataSource.getRowKey(row);
         assert key instanceof String : "Internal key was not a String but a "
                 + key.getClass().getSimpleName() + " (" + key + ")";
@@ -893,6 +892,13 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             ConnectorHierarchyChangeEvent connectorHierarchyChangeEvent) {
     }
 
+    public String getColumnId(Grid.Column<?, JsonObject> column) {
+        if (column instanceof CustomGridColumn) {
+            return ((CustomGridColumn) column).id;
+        }
+        return null;
+    }
+
     @Override
     public void layout() {
         getWidget().onResize();
diff --git a/client/src/com/vaadin/client/renderers/ClickableRenderer.java b/client/src/com/vaadin/client/renderers/ClickableRenderer.java
index cdea5ea1b4..21f0e28c76 100644
--- a/client/src/com/vaadin/client/renderers/ClickableRenderer.java
+++ b/client/src/com/vaadin/client/renderers/ClickableRenderer.java
@@ -26,7 +26,7 @@ import com.google.gwt.event.shared.EventHandler;
 import com.google.gwt.event.shared.HandlerManager;
 import com.google.gwt.user.client.ui.Widget;
 import com.google.web.bindery.event.shared.HandlerRegistration;
-import com.vaadin.client.widget.escalator.Cell;
+import com.vaadin.client.widget.grid.CellReference;
 import com.vaadin.client.widget.grid.GridUtil;
 import com.vaadin.client.widgets.Grid;
 
@@ -80,7 +80,7 @@ public abstract class ClickableRenderer<T, W extends Widget> extends
         static final Type<RendererClickHandler> TYPE = new Type<RendererClickHandler>(
                 BrowserEvents.CLICK, new RendererClickEvent());
 
-        private Cell cell;
+        private CellReference<R> cell;
 
         private R row;
 
@@ -92,7 +92,7 @@ public abstract class ClickableRenderer<T, W extends Widget> extends
          * 
          * @return the cell
          */
-        public Cell getCell() {
+        public CellReference<R> getCell() {
             return cell;
         }
 
@@ -125,7 +125,7 @@ public abstract class ClickableRenderer<T, W extends Widget> extends
             Grid<R> grid = (Grid<R>) GridUtil.findClosestParentGrid(e);
 
             cell = GridUtil.findCell(grid, e);
-            row = grid.getDataSource().getRow(cell.getRow());
+            row = cell.getRow();
 
             handler.onClick(this);
         }
diff --git a/client/src/com/vaadin/client/widget/grid/GridUtil.java b/client/src/com/vaadin/client/widget/grid/GridUtil.java
index 9edaada9b1..a49dd36a57 100644
--- a/client/src/com/vaadin/client/widget/grid/GridUtil.java
+++ b/client/src/com/vaadin/client/widget/grid/GridUtil.java
@@ -42,9 +42,15 @@ public class GridUtil {
      * @return the cell or null if the element is not a grid cell or a
      *         descendant of one
      */
-    public static Cell findCell(Grid<?> grid, Element e) {
+    public static <T> CellReference<T> findCell(Grid<T> grid, Element e) {
         RowContainer container = getEscalator(grid).findRowContainer(e);
-        return container != null ? container.getCell(e) : null;
+        if (container == null) {
+            return null;
+        }
+        Cell cell = container.getCell(e);
+        EventCellReference<T> cellReference = new EventCellReference<T>(grid);
+        cellReference.set(cell);
+        return cellReference;
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index 60df269f10..8aee5ac19e 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -2382,9 +2382,35 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          * @return the item id corresponding to {@code key}
          */
         protected Object getItemId(String rowKey) {
+            return getParentGrid().getKeyMapper().getItemId(rowKey);
+        }
+
+        /**
+         * Gets the column for a column id.
+         * <p>
+         * An id is used to identify a particular column on both a server and a
+         * client. This method can be used to get the column for the column id
+         * that the client has sent.
+         * 
+         * @param columnId
+         *            the column id for which to retrieve a column
+         * @return the column corresponding to {@code columnId}
+         */
+        protected Column getColumn(String columnId) {
+            return getParentGrid().getColumnByColumnId(columnId);
+        }
+
+        /**
+         * Gets the parent Grid of the renderer.
+         * 
+         * @return parent grid
+         * @throws IllegalStateException
+         *             if parent is not Grid
+         */
+        protected Grid getParentGrid() {
             if (getParent() instanceof Grid) {
                 Grid grid = (Grid) getParent();
-                return grid.getKeyMapper().getItemId(rowKey);
+                return grid;
             } else {
                 throw new IllegalStateException(
                         "Renderers can be used only with Grid");
diff --git a/server/src/com/vaadin/ui/renderer/ClickableRenderer.java b/server/src/com/vaadin/ui/renderer/ClickableRenderer.java
index 0d745ab29e..ef293c7e3c 100644
--- a/server/src/com/vaadin/ui/renderer/ClickableRenderer.java
+++ b/server/src/com/vaadin/ui/renderer/ClickableRenderer.java
@@ -23,6 +23,7 @@ import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.ui.grid.renderers.RendererClickRpc;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.AbstractRenderer;
+import com.vaadin.ui.Grid.Column;
 import com.vaadin.util.ReflectTools;
 
 /**
@@ -64,11 +65,13 @@ public class ClickableRenderer<T> extends AbstractRenderer<T> {
     public static class RendererClickEvent extends ClickEvent {
 
         private Object itemId;
+        private Column column;
 
-        protected RendererClickEvent(Grid source, Object itemId,
+        protected RendererClickEvent(Grid source, Object itemId, Column column,
                 MouseEventDetails mouseEventDetails) {
             super(source, mouseEventDetails);
             this.itemId = itemId;
+            this.column = column;
         }
 
         /**
@@ -79,20 +82,25 @@ public class ClickableRenderer<T> extends AbstractRenderer<T> {
         public Object getItemId() {
             return itemId;
         }
+
+        /**
+         * Returns the {@link Column} where the click event originated.
+         * 
+         * @return the column of the click event
+         */
+        public Column getColumn() {
+            return column;
+        }
     }
 
     protected ClickableRenderer(Class<T> presentationType) {
         super(presentationType);
         registerRpc(new RendererClickRpc() {
             @Override
-            public void click(int row, int column,
+            public void click(String rowKey, String columnId,
                     MouseEventDetails mouseDetails) {
-
-                Grid grid = (Grid) getParent();
-                Object itemId = grid.getContainerDataSource().getIdByIndex(row);
-                // TODO map column index to property ID or send column ID
-                // instead of index from the client
-                fireEvent(new RendererClickEvent(grid, itemId, mouseDetails));
+                fireEvent(new RendererClickEvent(getParentGrid(),
+                        getItemId(rowKey), getColumn(columnId), mouseDetails));
             }
         });
     }
diff --git a/shared/src/com/vaadin/shared/ui/grid/renderers/RendererClickRpc.java b/shared/src/com/vaadin/shared/ui/grid/renderers/RendererClickRpc.java
index f83b5aa44f..658caef050 100644
--- a/shared/src/com/vaadin/shared/ui/grid/renderers/RendererClickRpc.java
+++ b/shared/src/com/vaadin/shared/ui/grid/renderers/RendererClickRpc.java
@@ -26,5 +26,6 @@ public interface RendererClickRpc extends ServerRpc {
      * @param mouseDetails
      *            Details about the mouse when the event took place
      */
-    public void click(int row, int column, MouseEventDetails mouseDetails);
+    public void click(String rowKey, String columnId,
+            MouseEventDetails mouseDetails);
 }
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/RowAwareRendererConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/RowAwareRendererConnector.java
index 3c5b7cdf1f..63faf1d651 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/RowAwareRendererConnector.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/RowAwareRendererConnector.java
@@ -30,6 +30,8 @@ import com.vaadin.client.widget.grid.RendererCellReference;
 import com.vaadin.shared.communication.ServerRpc;
 import com.vaadin.shared.ui.Connect;
 
+import elemental.json.JsonObject;
+
 @Connect(com.vaadin.tests.components.grid.RowAwareRenderer.class)
 public class RowAwareRendererConnector extends AbstractRendererConnector<Void> {
     public interface RowAwareRendererRpc extends ServerRpc {
@@ -59,10 +61,10 @@ public class RowAwareRendererConnector extends AbstractRendererConnector<Void> {
 
         @Override
         public boolean onBrowserEvent(CellReference<?> cell, NativeEvent event) {
-            int row = cell.getRowIndex();
-            String key = getRowKey(row);
+            String key = getRowKey((JsonObject) cell.getRow());
             getRpcProxy(RowAwareRendererRpc.class).clicky(key);
-            cell.getElement().setInnerText("row: " + row + ", key: " + key);
+            cell.getElement().setInnerText(
+                    "row: " + cell.getRowIndex() + ", key: " + key);
             return true;
         }
     }