Adds details generator swap support for Grid (#16644)

Change-Id: I741970a7bcebd27d3aa28d608d767b4b4f063ae8

8 files changed, 291 insertions, 126 deletions

diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
index 1787dc5c97..e6b9c89483 100644
--- a/client/src/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -77,7 +77,7 @@ import com.vaadin.client.widgets.Grid.HeaderRow;
 import com.vaadin.shared.Connector;
 import com.vaadin.shared.data.sort.SortDirection;
 import com.vaadin.shared.ui.Connect;
-import com.vaadin.shared.ui.grid.ConnectorIndexChange;
+import com.vaadin.shared.ui.grid.DetailsConnectorChange;
 import com.vaadin.shared.ui.grid.EditorClientRpc;
 import com.vaadin.shared.ui.grid.EditorServerRpc;
 import com.vaadin.shared.ui.grid.GridClientRpc;
@@ -382,7 +382,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             }
         }
 
-        public void setDetailsConnectorChanges(Set<ConnectorIndexChange> changes) {
+        public void setDetailsConnectorChanges(
+                Set<DetailsConnectorChange> changes) {
             /*
              * To avoid overwriting connectors while moving them about, we'll
              * take all the affected connectors, first all remove those that are
@@ -390,7 +391,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
              */
 
             /* Remove moved/removed connectors from bookkeeping */
-            for (ConnectorIndexChange change : changes) {
+            for (DetailsConnectorChange change : changes) {
                 Integer oldIndex = change.getOldIndex();
                 Connector removedConnector = indexToDetailsMap.remove(oldIndex);
 
@@ -402,7 +403,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             }
 
             /* Add moved/added connectors to bookkeeping */
-            for (ConnectorIndexChange change : changes) {
+            for (DetailsConnectorChange change : changes) {
                 Integer newIndex = change.getNewIndex();
                 ComponentConnector connector = (ComponentConnector) change
                         .getConnector();
@@ -456,8 +457,11 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         }
 
         public void responseReceived(int fetchId) {
-            boolean success = pendingFetches.remove(fetchId);
-            assert success : "Received a response with an unidentified fetch id";
+            /* Ignore negative fetchIds (they're pushed, not fetched) */
+            if (fetchId >= 0) {
+                boolean success = pendingFetches.remove(fetchId);
+                assert success : "Received a response with an unidentified fetch id";
+            }
         }
 
         @Override
@@ -607,18 +611,20 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
             @Override
             public void setDetailsConnectorChanges(
-                    Set<ConnectorIndexChange> connectorChanges, int fetchId) {
+                    Set<DetailsConnectorChange> connectorChanges, int fetchId) {
                 customDetailsGenerator
                         .setDetailsConnectorChanges(connectorChanges);
 
                 // refresh moved/added details rows
-                for (ConnectorIndexChange change : connectorChanges) {
-                    Integer newIndex = change.getNewIndex();
-                    if (newIndex != null) {
-                        int index = newIndex.intValue();
-                        getWidget().setDetailsVisible(index, false);
-                        getWidget().setDetailsVisible(index, true);
+                for (DetailsConnectorChange change : connectorChanges) {
+                    Integer index = change.getNewIndex();
+                    if (index == null) {
+                        index = change.getOldIndex();
                     }
+
+                    int i = index.intValue();
+                    getWidget().setDetailsVisible(i, false);
+                    getWidget().setDetailsVisible(i, true);
                 }
                 detailsConnectorFetcher.responseReceived(fetchId);
             }
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index f4aaf798b7..8243782c4e 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -6387,12 +6387,13 @@ public class Grid<T> extends ResizeComposite implements
          * see GridSpacerUpdater.init for implementation details.
          */
 
-        if (visible && !isDetailsVisible(rowIndex)) {
+        boolean isVisible = isDetailsVisible(rowIndex);
+        if (visible && !isVisible) {
             escalator.getBody().setSpacer(rowIndex, DETAILS_ROW_INITIAL_HEIGHT);
             visibleDetails.add(rowIndexInteger);
         }
 
-        else if (!visible && isDetailsVisible(rowIndex)) {
+        else if (!visible && isVisible) {
             escalator.getBody().setSpacer(rowIndex, -1);
             visibleDetails.remove(rowIndexInteger);
         }
diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java
index 620933c379..97d141cd6e 100644
--- a/server/src/com/vaadin/data/RpcDataProviderExtension.java
+++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java
@@ -30,6 +30,7 @@ import java.util.logging.Logger;
 
 import com.google.gwt.thirdparty.guava.common.collect.BiMap;
 import com.google.gwt.thirdparty.guava.common.collect.HashBiMap;
+import com.google.gwt.thirdparty.guava.common.collect.ImmutableSet;
 import com.vaadin.data.Container.Indexed;
 import com.vaadin.data.Container.Indexed.ItemAddEvent;
 import com.vaadin.data.Container.Indexed.ItemRemoveEvent;
@@ -1214,4 +1215,10 @@ public class RpcDataProviderExtension extends AbstractExtension {
     public boolean isDetailsVisible(Object itemId) {
         return visibleDetails.contains(itemId);
     }
+
+    public void refreshDetails() {
+        for (Object itemId : ImmutableSet.copyOf(visibleDetails)) {
+            detailComponentManager.refresh(itemId);
+        }
+    }
 }
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index da5cedd999..ec1dd45536 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -38,6 +38,7 @@ import java.util.logging.Logger;
 
 import com.google.gwt.thirdparty.guava.common.collect.BiMap;
 import com.google.gwt.thirdparty.guava.common.collect.HashBiMap;
+import com.google.gwt.thirdparty.guava.common.collect.Maps;
 import com.google.gwt.thirdparty.guava.common.collect.Sets;
 import com.google.gwt.thirdparty.guava.common.collect.Sets.SetView;
 import com.vaadin.data.Container;
@@ -77,7 +78,7 @@ import com.vaadin.server.KeyMapper;
 import com.vaadin.server.VaadinSession;
 import com.vaadin.shared.MouseEventDetails;
 import com.vaadin.shared.data.sort.SortDirection;
-import com.vaadin.shared.ui.grid.ConnectorIndexChange;
+import com.vaadin.shared.ui.grid.DetailsConnectorChange;
 import com.vaadin.shared.ui.grid.EditorClientRpc;
 import com.vaadin.shared.ui.grid.EditorServerRpc;
 import com.vaadin.shared.ui.grid.GridClientRpc;
@@ -2840,7 +2841,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          * {@link Collection#isEmpty() empty}, then this field is consistent
          * with the connector hierarchy.
          */
-        private final Map<Object, Component> visibleDetailsComponents = new HashMap<Object, Component>();
+        private final Map<Object, Component> visibleDetailsComponents = Maps
+                .newHashMap();
 
         /** A lookup map for which row contains which details component. */
         private BiMap<Integer, Component> rowIndexToDetails = HashBiMap
@@ -2863,7 +2865,13 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          * could find out the same thing by taking out all the other components
          * and checking whether Grid is their parent or not.
          */
-        private final Set<Component> unattachedComponents = new HashSet<Component>();
+        private final Set<Component> unattachedComponents = Sets.newHashSet();
+
+        /**
+         * Keeps tabs on all the details that did not get a component during
+         * {@link #createDetails(Object, int)}.
+         */
+        private final Map<Object, Integer> emptyDetails = Maps.newHashMap();
 
         /**
          * Creates a details component by the request of the client side, with
@@ -2887,14 +2895,14 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             assert itemId != null : "itemId was null";
             Integer newRowIndex = Integer.valueOf(rowIndex);
 
-            assert !visibleDetailsComponents.containsKey(itemId) : "itemId already has a component. Should be destroyed first.";
+            assert !visibleDetailsComponents.containsKey(itemId) : "itemId "
+                    + "already has a component. Should be destroyed first.";
 
             RowReference rowReference = new RowReference(Grid.this);
             rowReference.set(itemId);
 
             Component details = getDetailsGenerator().getDetails(rowReference);
             if (details != null) {
-
                 if (details.getParent() != null) {
                     String generatorName = getDetailsGenerator().getClass()
                             .getName();
@@ -2916,6 +2924,20 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 visibleDetailsComponents.put(itemId, details);
                 rowIndexToDetails.put(newRowIndex, details);
                 unattachedComponents.add(details);
+
+                assert !emptyDetails.containsKey(itemId) : "Bookeeping thinks "
+                        + "itemId is empty even though we just created a "
+                        + "component for it (" + itemId + ")";
+            } else {
+                assert !emptyDetails.containsKey(itemId) : "Bookkeeping has "
+                        + "already itemId marked as empty (itemId: " + itemId
+                        + ", old index: " + emptyDetails.get(itemId)
+                        + ", new index: " + newRowIndex + ")";
+                assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping"
+                        + " already had another itemId for this empty index "
+                        + "(index: " + newRowIndex + ", new itemId: " + itemId
+                        + ")";
+                emptyDetails.put(itemId, newRowIndex);
             }
 
             /*
@@ -2934,6 +2956,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          *            the item id for which to destroy the details component
          */
         public void destroyDetails(Object itemId) {
+            emptyDetails.remove(itemId);
+
             Component removedComponent = visibleDetailsComponents
                     .remove(itemId);
             if (removedComponent == null) {
@@ -2972,8 +2996,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          * 
          * @return information on how the connectors have changed
          */
-        Set<ConnectorIndexChange> getAndResetConnectorChanges() {
-            Set<ConnectorIndexChange> changes = new HashSet<ConnectorIndexChange>();
+        Set<DetailsConnectorChange> getAndResetConnectorChanges() {
+            Set<DetailsConnectorChange> changes = new HashSet<DetailsConnectorChange>();
 
             // populate diff with added/changed
             for (Entry<Integer, Component> entry : rowIndexToDetails.entrySet()) {
@@ -2996,7 +3020,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 }
 
                 if (!SharedUtil.equals(oldIndex, newIndex)) {
-                    changes.add(new ConnectorIndexChange(component, oldIndex,
+                    changes.add(new DetailsConnectorChange(component, oldIndex,
                             newIndex));
                 }
             }
@@ -3008,7 +3032,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 Component component = entry.getValue();
                 Integer newIndex = rowIndexToDetails.inverse().get(component);
                 if (newIndex == null) {
-                    changes.add(new ConnectorIndexChange(null, oldIndex, null));
+                    changes.add(new DetailsConnectorChange(null, oldIndex, null));
                 }
             }
 
@@ -3017,6 +3041,21 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
 
             return changes;
         }
+
+        public void refresh(Object itemId) {
+            Component component = visibleDetailsComponents.get(itemId);
+            Integer rowIndex = null;
+            if (component != null) {
+                rowIndex = rowIndexToDetails.inverse().get(component);
+                destroyDetails(itemId);
+            } else {
+                rowIndex = emptyDetails.remove(itemId);
+            }
+
+            assert rowIndex != null : "Given itemId does not map to an existing detail row ("
+                    + itemId + ")";
+            createDetails(itemId, rowIndex.intValue());
+        }
     }
 
     /**
@@ -5374,7 +5413,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
 
         this.detailsGenerator = detailsGenerator;
 
-        getLogger().warning("[[details]] update details on generator swap");
+        datasourceExtension.refreshDetails();
+        getRpcProxy(GridClientRpc.class).setDetailsConnectorChanges(
+                detailComponentManager.getAndResetConnectorChanges(), -1);
     }
 
     /**
diff --git a/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java
index 16be92007e..40f4541fb1 100644
--- a/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java
+++ b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java
@@ -26,14 +26,14 @@ import com.vaadin.shared.Connector;
  * @since
  * @author Vaadin Ltd
  */
-public class ConnectorIndexChange implements Serializable {
+public class DetailsConnectorChange implements Serializable {
 
     private Connector connector;
     private Integer oldIndex;
     private Integer newIndex;
 
     /** Create a new connector index change */
-    public ConnectorIndexChange() {
+    public DetailsConnectorChange() {
     }
 
     /**
@@ -49,7 +49,7 @@ public class ConnectorIndexChange implements Serializable {
      * @param newIndex
      *            the new index
      */
-    public ConnectorIndexChange(Connector connector, Integer oldIndex,
+    public DetailsConnectorChange(Connector connector, Integer oldIndex,
             Integer newIndex) {
         this.connector = connector;
         this.oldIndex = oldIndex;
@@ -59,8 +59,12 @@ public class ConnectorIndexChange implements Serializable {
     }
 
     private boolean assertStateIsOk() {
-        assert (connector != null && newIndex != null)
-                || (connector == null && oldIndex != null && newIndex == null) : "connector: "
+        boolean connectorAndNewIndexIsNotNull = connector != null
+                && newIndex != null;
+        boolean connectorAndNewIndexIsNullThenOldIndexIsSet = connector == null
+                && newIndex == null && oldIndex != null;
+
+        assert (connectorAndNewIndexIsNotNull || connectorAndNewIndexIsNullThenOldIndexIsSet) : "connector: "
                 + nullityString(connector)
                 + ", oldIndex: "
                 + nullityString(oldIndex)
diff --git a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java
index 672c83ff53..98e7fac567 100644
--- a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java
+++ b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java
@@ -65,9 +65,10 @@ public interface GridClientRpc extends ClientRpc {
      * @param connectorChanges
      *            the indexing changes of details connectors
      * @param fetchId
-     *            the id of the request for fetching the changes
+     *            the id of the request for fetching the changes. A negative
+     *            number indicates a push (not requested by the client side)
      */
     public void setDetailsConnectorChanges(
-            Set<ConnectorIndexChange> connectorChanges, int fetchId);
+            Set<DetailsConnectorChange> connectorChanges, int fetchId);
 
 }
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 08f0d7d5d2..63fb903f53 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
@@ -47,10 +47,12 @@ import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
 import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.Component;
+import com.vaadin.ui.CssLayout;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.CellReference;
 import com.vaadin.ui.Grid.CellStyleGenerator;
 import com.vaadin.ui.Grid.Column;
+import com.vaadin.ui.Grid.DetailsGenerator;
 import com.vaadin.ui.Grid.FooterCell;
 import com.vaadin.ui.Grid.HeaderCell;
 import com.vaadin.ui.Grid.HeaderRow;
@@ -60,6 +62,7 @@ import com.vaadin.ui.Grid.RowStyleGenerator;
 import com.vaadin.ui.Grid.SelectionMode;
 import com.vaadin.ui.Grid.SelectionModel;
 import com.vaadin.ui.Label;
+import com.vaadin.ui.Notification;
 import com.vaadin.ui.Panel;
 import com.vaadin.ui.renderers.DateRenderer;
 import com.vaadin.ui.renderers.HtmlRenderer;
@@ -114,6 +117,53 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
 
     private Panel detailsPanel;
 
+    private final DetailsGenerator detailedDetailsGenerator = new DetailsGenerator() {
+        @Override
+        public Component getDetails(final RowReference rowReference) {
+            CssLayout cssLayout = new CssLayout();
+            cssLayout.setHeight("200px");
+            cssLayout.setWidth("100%");
+
+            Item item = rowReference.getItem();
+            for (Object propertyId : item.getItemPropertyIds()) {
+                Property<?> prop = item.getItemProperty(propertyId);
+                String string = prop.getValue().toString();
+                cssLayout.addComponent(new Label(string));
+            }
+
+            final int rowIndex = grid.getContainerDataSource().indexOfId(
+                    rowReference.getItemId());
+            ClickListener clickListener = new ClickListener() {
+                @Override
+                public void buttonClick(ClickEvent event) {
+                    Notification.show("You clicked on the "
+                            + "button in the details for " + "row " + rowIndex);
+                }
+            };
+            cssLayout.addComponent(new Button("Press me", clickListener));
+            return cssLayout;
+        }
+    };
+
+    private final DetailsGenerator watchingDetailsGenerator = new DetailsGenerator() {
+        private int id = 0;
+
+        @Override
+        public Component getDetails(RowReference rowReference) {
+            return new Label("You are watching item id "
+                    + rowReference.getItemId() + " (" + (id++) + ")");
+        }
+    };
+
+    private final DetailsGenerator hierarchicalDetailsGenerator = new DetailsGenerator() {
+        @Override
+        public Component getDetails(RowReference rowReference) {
+            detailsPanel = new Panel();
+            detailsPanel.setContent(new Label("One"));
+            return detailsPanel;
+        }
+    };
+
     @Override
     @SuppressWarnings("unchecked")
     protected Grid constructComponent() {
@@ -1059,40 +1109,32 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
     }
 
     private void createDetailsActions() {
-        createClickAction("custom details generator", "Details",
-                new Command<Grid, Void>() {
-                    @Override
-                    public void execute(Grid c, Void value, Object data) {
-                        grid.setDetailsGenerator(new Grid.DetailsGenerator() {
-                            private int seq = 0;
+        Command<Grid, DetailsGenerator> swapDetailsGenerator = new Command<Grid, DetailsGenerator>() {
+            @Override
+            public void execute(Grid c, DetailsGenerator generator, Object data) {
+                grid.setDetailsGenerator(generator);
+            }
+        };
 
-                            @Override
-                            public Component getDetails(
-                                    RowReference rowReference) {
-                                return new Label("You are watching item id "
-                                        + rowReference.getItemId() + " ("
-                                        + (seq++) + ")");
-                            }
-                        });
-                    }
-                }, null);
-        createClickAction("hierarchy details generator", "Details",
-                new Command<Grid, Void>() {
-                    @Override
-                    public void execute(Grid c, Void value, Object data) {
-                        grid.setDetailsGenerator(new Grid.DetailsGenerator() {
-                            @Override
-                            public Component getDetails(
-                                    RowReference rowReference) {
-                                detailsPanel = new Panel();
-                                detailsPanel.setContent(new Label("One"));
-                                return detailsPanel;
-                            }
-                        });
-                    }
-                }, null);
+        Command<Grid, Boolean> openOrCloseItemId = new Command<Grid, Boolean>() {
+            @Override
+            @SuppressWarnings("boxing")
+            public void execute(Grid g, Boolean visible, Object itemId) {
+                g.setDetailsVisible(itemId, visible);
+            }
+        };
 
-        createClickAction("change hierarchy in generator", "Details",
+        createCategory("Generators", "Details");
+        createClickAction("NULL", "Generators", swapDetailsGenerator,
+                DetailsGenerator.NULL);
+        createClickAction("\"Watching\"", "Generators", swapDetailsGenerator,
+                watchingDetailsGenerator);
+        createClickAction("Detailed", "Generators", swapDetailsGenerator,
+                detailedDetailsGenerator);
+        createClickAction("Hierarchical", "Generators", swapDetailsGenerator,
+                hierarchicalDetailsGenerator);
+
+        createClickAction("- Change Component", "Generators",
                 new Command<Grid, Void>() {
                     @Override
                     public void execute(Grid c, Void value, Object data) {
@@ -1105,7 +1147,7 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                     }
                 }, null);
 
-        createClickAction("toggle firstItemId", "Details",
+        createClickAction("Toggle firstItemId", "Details",
                 new Command<Grid, Void>() {
                     @Override
                     public void execute(Grid g, Void value, Object data) {
@@ -1117,26 +1159,11 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                     }
                 }, null);
 
-        createBooleanAction("firstItemId", "Details", false,
-                new Command<Grid, Boolean>() {
-                    @Override
-                    @SuppressWarnings("boxing")
-                    public void execute(Grid g, Boolean visible, Object data) {
-                        g.setDetailsVisible(g.getContainerDataSource()
-                                .firstItemId(), visible);
-                    }
-                });
+        createBooleanAction("Open firstItemId", "Details", false,
+                openOrCloseItemId, ds.firstItemId());
 
-        createBooleanAction("lastItemId-5", "Details", false,
-                new Command<Grid, Boolean>() {
-                    @Override
-                    @SuppressWarnings("boxing")
-                    public void execute(Grid g, Boolean visible, Object data) {
-                        Object fifthLastItemId = g.getContainerDataSource()
-                                .getItemIds(ROWS - 6, 1).get(0);
-                        g.setDetailsVisible(fifthLastItemId, visible);
-                    }
-                });
+        createBooleanAction("Open 995", "Details", false, openOrCloseItemId,
+                ds.getIdByIndex(995));
     }
 
     @Override
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
index e9b5b688d1..f3f58b002e 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java
@@ -16,6 +16,7 @@
 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.assertTrue;
 import static org.junit.Assert.fail;
@@ -27,7 +28,7 @@ import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
 
 import com.vaadin.testbench.TestBenchElement;
-import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures;
+import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
 
 public class GridDetailsServerTest extends GridBasicFeaturesTest {
@@ -37,20 +38,21 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
      * able to scroll that particular details row into view, making tests
      * awkward with two scroll commands back to back.
      */
-    private static final int ALMOST_LAST_ITEM_INDEX = GridBasicFeatures.ROWS - 5;
-    private static final String[] ALMOST_LAST_ITEM_DETAILS = new String[] {
-            "Component", "Details", "lastItemId-5" };
-
-    private static final String[] FIRST_ITEM_DETAILS = new String[] {
-            "Component", "Details", "firstItemId" };
+    private static final int ALMOST_LAST_INDEX = 995;
+    private static final String[] OPEN_ALMOST_LAST_ITEM_DETAILS = new String[] {
+            "Component", "Details", "Open " + ALMOST_LAST_INDEX };
+    private static final String[] OPEN_FIRST_ITEM_DETAILS = new String[] {
+            "Component", "Details", "Open firstItemId" };
     private static final String[] TOGGLE_FIRST_ITEM_DETAILS = new String[] {
-            "Component", "Details", "toggle firstItemId" };
-    private static final String[] CUSTOM_DETAILS_GENERATOR = new String[] {
-            "Component", "Details", "custom details generator" };
-    private static final String[] HIERARCHY_DETAILS_GENERATOR = new String[] {
-            "Component", "Details", "hierarchy details generator" };
+            "Component", "Details", "Toggle firstItemId" };
+    private static final String[] DETAILS_GENERATOR_NULL = new String[] {
+            "Component", "Details", "Generators", "NULL" };
+    private static final String[] DETAILS_GENERATOR_WATCHING = new String[] {
+            "Component", "Details", "Generators", "\"Watching\"" };
+    private static final String[] DETAILS_GENERATOR_HIERARCHICAL = new String[] {
+            "Component", "Details", "Generators", "Hierarchical" };
     private static final String[] CHANGE_HIERARCHY = new String[] {
-            "Component", "Details", "change hierarchy in generator" };
+            "Component", "Details", "Generators", "- Change Component" };
 
     @Before
     public void setUp() {
@@ -65,41 +67,42 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
         } catch (NoSuchElementException ignore) {
             // expected
         }
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         assertNotNull("details should've opened", getGridElement()
                 .getDetails(0));
     }
 
     @Test(expected = NoSuchElementException.class)
     public void closeVisibleDetails() {
-        selectMenuPath(FIRST_ITEM_DETAILS);
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
 
         getGridElement().getDetails(0);
     }
 
     @Test
-    public void openDetailsOutsideOfActiveRange() {
+    public void openDetailsOutsideOfActiveRange() throws InterruptedException {
         getGridElement().scroll(10000);
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         getGridElement().scroll(0);
+        Thread.sleep(50);
         assertNotNull("details should've been opened", getGridElement()
                 .getDetails(0));
     }
 
     @Test(expected = NoSuchElementException.class)
     public void closeDetailsOutsideOfActiveRange() {
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         getGridElement().scroll(10000);
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         getGridElement().scroll(0);
         getGridElement().getDetails(0);
     }
 
     @Test
     public void componentIsVisibleClientSide() {
-        selectMenuPath(CUSTOM_DETAILS_GENERATOR);
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
 
         TestBenchElement details = getGridElement().getDetails(0);
         assertNotNull("No widget detected inside details",
@@ -107,11 +110,11 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     }
 
     @Test
-    public void togglingAVisibleDetailsRowWithSeparateRoundtrips() {
-        selectMenuPath(CUSTOM_DETAILS_GENERATOR);
-        selectMenuPath(FIRST_ITEM_DETAILS); // open
-        selectMenuPath(FIRST_ITEM_DETAILS); // close
-        selectMenuPath(FIRST_ITEM_DETAILS); // open
+    public void openingDetailsTwice() {
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // open
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // close
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // open
 
         TestBenchElement details = getGridElement().getDetails(0);
         assertNotNull("No widget detected inside details",
@@ -120,7 +123,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test(expected = NoSuchElementException.class)
     public void scrollingDoesNotCreateAFloodOfDetailsRows() {
-        selectMenuPath(CUSTOM_DETAILS_GENERATOR);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
 
         // scroll somewhere to hit uncached rows
         getGridElement().scrollToRow(101);
@@ -133,8 +136,8 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     public void openingDetailsOutOfView() {
         getGridElement().scrollToRow(500);
 
-        selectMenuPath(CUSTOM_DETAILS_GENERATOR);
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
 
         getGridElement().scrollToRow(0);
 
@@ -145,8 +148,8 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test
     public void togglingAVisibleDetailsRowWithOneRoundtrip() {
-        selectMenuPath(CUSTOM_DETAILS_GENERATOR);
-        selectMenuPath(FIRST_ITEM_DETAILS); // open
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // open
 
         assertTrue("Unexpected generator content",
                 getGridElement().getDetails(0).getText().endsWith("(0)"));
@@ -156,36 +159,111 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     }
 
     @Test
-    @Ignore("This will be patched with https://dev.vaadin.com/review/#/c/7917/")
     public void almosLastItemIdIsRendered() {
-        selectMenuPath(CUSTOM_DETAILS_GENERATOR);
-        selectMenuPath(ALMOST_LAST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(OPEN_ALMOST_LAST_ITEM_DETAILS);
         scrollGridVerticallyTo(100000);
 
         TestBenchElement details = getGridElement().getDetails(
-                ALMOST_LAST_ITEM_INDEX);
+                ALMOST_LAST_INDEX);
         assertNotNull(details);
         assertTrue("Unexpected details content",
-                details.getText().endsWith(ALMOST_LAST_ITEM_INDEX + " (0)"));
+                details.getText().endsWith(ALMOST_LAST_INDEX + " (0)"));
     }
 
     @Test
     public void hierarchyChangesWorkInDetails() {
-        selectMenuPath(HIERARCHY_DETAILS_GENERATOR);
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         assertEquals("One", getGridElement().getDetails(0).getText());
         selectMenuPath(CHANGE_HIERARCHY);
         assertEquals("Two", getGridElement().getDetails(0).getText());
     }
 
+    @Ignore("This use case is not currently supported by Grid. If the detail "
+            + "is out of view, the component is detached from the UI and a "
+            + "new instance is generated when scrolled back. Support will "
+            + "maybe be incorporated at a later time")
     @Test
-    @Ignore("This will be patched with https://dev.vaadin.com/review/#/c/7917/")
     public void hierarchyChangesWorkInDetailsWhileOutOfView() {
-        selectMenuPath(HIERARCHY_DETAILS_GENERATOR);
-        selectMenuPath(FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         scrollGridVerticallyTo(10000);
         selectMenuPath(CHANGE_HIERARCHY);
         scrollGridVerticallyTo(0);
         assertEquals("Two", getGridElement().getDetails(0).getText());
     }
+
+    @Test
+    public void swappingDetailsGenerators_noDetailsShown() {
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(DETAILS_GENERATOR_NULL);
+        assertFalse("Got some errors", $(NotificationElement.class).exists());
+    }
+
+    @Test
+    public void swappingDetailsGenerators_shownDetails() {
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        assertTrue("Details should be empty at the start", getGridElement()
+                .getDetails(0).getText().isEmpty());
+
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        assertFalse("Details should not be empty after swapping generator",
+                getGridElement().getDetails(0).getText().isEmpty());
+    }
+
+    @Test
+    public void swappingDetailsGenerators_whileDetailsScrolledOut_showNever() {
+        scrollGridVerticallyTo(1000);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        assertFalse("Got some errors", $(NotificationElement.class).exists());
+    }
+
+    @Test
+    public void swappingDetailsGenerators_whileDetailsScrolledOut_showAfter() {
+        scrollGridVerticallyTo(1000);
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        scrollGridVerticallyTo(0);
+
+        assertFalse("Got some errors", $(NotificationElement.class).exists());
+        assertNotNull("Could not find a details", getGridElement()
+                .getDetails(0));
+    }
+
+    @Test
+    public void swappingDetailsGenerators_whileDetailsScrolledOut_showBefore() {
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        scrollGridVerticallyTo(1000);
+
+        assertFalse("Got some errors", $(NotificationElement.class).exists());
+        assertNotNull("Could not find a details", getGridElement()
+                .getDetails(0));
+    }
+
+    @Test
+    public void swappingDetailsGenerators_whileDetailsScrolledOut_showBeforeAndAfter() {
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        scrollGridVerticallyTo(1000);
+        scrollGridVerticallyTo(0);
+
+        assertFalse("Got some errors", $(NotificationElement.class).exists());
+        assertNotNull("Could not find a details", getGridElement()
+                .getDetails(0));
+    }
+
+    @Test
+    public void nullDetailComponentToggling() {
+        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        selectMenuPath(DETAILS_GENERATOR_NULL);
+        assertTrue("Details should be empty with null component",
+                getGridElement().getDetails(0).getText().isEmpty());
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
+        assertFalse("Details should be not empty with details component",
+                getGridElement().getDetails(0).getText().isEmpty());
+    }
 }