Redesign user-defined column identifiers for Grid (#7983)

Closes vaadin/framework8-issues#494

4 files changed, 108 insertions, 109 deletions

diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java
index 9cdab40e01..dabefcc9ac 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java
@@ -56,8 +56,8 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         HeightMode heightMode = HeightMode.ROW;
         double heightByRows = 13.7d;
 
-        grid.addColumn(Person::getFirstName);
-        grid.addColumn("id", Person::getLastName);
+        grid.addColumn(Person::getFirstName).setCaption("First Name");
+        grid.addColumn(Person::getLastName).setId("id").setCaption("Id");
 
         grid.setFrozenColumnCount(frozenColumns);
         grid.setSelectionMode(SelectionMode.MULTI);
@@ -66,9 +66,9 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
 
         String design = String.format(
                 "<%s height-mode='%s' frozen-columns='%d' rows='%s' selection-mode='%s'><table><colgroup>"
-                        + "<col column-id='generatedColumn0' sortable>"
+                        + "<col column-id='column0' sortable>"
                         + "<col column-id='id' sortable>" + "</colgroup><thead>"
-                        + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
+                        + "<tr default><th plain-text column-ids='column0'>First Name</th>"
                         + "<th plain-text column-ids='id'>Id</th></tr>"
                         + "</thead></table></%s>",
                 getComponentTag(),
@@ -85,11 +85,12 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
     public void mergedHeaderCells() {
         Grid<Person> grid = new Grid<>();
 
-        Column<Person, String> column1 = grid.addColumn(Person::getFirstName);
-        Column<Person, String> column2 = grid.addColumn("id",
-                Person::getLastName);
-        Column<Person, String> column3 = grid.addColumn("mail",
-                Person::getEmail);
+        Column<Person, String> column1 = grid.addColumn(Person::getFirstName)
+                .setCaption("First Name");
+        Column<Person, String> column2 = grid.addColumn(Person::getLastName)
+                .setId("id").setCaption("Id");
+        Column<Person, String> column3 = grid.addColumn(Person::getEmail)
+                .setId("mail").setCaption("Mail");
 
         HeaderRow header = grid.addHeaderRowAt(1);
         String headerRowText1 = "foo";
@@ -99,17 +100,19 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         String headerRowText3 = "foobar";
         join.setText(headerRowText3);
 
-        String design = String.format("<%s><table><colgroup>"
-                + "<col column-id='generatedColumn0' sortable>"
-                + "<col column-id='id' sortable>"
-                + "<col column-id='mail' sortable>" + "</colgroup><thead>"
-                + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
-                + "<th plain-text column-ids='id'>Id</th>"
-                + "<th plain-text column-ids='mail'>Mail</th></tr>"
-                + "<tr><th plain-text column-ids='generatedColumn0'>%s</th>"
-                + "<th colspan='2' plain-text column-ids='id,mail'>foobar</th></tr>"
-                + "</thead></table></%s>", getComponentTag(), headerRowText1,
-                headerRowText3, getComponentTag());
+        String design = String.format(
+                "<%s><table><colgroup>" + "<col column-id='column0' sortable>"
+                        + "<col column-id='id' sortable>"
+                        + "<col column-id='mail' sortable>"
+                        + "</colgroup><thead>"
+                        + "<tr default><th plain-text column-ids='column0'>First Name</th>"
+                        + "<th plain-text column-ids='id'>Id</th>"
+                        + "<th plain-text column-ids='mail'>Mail</th></tr>"
+                        + "<tr><th plain-text column-ids='column0'>%s</th>"
+                        + "<th colspan='2' plain-text column-ids='id,mail'>foobar</th></tr>"
+                        + "</thead></table></%s>",
+                getComponentTag(), headerRowText1, headerRowText3,
+                getComponentTag());
 
         testRead(design, grid);
         testWrite(design, grid);
@@ -119,11 +122,12 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
     public void mergedFooterCells() {
         Grid<Person> grid = new Grid<>();
 
-        Column<Person, String> column1 = grid.addColumn(Person::getFirstName);
-        Column<Person, String> column2 = grid.addColumn("id",
-                Person::getLastName);
-        Column<Person, String> column3 = grid.addColumn("mail",
-                Person::getEmail);
+        Column<Person, String> column1 = grid.addColumn(Person::getFirstName)
+                .setCaption("First Name");
+        Column<Person, String> column2 = grid.addColumn(Person::getLastName)
+                .setId("id").setCaption("Id");
+        Column<Person, String> column3 = grid.addColumn(Person::getEmail)
+                .setId("mail").setCaption("Mail");
 
         FooterRow footer = grid.addFooterRowAt(0);
 
@@ -137,17 +141,19 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         String footerRowText2 = "foobar";
         footer.join(cell2, cell3).setHtml(footerRowText2);
 
-        String design = String.format("<%s><table><colgroup>"
-                + "<col column-id='generatedColumn0' sortable>"
-                + "<col column-id='id' sortable>"
-                + "<col column-id='mail' sortable>" + "</colgroup><thead>"
-                + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
-                + "<th plain-text column-ids='id'>Id</th>"
-                + "<th plain-text column-ids='mail'>Mail</th></tr></thead>"
-                + "<tfoot><tr><td plain-text column-ids='generatedColumn0'>%s</td>"
-                + "<td colspan='2' column-ids='id,mail'>%s</td></tr></tfoot>"
-                + "</table></%s>", getComponentTag(), footerRowText1,
-                footerRowText2, getComponentTag());
+        String design = String.format(
+                "<%s><table><colgroup>" + "<col column-id='column0' sortable>"
+                        + "<col column-id='id' sortable>"
+                        + "<col column-id='mail' sortable>"
+                        + "</colgroup><thead>"
+                        + "<tr default><th plain-text column-ids='column0'>First Name</th>"
+                        + "<th plain-text column-ids='id'>Id</th>"
+                        + "<th plain-text column-ids='mail'>Mail</th></tr></thead>"
+                        + "<tfoot><tr><td plain-text column-ids='column0'>%s</td>"
+                        + "<td colspan='2' column-ids='id,mail'>%s</td></tr></tfoot>"
+                        + "</table></%s>",
+                getComponentTag(), footerRowText1, footerRowText2,
+                getComponentTag());
 
         testRead(design, grid);
         testWrite(design, grid);
@@ -158,9 +164,10 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         Grid<Person> grid = new Grid<>();
 
         String secondColumnId = "id";
-        Column<Person, String> column1 = grid.addColumn(Person::getFirstName);
-        Column<Person, String> column2 = grid.addColumn(secondColumnId,
-                Person::getLastName);
+        Column<Person, String> column1 = grid.addColumn(Person::getFirstName)
+                .setCaption("First Name");
+        Column<Person, String> column2 = grid.addColumn(Person::getLastName)
+                .setId(secondColumnId).setCaption("Id");
 
         String caption = "test-caption";
         column1.setCaption(caption);
@@ -189,10 +196,10 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
 
         String design = String.format(
                 "<%s><table><colgroup>"
-                        + "<col column-id='generatedColumn0' sortable='%s' editable resizable='%s' hidable hidden>"
+                        + "<col column-id='column0' sortable='%s' editable resizable='%s' hidable hidden>"
                         + "<col column-id='id' sortable hiding-toggle-caption='%s' width='%s' min-width='%s' max-width='%s' expand='%s'>"
                         + "</colgroup><thead>"
-                        + "<tr default><th plain-text column-ids='generatedColumn0'>%s</th>"
+                        + "<tr default><th plain-text column-ids='column0'>%s</th>"
                         + "<th plain-text column-ids='id'>%s</th>"
                         + "</tr></thead>" + "</table></%s>",
                 getComponentTag(), sortable, resizable, hidingToggleCaption,
@@ -207,22 +214,24 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
     public void headerFooterSerialization() {
         Grid<Person> grid = new Grid<>();
 
-        Column<Person, String> column1 = grid.addColumn(Person::getFirstName);
-        Column<Person, String> column2 = grid.addColumn("id",
-                Person::getLastName);
+        Column<Person, String> column1 = grid.addColumn(Person::getFirstName)
+                .setCaption("First Name");
+        Column<Person, String> column2 = grid.addColumn(Person::getLastName)
+                .setId("id").setCaption("Id");
 
         FooterRow footerRow = grid.addFooterRowAt(0);
         footerRow.getCell(column1).setText("x");
         footerRow.getCell(column2).setHtml("y");
 
-        String design = String.format("<%s><table><colgroup>"
-                + "<col column-id='generatedColumn0' sortable>"
-                + "<col column-id='id' sortable></colgroup><thead>"
-                + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
-                + "<th plain-text column-ids='id'>Id</th></tr>"
-                + "</thead><tbody></tbody>"
-                + "<tfoot><tr><td plain-text column-ids='generatedColumn0'>x</td>"
-                + "<td column-ids='id'>y</td></tr></tfoot>" + "</table></%s>",
+        String design = String.format(
+                "<%s><table><colgroup>" + "<col column-id='column0' sortable>"
+                        + "<col column-id='id' sortable></colgroup><thead>"
+                        + "<tr default><th plain-text column-ids='column0'>First Name</th>"
+                        + "<th plain-text column-ids='id'>Id</th></tr>"
+                        + "</thead><tbody></tbody>"
+                        + "<tfoot><tr><td plain-text column-ids='column0'>x</td>"
+                        + "<td column-ids='id'>y</td></tr></tfoot>"
+                        + "</table></%s>",
                 getComponentTag(), getComponentTag());
 
         testRead(design, grid);
@@ -238,14 +247,13 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         Person person2 = createPerson("name", "last-name");
         grid.setItems(person1, person2);
 
-        grid.addColumn(Person::getFirstName);
-        grid.addColumn("id", Person::getLastName);
+        grid.addColumn(Person::getFirstName).setCaption("First Name");
+        grid.addColumn(Person::getLastName).setId("id").setCaption("Id");
 
         String design = String.format(
-                "<%s><table><colgroup>"
-                        + "<col column-id='generatedColumn0' sortable>"
+                "<%s><table><colgroup>" + "<col column-id='column0' sortable>"
                         + "<col column-id='id' sortable></colgroup><thead>"
-                        + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
+                        + "<tr default><th plain-text column-ids='column0'>First Name</th>"
                         + "<th plain-text column-ids='id'>Id</th></tr>"
                         + "</thead><tbody>"
                         + "<tr item='%s'><td>%s</td><td>%s</td></tr>"
@@ -281,8 +289,8 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         Person person3 = createPerson("foo", "last-name");
         grid.setItems(person1, person2, person3);
 
-        grid.addColumn(Person::getFirstName);
-        grid.addColumn("id", Person::getLastName);
+        grid.addColumn(Person::getFirstName).setCaption("First Name");
+        grid.addColumn(Person::getLastName).setId("id").setCaption("Id");
 
         Multi<Person> model = (Multi<Person>) grid
                 .setSelectionMode(SelectionMode.MULTI);
@@ -290,9 +298,9 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
 
         String design = String.format(
                 "<%s selection-mode='multi'><table><colgroup>"
-                        + "<col column-id='generatedColumn0' sortable>"
+                        + "<col column-id='column0' sortable>"
                         + "<col column-id='id' sortable></colgroup><thead>"
-                        + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
+                        + "<tr default><th plain-text column-ids='column0'>First Name</th>"
                         + "<th plain-text column-ids='id'>Id</th></tr>"
                         + "</thead><tbody>"
                         + "<tr item='%s' selected><td>%s</td><td>%s</td></tr>"
@@ -318,18 +326,17 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         Person person2 = createPerson("name", "last-name");
         grid.setItems(person1, person2);
 
-        grid.addColumn(Person::getFirstName);
-        grid.addColumn("id", Person::getLastName);
+        grid.addColumn(Person::getFirstName).setCaption("First Name");
+        grid.addColumn(Person::getLastName).setId("id").setCaption("Id");
 
         Single<Person> model = (Single<Person>) grid
                 .setSelectionMode(SelectionMode.SINGLE);
         model.select(person2);
 
         String design = String.format(
-                "<%s><table><colgroup>"
-                        + "<col column-id='generatedColumn0' sortable>"
+                "<%s><table><colgroup>" + "<col column-id='column0' sortable>"
                         + "<col column-id='id' sortable></colgroup><thead>"
-                        + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
+                        + "<tr default><th plain-text column-ids='column0'>First Name</th>"
                         + "<th plain-text column-ids='id'>Id</th></tr>"
                         + "</thead><tbody>"
                         + "<tr item='%s'><td>%s</td><td>%s</td></tr>"
@@ -353,16 +360,16 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         Person person2 = createPerson("name", "last-name");
         grid.setItems(person1, person2);
 
-        grid.addColumn(Person::getFirstName);
-        grid.addColumn("id", Person::getLastName);
+        grid.addColumn(Person::getFirstName).setCaption("First Name");
+        grid.addColumn(Person::getLastName).setId("id").setCaption("Id");
 
         grid.setSelectionMode(SelectionMode.MULTI);
         grid.asMultiSelect().setReadOnly(true);
 
         String formatString = "<%s %s selection-allowed><table><colgroup>"
-                + "<col column-id='generatedColumn0' sortable>"
+                + "<col column-id='column0' sortable>"
                 + "<col column-id='id' sortable>" + "</colgroup><thead>"
-                + "<tr default><th plain-text column-ids='generatedColumn0'>Generated Column0</th>"
+                + "<tr default><th plain-text column-ids='column0'>First Name</th>"
                 + "<th plain-text column-ids='id'>Id</th></tr>"
                 + "</thead><tbody>"
                 + "<tr item='%s'><td>%s</td><td>%s</td></tr>"
@@ -394,7 +401,8 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
     @Test
     public void testComponentInGridHeader() {
         Grid<Person> grid = new Grid<>();
-        Column<Person, String> column = grid.addColumn(Person::getFirstName);
+        Column<Person, String> column = grid.addColumn(Person::getFirstName)
+                .setCaption("First Name");
 
         String html = "<b>Foo</b>";
         Label component = new Label(html);
@@ -403,16 +411,15 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         //@formatter:off
         String design = String.format( "<%s><table>"
                 + "<colgroup>"
-                + "   <col sortable column-id='generatedColumn0'>"
+                + "   <col sortable column-id='column0'>"
                 + "</colgroup>"
                 + "<thead>"
-                + "<tr default><th column-ids='generatedColumn0'><vaadin-label>%s</vaadin-label></th></tr>"
+                + "<tr default><th column-ids='column0'><vaadin-label>%s</vaadin-label></th></tr>"
                 + "</thead>"
                 + "</table></%s>", getComponentTag(), html, getComponentTag());
         //@formatter:on
 
-        grid.getDefaultHeaderRow().getCell(column.getId())
-                .setComponent(component);
+        grid.getDefaultHeaderRow().getCell(column).setComponent(component);
 
         testRead(design, grid, true);
         testWrite(design, grid);
@@ -421,7 +428,8 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
     @Test
     public void testComponentInGridFooter() {
         Grid<Person> grid = new Grid<>();
-        Column<Person, String> column = grid.addColumn(Person::getFirstName);
+        Column<Person, String> column = grid.addColumn(Person::getFirstName)
+                .setCaption("First Name");
 
         String html = "<b>Foo</b>";
         Label component = new Label(html);
@@ -433,11 +441,11 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
       //@formatter:off
         String design = String.format( "<%s><table>"
                 + "<colgroup>"
-                + "   <col sortable column-id='generatedColumn0'>"
+                + "   <col sortable column-id='column0'>"
                 + "</colgroup>"
                 + "<thead>"
                 +"<tfoot>"
-                + "<tr><td column-ids='generatedColumn0'><vaadin-label>%s</vaadin-label></td></tr>"
+                + "<tr><td column-ids='column0'><vaadin-label>%s</vaadin-label></td></tr>"
                 + "</tfoot>"
                 + "</table>"
                 + "</%s>", getComponentTag(), html, getComponentTag());
@@ -452,14 +460,14 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         //@formatter:off
         String design = "<vaadin-grid><table>"
                 + "<colgroup>"
-                + "   <col sortable column-id='generatedColumn0'>"
+                + "   <col sortable column-id='column0'>"
                 + "</colgroup>"
                 + "<thead />"
                 + "</table>"
                 + "</vaadin-grid>";
         //@formatter:on
         Grid<Person> grid = new Grid<>();
-        grid.addColumn(Person::getFirstName);
+        grid.addColumn(Person::getFirstName).setCaption("First Name");
         grid.removeHeaderRow(grid.getDefaultHeaderRow());
 
         testWrite(design, grid);
@@ -509,7 +517,7 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
                 + "<tr><td %s column-ids='%s'>&gt; Test</td></tr>"
                 + "</tfoot>"
                 + "<tbody />"
-                + "</table></%s>", 
+                + "</table></%s>",
                 getComponentTag() , id, plainText, id, plainText, id, getComponentTag());
         //@formatter:on
 
@@ -531,8 +539,8 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest<Grid> {
         Assert.assertEquals(expected, actualFooter);
 
         grid = new Grid<>();
-        Column<Person, String> column = grid.addColumn(id,
-                Person::getFirstName);
+        Column<Person, String> column = grid.addColumn(Person::getFirstName)
+                .setId(id);
         HeaderRow header = grid.addHeaderRowAt(0);
         FooterRow footer = grid.addFooterRowAt(0);
         grid.removeHeaderRow(grid.getDefaultHeaderRow());
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDefaultHeaderTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDefaultHeaderTest.java
index b2ce1abd2f..41cdbc8310 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDefaultHeaderTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDefaultHeaderTest.java
@@ -1,12 +1,12 @@
 /*
  * Copyright 2000-2016 Vaadin Ltd.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -35,8 +35,10 @@ public class GridDefaultHeaderTest {
     public void setUp() {
         grid = new Grid<>();
 
-        column1 = grid.addColumn("First", ValueProvider.identity());
-        column2 = grid.addColumn("Second", ValueProvider.identity());
+        column1 = grid.addColumn(ValueProvider.identity()).setId("First")
+                .setCaption("First");
+        column2 = grid.addColumn(ValueProvider.identity()).setId("Second")
+                .setCaption("Second");
     }
 
     @Test
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridHeaderFooterTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridHeaderFooterTest.java
index 055d4712cc..ca6e7dd9f6 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridHeaderFooterTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridHeaderFooterTest.java
@@ -139,16 +139,18 @@ public class GridHeaderFooterTest {
 
     @Test
     public void addColumn_headerCellAdded() {
-        Column<?, ?> column = grid.addColumn("Col",
-                ValueProvider.identity());
+
+        Column<?, ?> column = grid.addColumn(ValueProvider.identity())
+                .setId("Col");
 
         assertNotNull(grid.getHeaderRow(0).getCell(column));
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void removeColumn_headerCellRemoved() {
-        Column<String, ?> column = grid.addColumn("Col",
-                ValueProvider.identity());
+
+        Column<String, ?> column = grid.addColumn(ValueProvider.identity())
+                .setId("Col");
         grid.removeColumn(column);
 
         grid.getHeaderRow(0).getCell(column);
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java
index 6955a99e91..d657fc055a 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridTest.java
@@ -30,9 +30,10 @@ public class GridTest {
     @Before
     public void setUp() {
         grid = new Grid<>();
-        grid.addColumn("foo", ValueProvider.identity());
+
+        grid.addColumn(ValueProvider.identity()).setId("foo");
         grid.addColumn(String::length, new NumberRenderer());
-        grid.addColumn("randomColumnId", ValueProvider.identity());
+        grid.addColumn(ValueProvider.identity()).setId("randomColumnId");
     }
 
     @Test
@@ -73,23 +74,9 @@ public class GridTest {
                 grid.getHeaderRow(0).getCell("foo").getText());
     }
 
-    @Test
-    public void testGridColumnGeneratedIdentifier() {
-        assertEquals("Unexpected caption on a generated Column",
-                "Generated Column0",
-                grid.getColumn("generatedColumn0").getCaption());
-    }
-
-    @Test
-    public void testGridColumnCaptionFromIdentifier() {
-        assertEquals("Unexpected caption on a generated Column",
-                "Random Column Id",
-                grid.getColumn("randomColumnId").getCaption());
-    }
-
     @Test(expected = IllegalArgumentException.class)
     public void testGridMultipleColumnsWithSameIdentifier() {
-        grid.addColumn("foo", t -> t);
+        grid.addColumn(t -> t).setId("foo");
     }
 
     @Test