Adds Widget support for DetailsGenerator (#16644)

Change-Id: Ib964b2aa102b8c56e65b0af87bed008248038599

4 files changed, 195 insertions, 15 deletions

diff --git a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java
index 665362ca8e..264aa4e614 100644
--- a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java
+++ b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java
@@ -15,6 +15,8 @@
  */
 package com.vaadin.client.widget.grid;
 
+import com.google.gwt.user.client.ui.Widget;
+
 /**
  * A callback interface for generating details for a particular row in Grid.
  * 
@@ -25,7 +27,7 @@ public interface DetailsGenerator {
 
     public static final DetailsGenerator NULL = new DetailsGenerator() {
         @Override
-        public String getDetails(int rowIndex) {
+        public Widget getDetails(int rowIndex) {
             return null;
         }
     };
@@ -40,6 +42,5 @@ public interface DetailsGenerator {
      *         details empty.
      */
     // TODO: provide a row object instead of index (maybe, needs discussion?)
-    // TODO: return a Widget instead of a String
-    String getDetails(int rowIndex);
+    Widget getDetails(int rowIndex);
 }
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 7890b13904..c0936bccec 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -2776,19 +2776,44 @@ public class Grid<T> extends ResizeComposite implements
     }
 
     private class GridSpacerUpdater implements SpacerUpdater {
+
+        private final Map<Element, Widget> elementToWidgetMap = new HashMap<Element, Widget>();
+
         @Override
         public void init(Spacer spacer) {
+
+            assert spacer.getElement().getFirstChild() == null : "The spacer's"
+                    + " element should be empty at this point. (row: "
+                    + spacer.getRow() + ", child: "
+                    + spacer.getElement().getFirstChild() + ")";
+
             int rowIndex = spacer.getRow();
 
-            String string = detailsGenerator.getDetails(rowIndex);
-            if (string == null) {
-                destroy(spacer);
+            Widget detailsWidget = null;
+            try {
+                detailsWidget = detailsGenerator.getDetails(rowIndex);
+            } catch (Throwable e) {
+                getLogger().log(
+                        Level.SEVERE,
+                        "Exception while generating details for row "
+                                + rowIndex, e);
+            }
+
+            if (detailsWidget == null) {
+                spacer.getElement().removeAllChildren();
                 escalator.getBody().setSpacer(rowIndex,
                         DETAILS_ROW_INITIAL_HEIGHT);
                 return;
             }
 
-            spacer.getElement().setInnerText(string);
+            Element element = detailsWidget.getElement();
+            spacer.getElement().appendChild(element);
+            setParent(detailsWidget, Grid.this);
+            Widget previousWidget = elementToWidgetMap.put(element,
+                    detailsWidget);
+
+            assert previousWidget == null : "Overwrote a pre-existing widget on row "
+                    + rowIndex + " without proper removal first.";
 
             /*
              * Once we have the content properly inside the DOM, we should
@@ -2803,7 +2828,30 @@ public class Grid<T> extends ResizeComposite implements
 
         @Override
         public void destroy(Spacer spacer) {
-            spacer.getElement().setInnerText("");
+
+            assert getElement().isOrHasChild(spacer.getElement()) : "Trying "
+                    + "to destroy a spacer that is not connected to this "
+                    + "Grid's DOM. (row: " + spacer.getRow() + ", element: "
+                    + spacer.getElement() + ")";
+
+            Widget detailsWidget = elementToWidgetMap.remove(spacer
+                    .getElement().getFirstChildElement());
+
+            if (detailsWidget != null) {
+                /*
+                 * The widget may be null here if the previous generator
+                 * returned a null widget.
+                 */
+
+                assert spacer.getElement().getFirstChild() != null : "The "
+                        + "details row to destroy did not contain a widget - "
+                        + "probably removed by something else without "
+                        + "permission? (row: " + spacer.getRow()
+                        + ", element: " + spacer.getElement() + ")";
+
+                setParent(detailsWidget, null);
+                spacer.getElement().removeAllChildren();
+            }
         }
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsTest.java
index 981a1cc217..5e8793c964 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsTest.java
@@ -15,6 +15,9 @@
  */
 package com.vaadin.tests.components.grid.basicfeatures.client;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -23,14 +26,22 @@ import static org.junit.Assert.fail;
 import org.junit.Before;
 import org.junit.Test;
 import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.WebElement;
 
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.ElementQuery;
 import com.vaadin.testbench.TestBenchElement;
+import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicClientFeaturesTest;
 
 public class GridDetailsTest extends GridBasicClientFeaturesTest {
 
     private static final String[] SET_GENERATOR = new String[] { "Component",
             "Row details", "Set generator" };
+    private static final String[] SET_FAULTY_GENERATOR = new String[] {
+            "Component", "Row details", "Set faulty generator" };
+    private static final String[] SET_EMPTY_GENERATOR = new String[] {
+            "Component", "Row details", "Set empty generator" };
     private static final String[] TOGGLE_DETAILS_FOR_ROW_1 = new String[] {
             "Component", "Row details", "Toggle details for row 1" };
     private static final String[] TOGGLE_DETAILS_FOR_ROW_100 = new String[] {
@@ -38,6 +49,7 @@ public class GridDetailsTest extends GridBasicClientFeaturesTest {
 
     @Before
     public void setUp() {
+        setDebug(true);
         openTestURL();
     }
 
@@ -96,4 +108,77 @@ public class GridDetailsTest extends GridBasicClientFeaturesTest {
         assertTrue("Unexpected details content",
                 details.getText().startsWith("Row: 100."));
     }
+
+    @Test
+    public void errorUpdaterShowsErrorNotification() {
+        assertFalse("No notifications should've been at the start",
+                $(NotificationElement.class).exists());
+
+        selectMenuPath(TOGGLE_DETAILS_FOR_ROW_1);
+        selectMenuPath(SET_FAULTY_GENERATOR);
+
+        ElementQuery<NotificationElement> notification = $(NotificationElement.class);
+        assertTrue("Was expecting an error notification here",
+                notification.exists());
+        notification.first().closeNotification();
+
+        assertEquals("The error details element should be empty", "",
+                getGridElement().getDetails(1).getText());
+    }
+
+    @Test
+    public void updaterStillWorksAfterError() {
+        selectMenuPath(TOGGLE_DETAILS_FOR_ROW_1);
+
+        selectMenuPath(SET_FAULTY_GENERATOR);
+        $(NotificationElement.class).first().closeNotification();
+        selectMenuPath(SET_GENERATOR);
+
+        assertNotEquals(
+                "New details should've been generated even after error", "",
+                getGridElement().getDetails(1).getText());
+    }
+
+    @Test
+    public void updaterRendersExpectedWidgets() {
+        selectMenuPath(SET_GENERATOR);
+        selectMenuPath(TOGGLE_DETAILS_FOR_ROW_1);
+
+        TestBenchElement detailsElement = getGridElement().getDetails(1);
+        assertNotNull(detailsElement.findElement(By.className("gwt-Label")));
+        assertNotNull(detailsElement.findElement(By.className("gwt-Button")));
+    }
+
+    @Test
+    public void widgetsInUpdaterWorkAsExpected() {
+        selectMenuPath(SET_GENERATOR);
+        selectMenuPath(TOGGLE_DETAILS_FOR_ROW_1);
+
+        TestBenchElement detailsElement = getGridElement().getDetails(1);
+        WebElement button = detailsElement.findElement(By
+                .className("gwt-Button"));
+        button.click();
+
+        WebElement label = detailsElement
+                .findElement(By.className("gwt-Label"));
+        assertEquals("clicked", label.getText());
+    }
+
+    @Test
+    public void emptyGenerator() {
+        selectMenuPath(SET_EMPTY_GENERATOR);
+        selectMenuPath(TOGGLE_DETAILS_FOR_ROW_1);
+
+        assertEquals("empty generator did not produce an empty details row",
+                "", getGridElement().getDetails(1).getText());
+    }
+
+    @Test(expected = NoSuchElementException.class)
+    public void removeDetailsRow() {
+        selectMenuPath(SET_GENERATOR);
+        selectMenuPath(TOGGLE_DETAILS_FOR_ROW_1);
+        selectMenuPath(TOGGLE_DETAILS_FOR_ROW_1);
+
+        getGridElement().getDetails(1);
+    }
 }
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
index 7c2ca3eedb..110b14c721 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
@@ -33,9 +33,11 @@ import com.google.gwt.user.client.Timer;
 import com.google.gwt.user.client.Window;
 import com.google.gwt.user.client.ui.Button;
 import com.google.gwt.user.client.ui.Composite;
+import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.HTML;
 import com.google.gwt.user.client.ui.Label;
 import com.google.gwt.user.client.ui.TextBox;
+import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.data.DataSource;
 import com.vaadin.client.data.DataSource.RowHandle;
 import com.vaadin.client.renderers.DateRenderer;
@@ -1224,17 +1226,59 @@ public class GridBasicClientFeaturesWidget extends
             public void execute() {
                 grid.setDetailsGenerator(new DetailsGenerator() {
                     @Override
-                    public String getDetails(int rowIndex) {
-                        return "Row: " + rowIndex + ". Lorem ipsum "
-                                + "dolor sit amet, consectetur adipiscing "
-                                + "elit. Morbi congue massa non augue "
-                                + "pulvinar, nec consectetur justo efficitur. "
-                                + "In nec arcu sit amet lorem hendrerit "
-                                + "mollis.";
+                    public Widget getDetails(int rowIndex) {
+                        FlowPanel panel = new FlowPanel();
+
+                        final Label label = new Label("Row: " + rowIndex + ".");
+                        Button button = new Button("Button",
+                                new ClickHandler() {
+                                    @Override
+                                    public void onClick(ClickEvent event) {
+                                        label.setText("clicked");
+                                    }
+                                });
+
+                        panel.add(label);
+                        panel.add(button);
+                        return panel;
                     }
                 });
             }
         }, menupath);
+
+        addMenuCommand("Set faulty generator", new ScheduledCommand() {
+            @Override
+            public void execute() {
+                grid.setDetailsGenerator(new DetailsGenerator() {
+                    @Override
+                    public Widget getDetails(int rowIndex) {
+                        throw new RuntimeException("This is by design.");
+                    }
+                });
+            }
+        }, menupath);
+
+        addMenuCommand("Set empty generator", new ScheduledCommand() {
+            @Override
+            public void execute() {
+                grid.setDetailsGenerator(new DetailsGenerator() {
+                    /*
+                     * While this is functionally equivalent to the NULL
+                     * generator, it's good to be explicit, since the behavior
+                     * isn't strictly tied between them. NULL generator might be
+                     * changed to render something different by default, and an
+                     * empty generator might behave differently also in the
+                     * future.
+                     */
+
+                    @Override
+                    public Widget getDetails(int rowIndex) {
+                        return null;
+                    }
+                });
+            }
+        }, menupath);
+
         addMenuCommand("Toggle details for row 1", new ScheduledCommand() {
             boolean visible = false;
 
@@ -1244,6 +1288,7 @@ public class GridBasicClientFeaturesWidget extends
                 grid.setDetailsVisible(1, visible);
             }
         }, menupath);
+
         addMenuCommand("Toggle details for row 100", new ScheduledCommand() {
             boolean visible = false;
 
@@ -1253,5 +1298,6 @@ public class GridBasicClientFeaturesWidget extends
                 grid.setDetailsVisible(100, visible);
             }
         }, menupath);
+
     }
 }