From b49b8c2fc5112c21aaf4d3562bdbfa8eb3fee9a9 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Fri, 7 Oct 2016 11:37:42 +0300 Subject: [PATCH] Use identifiers for Grid Columns Change-Id: Id229e533fc4ff58bdd2ce3862481f72210ed9e89 --- .../connectors/grid/ColumnConnector.java | 24 +++-- .../client/connectors/grid/GridConnector.java | 19 ++-- server/src/main/java/com/vaadin/ui/Grid.java | 98 ++++++++++++++----- .../tests/server/component/grid/GridTest.java | 26 ++++- .../components/grid/basics/GridBasics.java | 47 ++++----- .../grid/basics/GridColumnReorderTest.java | 6 +- 6 files changed, 157 insertions(+), 63 deletions(-) diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java index cc3dff651d..c3aa4fe5d2 100644 --- a/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java +++ b/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java @@ -36,7 +36,20 @@ import elemental.json.JsonValue; @Connect(com.vaadin.ui.Grid.Column.class) public class ColumnConnector extends AbstractExtensionConnector { - private Column column; + static abstract class CustomColumn extends Column { + + private final String connectorId; + + CustomColumn(String connectorId) { + this.connectorId = connectorId; + } + + public String getConnectorId() { + return connectorId; + } + } + + private CustomColumn column; /* This parent is needed because it's no longer available in onUnregister */ private GridConnector parent; @@ -44,16 +57,15 @@ public class ColumnConnector extends AbstractExtensionConnector { @Override protected void extend(ServerConnector target) { parent = getParent(); - String columnId = getState().id; - column = new Column() { + column = new CustomColumn(getConnectorId()) { @Override public Object getValue(JsonObject row) { final JsonObject rowData = row .getObject(DataCommunicatorConstants.DATA); - if (rowData.hasKey(columnId)) { - final JsonValue columnValue = rowData.get(columnId); + if (rowData.hasKey(getConnectorId())) { + final JsonValue columnValue = rowData.get(getConnectorId()); return getRendererConnector().decode(columnValue); } @@ -62,7 +74,7 @@ public class ColumnConnector extends AbstractExtensionConnector { } }; column.setRenderer(getRendererConnector().getRenderer()); - getParent().addColumn(column, columnId); + getParent().addColumn(column, getState().id); } @SuppressWarnings("unchecked") diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java index 02909e1fc3..4686b3bd21 100644 --- a/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java +++ b/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java @@ -38,6 +38,7 @@ import com.vaadin.client.TooltipInfo; import com.vaadin.client.WidgetUtil; import com.vaadin.client.annotations.OnStateChange; import com.vaadin.client.connectors.AbstractListingConnector; +import com.vaadin.client.connectors.grid.ColumnConnector.CustomColumn; import com.vaadin.client.data.DataSource; import com.vaadin.client.ui.SimpleManagedLayout; import com.vaadin.client.widget.grid.CellReference; @@ -107,8 +108,8 @@ public class GridConnector } /* Map to keep track of all added columns */ - private Map, String> columnToIdMap = new HashMap<>(); - private Map> idToColumn = new HashMap<>(); + private Map columnToIdMap = new HashMap<>(); + private Map idToColumn = new HashMap<>(); /* Child component list for HasComponentsConnector */ private List childComponents; @@ -134,7 +135,7 @@ public class GridConnector * the id of the column to get * @return the column with the given id */ - public Column getColumn(String columnId) { + public CustomColumn getColumn(String columnId) { return idToColumn.get(columnId); } @@ -164,8 +165,8 @@ public class GridConnector } Column column = cellRef.getColumn(); - if (columnToIdMap.containsKey(column)) { - String id = columnToIdMap.get(column); + if (column instanceof CustomColumn) { + String id = ((CustomColumn) column).getConnectorId(); JsonObject cellStyles = row .getObject(GridState.JSONKEY_CELLSTYLES); if (cellStyles.hasKey(id)) { @@ -284,7 +285,7 @@ public class GridConnector * @param id * communication id */ - public void addColumn(Column column, String id) { + public void addColumn(CustomColumn column, String id) { assert !columnToIdMap.containsKey(column) && !columnToIdMap .containsValue(id) : "Column with given id already exists."; getWidget().addColumn(column); @@ -299,7 +300,7 @@ public class GridConnector * @param column * column to remove */ - public void removeColumn(Column column) { + public void removeColumn(CustomColumn column) { assert columnToIdMap .containsKey(column) : "Given Column does not exist."; getWidget().removeColumn(column); @@ -407,11 +408,11 @@ public class GridConnector || row.hasKey(GridState.JSONKEY_CELLDESCRIPTION))) { Column column = cell.getColumn(); - if (columnToIdMap.containsKey(column)) { + if (column instanceof CustomColumn) { JsonObject cellDescriptions = row .getObject(GridState.JSONKEY_CELLDESCRIPTION); - String id = columnToIdMap.get(column); + String id = ((CustomColumn) column).getConnectorId(); if (cellDescriptions != null && cellDescriptions.hasKey(id)) { return new TooltipInfo(cellDescriptions.getString(id)); diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index 891c08aa47..a27130903e 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -41,7 +41,6 @@ import com.vaadin.event.ContextClickEvent; import com.vaadin.event.EventListener; import com.vaadin.server.EncodeResult; import com.vaadin.server.JsonCodec; -import com.vaadin.server.KeyMapper; import com.vaadin.server.data.SortOrder; import com.vaadin.shared.MouseEventDetails; import com.vaadin.shared.Registration; @@ -54,6 +53,7 @@ import com.vaadin.shared.ui.grid.GridServerRpc; import com.vaadin.shared.ui.grid.GridState; import com.vaadin.shared.ui.grid.HeightMode; import com.vaadin.shared.ui.grid.SectionState; +import com.vaadin.shared.util.SharedUtil; import com.vaadin.ui.components.grid.Header; import com.vaadin.ui.components.grid.Header.Row; import com.vaadin.ui.renderers.AbstractRenderer; @@ -841,7 +841,7 @@ public class Grid extends AbstractSingleSelect implements HasComponents { public void generateData(T data, JsonObject jsonObject) { ColumnState state = getState(false); - String communicationId = state.id; + String communicationId = getConnectorId(); assert communicationId != null : "No communication ID set for column " + state.caption; @@ -1575,8 +1575,9 @@ public class Grid extends AbstractSingleSelect implements HasComponents { } }; - private KeyMapper> columnKeys = new KeyMapper<>(); private Set> columnSet = new LinkedHashSet<>(); + private Map> columnKeys = new HashMap<>(); + private List>> sortOrder = new ArrayList<>(); private DetailsManager detailsManager; private Set extensionComponents = new HashSet<>(); @@ -1585,6 +1586,8 @@ public class Grid extends AbstractSingleSelect implements HasComponents { private Header header = new HeaderImpl(); + private int counter = 0; + /** * Constructor for the {@link Grid} component. */ @@ -1619,11 +1622,11 @@ public class Grid extends AbstractSingleSelect implements HasComponents { } /** - * Adds a new column to this {@link Grid} with given header caption, typed + * Adds a new column to this {@link Grid} with given identifier, typed * renderer and value provider. * - * @param caption - * the header caption + * @param identifier + * the identifier in camel case for the new column * @param valueProvider * the value provider * @param renderer @@ -1635,48 +1638,86 @@ public class Grid extends AbstractSingleSelect implements HasComponents { * * @see {@link AbstractRenderer} */ - public Column addColumn(String caption, + public Column addColumn(String identifier, Function valueProvider, AbstractRenderer renderer) { - final Column column = new Column<>(caption, valueProvider, + assert !columnKeys.containsKey(identifier) : "Duplicate identifier: " + + identifier; + + final Column column = new Column<>( + SharedUtil.camelCaseToHumanFriendly(identifier), valueProvider, renderer); - addColumn(column); + addColumn(identifier, column); return column; } /** - * Adds a new text column to this {@link Grid} with given header caption + * Adds a new text column to this {@link Grid} with given identifier and * string value provider. The column will use a {@link TextRenderer}. * - * @param caption + * @param identifier * the header caption * @param valueProvider * the value provider * * @return the new column */ - public Column addColumn(String caption, + public Column addColumn(String identifier, Function valueProvider) { - return addColumn(caption, valueProvider, new TextRenderer()); + return addColumn(identifier, valueProvider, new TextRenderer()); + } + + /** + * Adds a new text column to this {@link Grid} with string value provider. + * The column will use a {@link TextRenderer}. Identifier for the column is + * generated automatically. + * + * @param valueProvider + * the value provider + * + * @return the new column + */ + public Column addColumn(Function valueProvider) { + return addColumn(getGeneratedIdentifier(), valueProvider, + new TextRenderer()); + } + + /** + * Adds a new column to this {@link Grid} with typed renderer and value + * provider. Identifier for the column is generated automatically. + * + * @param valueProvider + * the value provider + * @param renderer + * the column value class + * @param + * the column value type + * + * @return the new column + * + * @see {@link AbstractRenderer} + */ + public Column addColumn(Function valueProvider, + AbstractRenderer renderer) { + return addColumn(getGeneratedIdentifier(), valueProvider, renderer); } - private void addColumn(Column column) { + private void addColumn(String identifier, Column column) { if (getColumns().contains(column)) { return; } - final String columnId = columnKeys.key(column); - column.extend(this); - column.setId(columnId); columnSet.add(column); + columnKeys.put(identifier, column); + column.setId(identifier); addDataGenerator(column); - getState().columnOrder.add(columnId); - getHeader().addColumn(columnId); + getState().columnOrder.add(identifier); + getHeader().addColumn(identifier); if (getDefaultHeaderRow() != null) { - getDefaultHeaderRow().getCell(columnId) + getDefaultHeaderRow().getCell(identifier) .setText(column.getCaption()); } } @@ -1689,7 +1730,7 @@ public class Grid extends AbstractSingleSelect implements HasComponents { */ public void removeColumn(Column column) { if (columnSet.remove(column)) { - columnKeys.remove(column); + columnKeys.remove(column.getId()); removeDataGenerator(column); getHeader().removeColumn(column.getId()); column.remove(); @@ -2205,11 +2246,24 @@ public class Grid extends AbstractSingleSelect implements HasComponents { removeColumns.stream().forEach(this::removeColumn); addColumns.removeAll(currentColumns); - addColumns.stream().forEach(this::addColumn); + addColumns.stream().forEach(c -> addColumn(getIdentifier(c), c)); setColumnOrder(columns); } + private String getIdentifier(Column column) { + return columnKeys.entrySet().stream() + .filter(entry -> entry.getValue().equals(column)) + .map(entry -> entry.getKey()).findFirst() + .orElse(getGeneratedIdentifier()); + } + + private String getGeneratedIdentifier() { + String columnId = "generatedColumn" + counter; + counter = counter + 1; + return columnId; + } + /** * Sets a new column order for the grid. All columns which are not ordered * here will remain in the order they were before as the last columns of 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 3dc6dad770..7c4af0b5bf 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 @@ -9,6 +9,7 @@ import org.junit.Test; import com.vaadin.shared.ui.grid.HeightMode; import com.vaadin.ui.Grid; +import com.vaadin.ui.renderers.NumberRenderer; public class GridTest { @@ -18,6 +19,8 @@ public class GridTest { public void setUp() { grid = new Grid<>(); grid.addColumn("foo", Function.identity()); + grid.addColumn(String::length, new NumberRenderer()); + grid.addColumn("randomColumnId", Function.identity()); } @Test @@ -34,7 +37,7 @@ public class GridTest { @Test(expected = IllegalArgumentException.class) public void testFrozenColumnCountTooBig() { - grid.setFrozenColumnCount(2); + grid.setFrozenColumnCount(5); } @Test(expected = IllegalArgumentException.class) @@ -50,4 +53,25 @@ public class GridTest { grid.getFrozenColumnCount()); } } + + @Test + public void testGridColumnIdentifier() { + grid.getColumn("foo").setCaption("Bar"); + assertEquals("Column header not updated correctly", "Bar", + 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()); + } } diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/basics/GridBasics.java b/uitest/src/main/java/com/vaadin/tests/components/grid/basics/GridBasics.java index 5962e4833d..9fce271c6d 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/basics/GridBasics.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/basics/GridBasics.java @@ -166,23 +166,23 @@ public class GridBasics extends AbstractReindeerTestUIWithLog { grid = new Grid<>(); grid.setItems(data); - grid.addColumn(COLUMN_CAPTIONS[0], - dataObj -> "(" + dataObj.getRowNumber() + ", 0)"); - grid.addColumn(COLUMN_CAPTIONS[1], - dataObj -> "(" + dataObj.getRowNumber() + ", 1)"); - grid.addColumn(COLUMN_CAPTIONS[2], - dataObj -> "(" + dataObj.getRowNumber() + ", 2)"); - - grid.addColumn(COLUMN_CAPTIONS[3], DataObject::getRowNumber, - new NumberRenderer()); - grid.addColumn(COLUMN_CAPTIONS[4], DataObject::getDate, - new DateRenderer()); - grid.addColumn(COLUMN_CAPTIONS[5], DataObject::getHtmlString, - new HtmlRenderer()); - grid.addColumn(COLUMN_CAPTIONS[6], DataObject::getBigRandom, - new NumberRenderer()); - grid.addColumn(COLUMN_CAPTIONS[7], data -> data.getSmallRandom() / 5d, - new ProgressBarRenderer()); + grid.addColumn(dataObj -> "(" + dataObj.getRowNumber() + ", 0)") + .setCaption(COLUMN_CAPTIONS[0]); + grid.addColumn(dataObj -> "(" + dataObj.getRowNumber() + ", 1)") + .setCaption(COLUMN_CAPTIONS[1]); + grid.addColumn(dataObj -> "(" + dataObj.getRowNumber() + ", 2)") + .setCaption(COLUMN_CAPTIONS[2]); + + grid.addColumn(DataObject::getRowNumber, new NumberRenderer()) + .setCaption(COLUMN_CAPTIONS[3]); + grid.addColumn(DataObject::getDate, new DateRenderer()) + .setCaption(COLUMN_CAPTIONS[4]); + grid.addColumn(DataObject::getHtmlString, new HtmlRenderer()) + .setCaption(COLUMN_CAPTIONS[5]); + grid.addColumn(DataObject::getBigRandom, new NumberRenderer()) + .setCaption(COLUMN_CAPTIONS[6]); + grid.addColumn(data -> data.getSmallRandom() / 5d, + new ProgressBarRenderer()).setCaption(COLUMN_CAPTIONS[7]); grid.addSelectionListener(e -> log("Selected: " + e.getValue())); @@ -205,10 +205,10 @@ public class GridBasics extends AbstractReindeerTestUIWithLog { @SuppressWarnings("unchecked") private void createColumnsMenu(MenuItem columnsMenu) { - for (int i = 0; i < grid.getColumns().size(); i++) { - final int index = i; - MenuItem columnMenu = columnsMenu.addItem("Column " + i, null); + for (Column col : grid.getColumns()) { + MenuItem columnMenu = columnsMenu.addItem(col.getCaption(), null); columnMenu.addItem("Move left", selectedItem -> { + int index = grid.getColumns().indexOf(col); if (index > 0) { List> columnOrder = new ArrayList<>( grid.getColumns()); @@ -218,6 +218,7 @@ public class GridBasics extends AbstractReindeerTestUIWithLog { } }); columnMenu.addItem("Move right", selectedItem -> { + int index = grid.getColumns().indexOf(col); if (index < grid.getColumns().size() - 1) { List> columnOrder = new ArrayList<>( grid.getColumns()); @@ -228,17 +229,17 @@ public class GridBasics extends AbstractReindeerTestUIWithLog { }); columnMenu .addItem("Sortable", - selectedItem -> grid.getColumns().get(index) + selectedItem -> col .setSortable(selectedItem.isChecked())) .setCheckable(true); columnMenu .addItem("Hidable", - selectedItem -> grid.getColumns().get(index) + selectedItem -> col .setHidable(selectedItem.isChecked())) .setCheckable(true); columnMenu .addItem("Hidden", - selectedItem -> grid.getColumns().get(index) + selectedItem -> col .setHidden(selectedItem.isChecked())) .setCheckable(true); } diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridColumnReorderTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridColumnReorderTest.java index 62156acec7..e77389d86e 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridColumnReorderTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridColumnReorderTest.java @@ -43,7 +43,8 @@ public class GridColumnReorderTest extends GridBasicsTest { public void testColumnReorder_onReorder_columnReorderEventTriggered() { selectMenuPath("Component", "Header", "Prepend header row"); selectMenuPath("Component", "State", "Column reorder listener"); - selectMenuPath("Component", "Columns", "Column " + 3, "Move left"); + selectMenuPath("Component", "Columns", GridBasics.COLUMN_CAPTIONS[3], + "Move left"); assertEquals("1. Registered a column reorder listener.", getLogRow(2)); assertEquals("2. Columns reordered, userOriginated: false", @@ -51,7 +52,8 @@ public class GridColumnReorderTest extends GridBasicsTest { assertColumnHeaderOrder(0, 1, 3, 2); // trigger another event - selectMenuPath("Component", "Columns", "Column " + 3, "Move left"); + selectMenuPath("Component", "Columns", GridBasics.COLUMN_CAPTIONS[3], + "Move right"); assertColumnHeaderOrder(0, 1, 2, 3); // test drag and drop is user originated -- 2.39.5