Merged column headers should work within declarative Grid. (#12206) (#…

…12223) * Merged column headers should work within declarative Grid. (#12206) - null check to prevent NPE from the merged column headers - convert to internal columnIds for merge handling - use correct cell in merge handling - switch away from streams in merge handling for easier readability - regression test Fixes: #10464
vaadin · Mar 5, 2021 · 82ee2b6 · 82ee2b6
1 parent 41be9fc
commit 82ee2b6
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 10 deletions.
diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java
@@ -4750,7 +4750,8 @@ private void readStructure(Element design, DesignContext context) {
             for (Column<T, ?> c : getColumns()) {
                 HeaderCell headerCell = getDefaultHeaderRow().getCell(c);
                 if (headerCell.getCellType() == GridStaticCellType.TEXT) {
-                    c.setCaption(headerCell.getText());
+                    String text = headerCell.getText();
+                    c.setCaption(text == null ? "" : text);
                 }
             }
         }

diff --git a/server/src/main/java/com/vaadin/ui/components/grid/StaticSection.java b/server/src/main/java/com/vaadin/ui/components/grid/StaticSection.java
@@ -290,18 +290,22 @@ protected void readDesign(Element trElement,
                                     + columnIdsString + "'");
                 }
 
-                Stream.of(columnIds).forEach(this::addCell);
-
-                Stream<String> idsStream = Stream.of(columnIds);
+                CELL cell;
                 if (colspan > 1) {
-                    CELL newCell = createCell();
-                    addMergedCell(createCell(),
-                            idsStream.collect(Collectors.toSet()));
-                    newCell.readDesign(element, designContext);
+                    Set<String> columnGroup = new HashSet<>();
+                    for (String columnId : columnIds) {
+                        addCell(columnId);
+                        // convert the public columnIds into internal columnIds
+                        columnGroup.add(getCell(columnId).getColumnId());
+                    }
+                    cell = createCell();
+                    addMergedCell(cell, columnGroup);
                 } else {
-                    idsStream.map(this::getCell).forEach(
-                            cell -> cell.readDesign(element, designContext));
+                    String columnId = columnIds[0];
+                    addCell(columnId);
+                    cell = getCell(columnId);
                 }
+                cell.readDesign(element, designContext);
             }
         }
 

diff --git a/uitest/src/main/java/com/vaadin/tests/declarative/DeclarativeGrid.java b/uitest/src/main/java/com/vaadin/tests/declarative/DeclarativeGrid.java
@@ -0,0 +1,65 @@
+package com.vaadin.tests.declarative;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import java.io.ByteArrayInputStream;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.declarative.Design;
+import com.vaadin.ui.declarative.DesignContext;
+
+public class DeclarativeGrid extends AbstractTestUI {
+
+    private String design = "" + //
+            "   <vaadin-grid>\n" + //
+            "    <table>\n" + //
+            "     <colgroup>\n" + //
+            "      <col column-id=\"project\" sortable=\"false\">\n" + //
+            "      <col column-id=\"status\" sortable=\"false\">\n" + //
+            "      <col column-id=\"date\" sortable=\"false\">\n" + //
+            "     </colgroup>\n" + //
+            "     <thead>\n" + //
+            "      <tr default>\n" + //
+            "       <th plain-text column-ids=\"project,status\" " + //
+            "          colspan=\"2\">Project and Status</th>\n" + //
+            "       <th plain-text column-ids=\"date\">Date</th>\n" + //
+            "      </tr>\n" + //
+            "     </thead>\n" + //
+            "     <tbody>\n" + //
+            "      <tr item=\"project1\">\n" + //
+            "       <td>Customer Project 1</td>\n" + //
+            "       <td>OK</td>\n" + //
+            "       <td>2020-12-31</td>\n" + //
+            "      </tr>\n" + //
+            "      <tr item=\"project2\">\n" + //
+            "       <td>Customer Project 2</td>\n" + //
+            "       <td>OK</td>\n" + //
+            "       <td>2020-07-02</td>\n" + //
+            "      </tr>\n" + //
+            "      <tr item=\"project3\">\n" + //
+            "       <td>Customer Project 3</td>\n" + //
+            "       <td>OK</td>\n" + //
+            "       <td>2019-10-01</td>\n" + //
+            "      </tr>\n" + //
+            "     </tbody>\n" + //
+            "    </table>\n" + //
+            "   </vaadin-grid>";
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        DesignContext dc = Design
+                .read(new ByteArrayInputStream(design.getBytes(UTF_8)), null);
+        addComponent(dc.getRootComponent());
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 10464;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Merged column header should not cause an exception.";
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/declarative/DeclarativeGridTest.java b/uitest/src/test/java/com/vaadin/tests/declarative/DeclarativeGridTest.java
@@ -0,0 +1,19 @@
+package com.vaadin.tests.declarative;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class DeclarativeGridTest extends MultiBrowserTest {
+
+    @Test
+    public void testMergedHeaderCell() {
+        openTestURL();
+        waitForElementPresent(By.className("v-label"));
+        // ensure the grid gets loaded and has the merged header
+        GridElement grid = $(GridElement.class).first();
+        grid.getHeaderCellByCaption("Project and Status");
+    }
+}