Do not open detail row if generator is the NULL one (#18663)

Change-Id: Ib853205dae7745ca4af11fb558899e6648e25da8

client/src/com/vaadin/client/widgets/Grid.java

@@ -8226,6 +8226,10 @@ public class Grid<T> extends ResizeComposite implements
      * @see #isDetailsVisible(int)
      */
     public void setDetailsVisible(int rowIndex, boolean visible) {
+        if (DetailsGenerator.NULL.equals(detailsGenerator)) {
+            return;
+        }
+
         Integer rowIndexInteger = Integer.valueOf(rowIndex);
 
         /*

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

@@ -6597,6 +6597,9 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
      *            to hide them
      */
     public void setDetailsVisible(Object itemId, boolean visible) {
+        if (DetailsGenerator.NULL.equals(detailsGenerator)) {
+            return;
+        }
         datasourceExtension.setDetailsVisible(itemId, visible);
     }
 

uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java

@@ -86,9 +86,7 @@ public class GridDetailsLocationTest extends MultiBrowserTest {
 
         for (int rowIndex : params) {
 
-            data.add(new Param(rowIndex, false, false));
             data.add(new Param(rowIndex, true, false));
-            data.add(new Param(rowIndex, false, true));
             data.add(new Param(rowIndex, true, true));
         }
 
@@ -137,23 +135,6 @@ public class GridDetailsLocationTest extends MultiBrowserTest {
         }
     }
 
-    @Test
-    public void testDetailsHeightWithNoGenerator() {
-        openTestURL();
-        toggleAndScroll(5);
-
-        verifyDetailsRowHeight(5, detailsDefaultHeight, 0);
-        verifyDetailsDecoratorLocation(5, 0, 0);
-
-        toggleAndScroll(0);
-
-        verifyDetailsRowHeight(0, detailsDefaultHeight, 0);
-        verifyDetailsDecoratorLocation(0, 0, 1);
-
-        verifyDetailsRowHeight(5, detailsDefaultHeight, 1);
-        verifyDetailsDecoratorLocation(5, 1, 0);
-    }
-
     @Test
     public void testDetailsHeightWithGenerator() {
         openTestURL();

uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java

@@ -823,6 +823,7 @@ public class GridColumnHidingTest extends GridBasicClientFeaturesTest {
 
     @Test
     public void testColumnHiding_detailsRowIsOpen_renderedCorrectly() {
+        selectMenuPath("Component", "Row details", "Set generator");
         selectMenuPath("Component", "Row details", "Toggle details for...",
                 "Row 1");
         assertColumnHeaderOrder(0, 1, 2, 3, 4);

uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsClientTest.java

@@ -17,6 +17,7 @@ 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;
@@ -58,14 +59,10 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
                 getGridElement().getDetails(1));
     }
 
-    @Test
-    public void nullRendererShowsDetailsPlaceholder() {
+    @Test(expected = NoSuchElementException.class)
+    public void nullRendererDoesNotShowDetailsPlaceholder() {
         toggleDetailsFor(1);
-        TestBenchElement details = getGridElement().getDetails(1);
-        assertNotNull("details for row 1 should not exist at the start",
-                details);
-        assertTrue("details should've been empty for null renderer", details
-                .getText().isEmpty());
+        getGridElement().getDetails(1);
     }
 
     @Test
@@ -78,6 +75,14 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
                 details.getText().startsWith("Row: 1."));
     }
 
+    @Test(expected = NoSuchElementException.class)
+    public void openDetailsThenAppyRendererShouldNotShowDetails() {
+        toggleDetailsFor(1);
+        selectMenuPath(SET_GENERATOR);
+
+        getGridElement().getDetails(1);
+    }
+
     @Test
     public void openHiddenDetailsThenScrollToIt() {
         try {
@@ -122,10 +127,24 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
         toggleDetailsFor(1);
 
         selectMenuPath(SET_FAULTY_GENERATOR);
-
         getGridElement().getDetails(1);
     }
 
+    @Test
+    public void settingNewGeneratorStillWorksAfterError() {
+        selectMenuPath(SET_FAULTY_GENERATOR);
+        toggleDetailsFor(1);
+        $(FixedNotificationElement.class).first().close();
+        toggleDetailsFor(1);
+
+        selectMenuPath(SET_GENERATOR);
+        toggleDetailsFor(1);
+
+        assertNotEquals(
+                "New details should've been generated even after error", "",
+                getGridElement().getDetails(1).getText());
+    }
+
     @Test
     public void updaterRendersExpectedWidgets() {
         selectMenuPath(SET_GENERATOR);
@@ -171,6 +190,7 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
 
     @Test
     public void rowElementClassNames() {
+        selectMenuPath(SET_GENERATOR);
         toggleDetailsFor(0);
         toggleDetailsFor(1);
 
@@ -183,25 +203,28 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
 
     @Test
     public void scrollDownToRowWithDetails() {
+        selectMenuPath(SET_GENERATOR);
         toggleDetailsFor(100);
         scrollToRow(100, ScrollDestination.ANY);
 
-        Range validScrollRange = Range.between(1700, 1715);
+        Range validScrollRange = Range.between(1691, 1706);
         assertTrue(validScrollRange.contains(getGridVerticalScrollPos()));
     }
 
     @Test
     public void scrollUpToRowWithDetails() {
+        selectMenuPath(SET_GENERATOR);
         toggleDetailsFor(100);
         scrollGridVerticallyTo(999999);
         scrollToRow(100, ScrollDestination.ANY);
 
-        Range validScrollRange = Range.between(1990, 2010);
+        Range validScrollRange = Range.between(1981, 2001);
         assertTrue(validScrollRange.contains(getGridVerticalScrollPos()));
     }
 
     @Test
     public void cannotScrollBeforeTop() {
+        selectMenuPath(SET_GENERATOR);
         toggleDetailsFor(1);
         scrollToRow(0, ScrollDestination.END);
         assertEquals(0, getGridVerticalScrollPos());
@@ -209,10 +232,11 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
 
     @Test
     public void cannotScrollAfterBottom() {
+        selectMenuPath(SET_GENERATOR);
         toggleDetailsFor(999);
         scrollToRow(999, ScrollDestination.START);
 
-        Range expectedRange = Range.withLength(19680, 20);
+        Range expectedRange = Range.withLength(19671, 20);
         assertTrue(expectedRange.contains(getGridVerticalScrollPos()));
     }
 

uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java

@@ -58,7 +58,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
         openTestURL();
     }
 
-    @Test
+    @Test(expected = NoSuchElementException.class)
     public void openVisibleDetails() {
         try {
             getGridElement().getDetails(0);
@@ -67,8 +67,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
             // expected
         }
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
-        assertNotNull("details should've opened", getGridElement()
-                .getDetails(0));
+        getGridElement().getDetails(0);
     }
 
     @Test(expected = NoSuchElementException.class)
@@ -98,6 +97,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     @Test
     public void openDetailsOutsideOfActiveRange() throws InterruptedException {
         getGridElement().scroll(10000);
+        selectMenuPath(DETAILS_GENERATOR_WATCHING);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         getGridElement().scroll(0);
         Thread.sleep(50);
@@ -215,13 +215,15 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test
     public void swappingDetailsGenerators_shownDetails() {
+        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
-        assertTrue("Details should be empty at the start", getGridElement()
-                .getDetails(0).getText().isEmpty());
+        assertTrue("Details should contain 'One' at first", getGridElement()
+                .getDetails(0).getText().contains("One"));
 
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
-        assertFalse("Details should not be empty after swapping generator",
-                getGridElement().getDetails(0).getText().isEmpty());
+        assertFalse(
+                "Details should contain 'Watching' after swapping generator",
+                getGridElement().getDetails(0).getText().contains("Watching"));
     }
 
     @Test
@@ -235,6 +237,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     @Test
     public void swappingDetailsGenerators_whileDetailsScrolledOut_showAfter() {
         scrollGridVerticallyTo(1000);
+        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
         scrollGridVerticallyTo(0);
@@ -246,6 +249,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test
     public void swappingDetailsGenerators_whileDetailsScrolledOut_showBefore() {
+        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
         scrollGridVerticallyTo(1000);
@@ -257,6 +261,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
 
     @Test
     public void swappingDetailsGenerators_whileDetailsScrolledOut_showBeforeAndAfter() {
+        selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
         selectMenuPath(DETAILS_GENERATOR_WATCHING);
         scrollGridVerticallyTo(1000);
@@ -267,24 +272,6 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
                 .getDetails(0));
     }
 
-    @Test
-    public void nullDetailComponentToggling() {
-        selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
-        selectMenuPath(DETAILS_GENERATOR_WATCHING);
-        selectMenuPath(DETAILS_GENERATOR_NULL);
-
-        try {
-            assertTrue("Details should be empty with null component",
-                    getGridElement().getDetails(0).getText().isEmpty());
-        } catch (NoSuchElementException e) {
-            fail("Expected to find a details row with empty content");
-        }
-
-        selectMenuPath(DETAILS_GENERATOR_WATCHING);
-        assertFalse("Details should be not empty with details component",
-                getGridElement().getDetails(0).getText().isEmpty());
-    }
-
     @Test
     public void noAssertErrorsOnEmptyDetailsAndScrollDown() {
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);