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

Change-Id: Ib853205dae7745ca4af11fb558899e6648e25da8

4 files changed, 48 insertions, 55 deletions

diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java
index 33f66d35be..a395d7e721 100644
--- a/uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java
+++ b/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));
         }
 
@@ -138,23 +136,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();
         useGenerator(true);
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java
index 0717cd84d0..a169e701c0 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java
+++ b/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);
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsClientTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsClientTest.java
index 73f9d584a6..01e7e52923 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsClientTest.java
+++ b/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,11 +127,25 @@ 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);
         toggleDetailsFor(1);
@@ -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()));
     }
 
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 326dbcd55f..a9ab7027db 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
@@ -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);
@@ -268,24 +273,6 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
     }
 
     @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);
         scrollGridVerticallyTo(500);