Fix handling of hidden Components in Grid(#10367)

2 files changed, 156 insertions, 0 deletions

diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponentsVisibility.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponentsVisibility.java
new file mode 100644
index 0000000000..8c8810f7a8
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridComponentsVisibility.java
@@ -0,0 +1,66 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+import java.util.stream.IntStream;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.renderers.ComponentRenderer;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class GridComponentsVisibility extends AbstractTestUIWithLog {
+
+    private Map<String, TextField> textFields = new HashMap<>();
+    private int counter = 0;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Grid<String> grid = new Grid<>();
+        grid.addColumn(string -> new Label(string), new ComponentRenderer())
+            .setId("label").setCaption("Column 0");
+        grid.getDefaultHeaderRow().getCell("label")
+            .setComponent(new Label("Label"));
+        grid.addComponentColumn(string -> {
+            if (textFields.containsKey(string)) {
+                log("Reusing old text field for: " + string);
+                return textFields.get(string);
+            }
+            TextField textField = new TextField();
+            textField.setValue(string);
+            textField.setWidth("100%");
+            textFields.put(string, textField);
+            return textField;
+        }).setId("textField").setCaption("TextField");
+        grid.addColumn(string -> {
+
+            Button button = new Button("Click Me!",
+                event -> toggleFieldVisibility(string));
+            button.setId(string.replace(' ', '_').toLowerCase(Locale.ROOT));
+            return button;
+        }, new ComponentRenderer()).setId("button").setCaption("Button");
+        // make sure the buttons and focus outlines fit completely in a row
+        grid.setRowHeight(40);
+
+        grid.getDefaultHeaderRow().join("textField", "button")
+            .setText("Other Components");
+
+        addComponent(grid);
+        grid.setSizeFull();
+
+        grid.setItems(IntStream.range(0, 5).boxed()
+            .map(i -> "Row " + (i + (counter * 1000))));
+
+    }
+
+    private void toggleFieldVisibility(String string) {
+        textFields.get(string).setVisible(!textFields.get(string).isVisible());
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsVisibilityTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsVisibilityTest.java
new file mode 100644
index 0000000000..14fa82a58e
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsVisibilityTest.java
@@ -0,0 +1,90 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.function.Function;
+import java.util.logging.Level;
+import java.util.stream.IntStream;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class GridComponentsVisibilityTest extends MultiBrowserTest {
+
+    @Test
+    public void changingVisibilityOfComponentInFirstRowShouldNotThrowClientSideExceptions() {
+        testHideComponent(grid -> 0);
+    }
+
+    @Test
+    public void changingVisibilityOfComponentShouldNotThrowClientSideExceptions() {
+        testHideComponent(grid -> ThreadLocalRandom.current().nextInt(1, (int) grid.getRowCount() - 1));
+    }
+
+    @Test
+    public void changingVisibilityOfComponentInLastRowShouldNotThrowClientSideExceptions() {
+        testHideComponent(grid -> (int) grid.getRowCount() - 1);
+    }
+
+    private void testHideComponent(Function<GridElement, Integer> rowUnderTestSupplier) {
+        openTestURL("debug");
+        GridElement grid = $(GridElement.class).first();
+        int rowUnderTest = rowUnderTestSupplier.apply(grid);
+        assertTrue("Text field should be visible", grid.getCell(rowUnderTest, 1)
+            .isElementPresent(TextFieldElement.class));
+        assertOtherConnectorsArePresent(grid, rowUnderTest);
+
+        clearDebugMessages();
+        clickVisibilityToggleButton(grid, rowUnderTest);
+        assertNotClientSideErrors();
+        assertOnlyTextFieldOnTestedRowIsNotPresent(grid, rowUnderTest);
+
+        clearDebugMessages();
+        clickVisibilityToggleButton(grid, rowUnderTest);
+        assertNotClientSideErrors();
+        assertAllTextFieldsArePresent(grid, rowUnderTest);
+
+        clearDebugMessages();
+        clickVisibilityToggleButton(grid, rowUnderTest);
+        assertNotClientSideErrors();
+        assertOnlyTextFieldOnTestedRowIsNotPresent(grid, rowUnderTest);
+
+        clickVisibilityToggleButton(grid, rowUnderTest);
+    }
+
+    private void assertOnlyTextFieldOnTestedRowIsNotPresent(GridElement grid, int rowUnderTest) {
+        assertFalse("Text field should not be visible", grid.getCell(rowUnderTest, 1)
+            .isElementPresent(TextFieldElement.class));
+        assertOtherConnectorsArePresent(grid, rowUnderTest);
+    }
+
+    private void assertAllTextFieldsArePresent(GridElement grid, int rowUnderTest) {
+        assertTrue("Text field should be visible", grid.getCell(rowUnderTest, 1)
+            .isElementPresent(TextFieldElement.class));
+        assertOtherConnectorsArePresent(grid, rowUnderTest);
+    }
+
+    private void assertNotClientSideErrors() {
+        assertNoErrorNotifications();
+        // Should not log "Widget is still attached to the DOM ..." error message
+        assertNoDebugMessage(Level.SEVERE);
+    }
+
+    private void clickVisibilityToggleButton(GridElement grid, int rowUnderTest) {
+        grid.getRow(rowUnderTest).getCell(2).findElement(By.className("v-button")).click();
+    }
+
+    private void assertOtherConnectorsArePresent(GridElement grid, int rowUnderTest) {
+        IntStream.range(1, (int) grid.getRowCount())
+            .filter(row -> row != rowUnderTest)
+            .forEach(row ->
+                assertTrue("Text field should be visible", grid.getCell(row, 1)
+                    .isElementPresent(TextFieldElement.class))
+            );
+    }
+}