Make Grid call ComplexRenderer.onActivate when needed (#13334)

Change-Id: Icd2ecbdb0780ba97e0955eb7c564f8f56ca14109

5 files changed, 85 insertions, 31 deletions

diff --git a/client/src/com/vaadin/client/ui/grid/Grid.java b/client/src/com/vaadin/client/ui/grid/Grid.java
index b3e2fd8a94..1ed5cb8a75 100644
--- a/client/src/com/vaadin/client/ui/grid/Grid.java
+++ b/client/src/com/vaadin/client/ui/grid/Grid.java
@@ -372,8 +372,9 @@ public class Grid<T> extends Composite implements
          * Handle events that can change the currently active cell.
          */
         public void handleNavigationEvent(Event event, Cell cell) {
-            if (event.getType().equals(BrowserEvents.CLICK) && cell != null) {
+            if (event.getType().equals(BrowserEvents.CLICK)) {
                 setActiveCell(cell);
+                // Grid should have focus when clicked.
                 getElement().focus();
             } else if (event.getType().equals(BrowserEvents.KEYDOWN)) {
                 int newRow = activeRow;
@@ -1261,7 +1262,7 @@ public class Grid<T> extends Composite implements
         // Sink header events and key events
         sinkEvents(getHeader().getConsumedEvents());
         sinkEvents(Arrays.asList(BrowserEvents.KEYDOWN, BrowserEvents.KEYUP,
-                BrowserEvents.KEYPRESS));
+                BrowserEvents.KEYPRESS, BrowserEvents.DBLCLICK));
 
         // Make ENTER and SHIFT+ENTER in the header perform sorting
         addHeaderKeyUpHandler(new HeaderKeyUpHandler() {
@@ -1994,26 +1995,35 @@ public class Grid<T> extends Composite implements
 
         Element e = Element.as(target);
         RowContainer container = escalator.findRowContainer(e);
-        Cell cell = container != null ? container.getCell(e) : null;
-
-        if (handleEditorRowEvent(event, container, cell)) {
-            return;
+        Cell cell;
+        boolean isGrid = Util.findWidget(e, null) == this;
+        if (container == null) {
+            cell = activeCellHandler.getActiveCell();
+            container = activeCellHandler.container;
+        } else {
+            cell = container.getCell(e);
         }
 
-        if (handleHeaderDefaultRowEvent(event, container, cell)) {
-            return;
-        }
+        if (isGrid) {
+            if (handleEditorRowEvent(event, container, cell)) {
+                return;
+            }
 
-        if (handleRendererEvent(event, container, cell)) {
-            return;
-        }
+            if (handleHeaderDefaultRowEvent(event, container, cell)) {
+                return;
+            }
 
-        if (handleNavigationEvent(event, container, cell)) {
-            return;
-        }
+            if (handleRendererEvent(event, container, cell)) {
+                return;
+            }
 
-        if (handleActiveCellEvent(event, container, cell)) {
-            return;
+            if (handleNavigationEvent(event, container, cell)) {
+                return;
+            }
+
+            if (handleActiveCellEvent(event, container, cell)) {
+                return;
+            }
         }
     }
 
@@ -2047,6 +2057,10 @@ public class Grid<T> extends Composite implements
         if (container == escalator.getBody() && cell != null) {
             GridColumn<?, T> gridColumn = getColumnFromVisibleIndex(cell
                     .getColumn());
+            boolean enterKey = event.getType().equals(BrowserEvents.KEYDOWN)
+                    && event.getKeyCode() == KeyCodes.KEY_ENTER;
+            boolean doubleClick = event.getType()
+                    .equals(BrowserEvents.DBLCLICK);
 
             if (gridColumn.getRenderer() instanceof ComplexRenderer) {
                 ComplexRenderer<?> cplxRenderer = (ComplexRenderer<?>) gridColumn
@@ -2056,6 +2070,11 @@ public class Grid<T> extends Composite implements
                         return true;
                     }
                 }
+
+                // Calls onActivate if KeyDown and Enter or double click
+                if ((enterKey || doubleClick) && cplxRenderer.onActivate(cell)) {
+                    return true;
+                }
             }
         }
         return false;
@@ -2064,8 +2083,7 @@ public class Grid<T> extends Composite implements
     private boolean handleActiveCellEvent(Event event, RowContainer container,
             Cell cell) {
         Collection<String> navigation = activeCellHandler.getNavigationEvents();
-        if (navigation.contains(event.getType())
-                && (Util.getFocusedElement() == getElement() || cell != null)) {
+        if (navigation.contains(event.getType())) {
             activeCellHandler.handleNavigationEvent(event, cell);
         }
         return false;
@@ -2152,6 +2170,8 @@ public class Grid<T> extends Composite implements
             lazySorter.setMultisort(true);
             lazySorter.schedule(GridConstants.LONG_TAP_DELAY);
 
+            return true;
+
         } else if (BrowserEvents.TOUCHMOVE.equals(event.getType())) {
             if (event.getTouches().length() > 1) {
                 return false;
@@ -2172,8 +2192,10 @@ public class Grid<T> extends Composite implements
                 lazySorter.cancel();
             }
 
+            return true;
+
         } else if (BrowserEvents.TOUCHEND.equals(event.getType())) {
-            if (event.getTouches().length() > 0) {
+            if (event.getTouches().length() > 1) {
                 return false;
             }
 
@@ -2184,24 +2206,27 @@ public class Grid<T> extends Composite implements
                 lazySorter.run();
             }
 
+            return true;
+
         } else if (BrowserEvents.TOUCHCANCEL.equals(event.getType())) {
-            if (event.getChangedTouches().length() > 1) {
+            if (event.getTouches().length() > 1) {
                 return false;
             }
 
             lazySorter.cancel();
 
+            return true;
+
         } else if (BrowserEvents.CLICK.equals(event.getType())) {
             lazySorter.setCellReference(cell);
             lazySorter.setMultisort(event.getShiftKey());
             lazySorter.run();
 
-            // Active cell handling is also monitoring the click
-            // event so we allow event to propagate for it
+            // Click events should go onward to active cell logic
+            return false;
+        } else {
             return false;
         }
-
-        return true;
     }
 
     @Override
diff --git a/client/src/com/vaadin/client/ui/grid/renderers/ComplexRenderer.java b/client/src/com/vaadin/client/ui/grid/renderers/ComplexRenderer.java
index f0c95e2ddf..ed9aefd260 100644
--- a/client/src/com/vaadin/client/ui/grid/renderers/ComplexRenderer.java
+++ b/client/src/com/vaadin/client/ui/grid/renderers/ComplexRenderer.java
@@ -16,6 +16,7 @@
 package com.vaadin.client.ui.grid.renderers;
 
 import java.util.Collection;
+import java.util.Collections;
 
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.NativeEvent;
@@ -70,14 +71,13 @@ public abstract class ComplexRenderer<T> implements Renderer<T> {
      * Returns the events that the renderer should consume. These are also the
      * events that the Grid will pass to
      * {@link #onBrowserEvent(Cell, NativeEvent)} when they occur.
-     * <code>null</code> if no events are consumed
      * 
-     * @return the consumed events, or null if no events are consumed
+     * @return a list of consumed events
      * 
      * @see com.google.gwt.dom.client.BrowserEvents
      */
     public Collection<String> getConsumedEvents() {
-        return null;
+        return Collections.emptyList();
     }
 
     /**
@@ -136,10 +136,12 @@ public abstract class ComplexRenderer<T> implements Renderer<T> {
      * Called when the cell is "activated" by pressing <code>enter</code>,
      * double clicking or performing a double tap on the cell.
      * 
+     * @param cell
+     *            the activated cell
      * @return <code>true</code> if event was handled and should not be
      *         interpreted as a generic gesture by Grid.
      */
-    public boolean onActivate() {
+    public boolean onActivate(Cell cell) {
         return false;
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridClientRenderers.java b/uitest/src/com/vaadin/tests/components/grid/GridClientRenderers.java
index fd3c8d5b2f..2656407023 100644
--- a/uitest/src/com/vaadin/tests/components/grid/GridClientRenderers.java
+++ b/uitest/src/com/vaadin/tests/components/grid/GridClientRenderers.java
@@ -21,7 +21,9 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 import org.junit.Test;
+import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.Actions;
 import org.openqa.selenium.remote.DesiredCapabilities;
 
 import com.vaadin.testbench.By;
@@ -31,6 +33,7 @@ import com.vaadin.testbench.elements.NativeButtonElement;
 import com.vaadin.testbench.elements.NativeSelectElement;
 import com.vaadin.testbench.elements.ServerClass;
 import com.vaadin.tests.annotations.TestCategory;
+import com.vaadin.tests.components.grid.GridElement.GridCellElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 import com.vaadin.tests.widgetset.client.grid.GridClientColumnRendererConnector.Renderers;
 import com.vaadin.tests.widgetset.server.grid.GridClientColumnRenderers;
@@ -248,6 +251,24 @@ public class GridClientRenderers extends MultiBrowserTest {
         }
     }
 
+    @Test
+    public void testComplexRendererOnActivate() {
+        openTestURL();
+
+        GridCellElement cell = getGrid().getCell(3, 1);
+        cell.click();
+        new Actions(getDriver()).sendKeys(Keys.ENTER).perform();
+
+        assertEquals("onActivate was not called on KeyDown Enter.",
+                "Activated!", cell.getText());
+
+        cell = getGrid().getCell(4, 1);
+        cell.click();
+        new Actions(getDriver()).moveToElement(cell).doubleClick().perform();
+        assertEquals("onActivate was not called on double click.",
+                "Activated!", cell.getText());
+    }
+
     private GridElement getGrid() {
         return $(MyClientGridElement.class).first();
     }
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java
index 19a68a87f4..21bf667bae 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java
@@ -35,8 +35,7 @@ public class GridStaticSectionComponentTest extends GridBasicFeaturesTest {
 
         getGridElement().$(ButtonElement.class).first().click();
 
-        // Clicking also triggers sorting
-        assertEquals("2. Button clicked!", getLogRow(2));
+        assertEquals("2. Button clicked!", getLogRow(0));
     }
 
     @Test
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java
index 7a9f8a06f5..c5571394bd 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridClientColumnRendererConnector.java
@@ -31,6 +31,7 @@ import com.google.gwt.user.client.ui.HasWidgets;
 import com.vaadin.client.data.DataChangeHandler;
 import com.vaadin.client.data.DataSource;
 import com.vaadin.client.ui.AbstractComponentConnector;
+import com.vaadin.client.ui.grid.Cell;
 import com.vaadin.client.ui.grid.FlyweightCell;
 import com.vaadin.client.ui.grid.Grid;
 import com.vaadin.client.ui.grid.GridColumn;
@@ -331,6 +332,12 @@ public class GridClientColumnRendererConnector extends
 
                     super.setContentVisible(cell, hasData);
                 }
+
+                @Override
+                public boolean onActivate(Cell cell) {
+                    cell.getElement().setInnerHTML("<span>Activated!</span>");
+                    return true;
+                }
             };
 
         default: