From f537b9fdb2ba3993090d22011adfe247e4e2b6bd Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Tue, 17 Feb 2015 10:54:40 +0200 Subject: Toggle column reordering from server side. Tests for reordering. #16643 Change-Id: Ib52143ce387f6376878bf3d1c401615a15f1a3cc --- server/src/com/vaadin/ui/Grid.java | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'server/src') diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 2bc42676c3..764960606a 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -3469,6 +3469,31 @@ public class Grid extends AbstractComponent implements SelectionNotifier, return columnKeys.get(columnId); } + /** + * Returns whether column reordering is allowed. Default value is + * false. + * + * @since + * @return true if reordering is allowed + */ + public boolean isColumnReorderingAllowed() { + return getState(false).columnReorderingAllowed; + } + + /** + * Sets whether or not column reordering is allowed. Default value is + * false. + * + * @since + * @param columnReorderingAllowed + * specifies whether column reordering is allowed + */ + public void setColumnReorderingAllowed(boolean columnReorderingAllowed) { + if (isColumnReorderingAllowed() != columnReorderingAllowed) { + getState().columnReorderingAllowed = columnReorderingAllowed; + } + } + @Override protected GridState getState() { return (GridState) super.getState(); -- cgit v1.2.3 From 9b2f51ca9ff48ad9484b8ee67770ab1a58352b35 Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Wed, 18 Feb 2015 10:05:25 +0200 Subject: Update server side state when columns are reordered. (#16643) Change-Id: I96c65dbb96614a5f5782b747fb8588647211cf4b --- .../vaadin/client/connectors/GridConnector.java | 52 ++++++++++++++++---- server/src/com/vaadin/ui/Grid.java | 40 +++++++++++++++ .../com/vaadin/shared/ui/grid/GridServerRpc.java | 12 +++++ .../grid/basicfeatures/GridBasicFeatures.java | 42 ++++++++++++++++ .../server/GridColumnReorderTest.java | 57 ++++++++++++++++++++++ 5 files changed, 194 insertions(+), 9 deletions(-) (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index f263b47642..2ae04b2be2 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -50,6 +50,8 @@ import com.vaadin.client.widget.grid.RowReference; import com.vaadin.client.widget.grid.RowStyleGenerator; import com.vaadin.client.widget.grid.events.BodyClickHandler; import com.vaadin.client.widget.grid.events.BodyDoubleClickHandler; +import com.vaadin.client.widget.grid.events.ColumnReorderEvent; +import com.vaadin.client.widget.grid.events.ColumnReorderHandler; import com.vaadin.client.widget.grid.events.GridClickEvent; import com.vaadin.client.widget.grid.events.GridDoubleClickEvent; import com.vaadin.client.widget.grid.events.SelectAllEvent; @@ -360,6 +362,24 @@ public class GridConnector extends AbstractHasComponentsConnector implements } } + private ColumnReorderHandler columnReorderHandler = new ColumnReorderHandler() { + + @Override + public void onColumnReorder(ColumnReorderEvent event) { + if (!columnsUpdatedFromState) { + List> columns = getWidget().getColumns(); + final List newColumnOrder = new ArrayList(); + for (Column column : columns) { + newColumnOrder.add(((CustomGridColumn) column).id); + } + getRpcProxy(GridServerRpc.class).columnsReordered( + newColumnOrder, columnOrder); + columnOrder = newColumnOrder; + getState().columnOrder = newColumnOrder; + } + } + }; + /** * Maps a generated column id to a grid column instance */ @@ -370,13 +390,22 @@ public class GridConnector extends AbstractHasComponentsConnector implements private List columnOrder = new ArrayList(); /** - * updateFromState is set to true when {@link #updateSelectionFromState()} - * makes changes to selection. This flag tells the - * {@code internalSelectionChangeHandler} to not send same data straight - * back to server. Said listener sets it back to false when handling that - * event. + * {@link #selectionUpdatedFromState} is set to true when + * {@link #updateSelectionFromState()} makes changes to selection. This flag + * tells the {@code internalSelectionChangeHandler} to not send same data + * straight back to server. Said listener sets it back to false when + * handling that event. + */ + private boolean selectionUpdatedFromState; + + /** + * {@link #columnsUpdatedFromState} is set to true when + * {@link #updateColumnOrderFromState(List)} is updating the column order + * for the widget. This flag tells the {@link #columnReorderHandler} to not + * send same data straight back to server. After updates, listener sets the + * value back to false. */ - private boolean updatedFromState = false; + private boolean columnsUpdatedFromState; private RpcDataSource dataSource; @@ -386,7 +415,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements if (event.isBatchedSelection()) { return; } - if (!updatedFromState) { + if (!selectionUpdatedFromState) { for (JsonObject row : event.getRemoved()) { selectedKeys.remove(dataSource.getRowKey(row)); } @@ -398,7 +427,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements getRpcProxy(GridServerRpc.class).select( new ArrayList(selectedKeys)); } else { - updatedFromState = false; + selectionUpdatedFromState = false; } } }; @@ -496,6 +525,9 @@ public class GridConnector extends AbstractHasComponentsConnector implements }); getWidget().setEditorHandler(new CustomEditorHandler()); + + getWidget().addColumnReorderHandler(columnReorderHandler); + getLayoutManager().registerDependency(this, getWidget().getElement()); layout(); } @@ -589,7 +621,9 @@ public class GridConnector extends AbstractHasComponentsConnector implements columns[i] = columnIdToColumn.get(id); i++; } + columnsUpdatedFromState = true; getWidget().setColumnOrder(columns); + columnsUpdatedFromState = false; columnOrder = stateColumnOrder; } @@ -886,7 +920,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements if (changed) { // At least for now there's no way to send the selected and/or // deselected row data. Some data is only stored as keys - updatedFromState = true; + selectionUpdatedFromState = true; getWidget().fireEvent( new SelectionEvent(getWidget(), (List) null, null, false)); diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 764960606a..72a772713f 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -18,6 +18,7 @@ package com.vaadin.ui; import java.io.Serializable; import java.lang.reflect.Method; +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -70,6 +71,7 @@ import com.vaadin.event.SortEvent.SortListener; import com.vaadin.event.SortEvent.SortNotifier; import com.vaadin.server.AbstractClientConnector; import com.vaadin.server.AbstractExtension; +import com.vaadin.server.EncodeResult; import com.vaadin.server.ErrorMessage; import com.vaadin.server.JsonCodec; import com.vaadin.server.KeyMapper; @@ -3088,6 +3090,44 @@ public class Grid extends AbstractComponent implements SelectionNotifier, fireEvent(new ItemClickEvent(Grid.this, item, itemId, propertyId, details)); } + + @Override + public void columnsReordered(List newColumnOrder, + List oldColumnOrder) { + final String diffStateKey = "columnOrder"; + ConnectorTracker connectorTracker = getUI() + .getConnectorTracker(); + JsonObject diffState = connectorTracker.getDiffState(Grid.this); + // discard the change if the columns have been reordered from + // the server side, as the server side is always right + if (getState(false).columnOrder.equals(oldColumnOrder)) { + // Don't mark as dirty since client has the state already + getState(false).columnOrder = newColumnOrder; + // write changes to diffState so that possible reverting the + // column order is sent to client + assert diffState.hasKey(diffStateKey) : "Field name has changed"; + Type type = null; + try { + type = (getState(false).getClass().getDeclaredField( + diffStateKey).getGenericType()); + } catch (NoSuchFieldException e) { + e.printStackTrace(); + } catch (SecurityException e) { + e.printStackTrace(); + } + EncodeResult encodeResult = JsonCodec.encode( + getState(false).columnOrder, diffState, type, + connectorTracker); + + diffState.put(diffStateKey, encodeResult.getEncodedValue()); + // TODO fire column reorder event + } else { + // make sure the client is reverted to the order that the + // server thinks it is + diffState.remove(diffStateKey); + markAsDirty(); + } + } }); registerRpc(new EditorServerRpc() { diff --git a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java index c90a016383..4dec5530aa 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java @@ -47,4 +47,16 @@ public interface GridServerRpc extends ServerRpc { * mouse event details */ void itemClick(String rowKey, String columnId, MouseEventDetails details); + + /** + * Informs the server that the columns of the Grid have been reordered. + * + * @since + * @param newColumnOrder + * a list of column ids in the new order + * @param oldColumnOrder + * a list of column ids in order before the change + */ + void columnsReordered(List newColumnOrder, + List oldColumnOrder); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index 3a6aca11f2..792ef89d42 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -248,10 +248,25 @@ public class GridBasicFeatures extends AbstractComponentTest { addFilterActions(); + addInternalActions(); + this.grid = grid; return grid; } + private void addInternalActions() { + createClickAction("Update column order without updating client", + "Internals", new Command() { + @Override + public void execute(Grid grid, Void value, Object data) { + List columns = grid.getColumns(); + grid.setColumnOrder(columns.get(1).getPropertyId(), + columns.get(0).getPropertyId()); + grid.getUI().getConnectorTracker().markClean(grid); + } + }, null); + } + private void addFilterActions() { createClickAction("Column 1 starts with \"(23\"", "Filter", new Command() { @@ -658,6 +673,33 @@ public class GridBasicFeatures extends AbstractComponentTest { } } }, null, c); + createClickAction("Move left", getColumnProperty(c), + new Command() { + + @Override + public void execute(Grid grid, String value, Object data) { + final String columnProperty = getColumnProperty((Integer) data); + List cols = grid.getColumns(); + List reordered = new ArrayList(); + boolean addAsLast = false; + for (int i = 0; i < cols.size(); i++) { + Column col = cols.get(i); + if (col.getPropertyId().equals(columnProperty)) { + if (i == 0) { + addAsLast = true; + } else { + reordered.add(i - 1, columnProperty); + } + } else { + reordered.add(col.getPropertyId()); + } + } + if (addAsLast) { + reordered.add(columnProperty); + } + grid.setColumnOrder(reordered.toArray()); + } + }, null, c); createBooleanAction("Sortable", getColumnProperty(c), true, new Command() { diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java index 7b62ff85f9..7f4e9bb7e9 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java @@ -70,6 +70,26 @@ public class GridColumnReorderTest extends GridBasicFeaturesTest { assertColumnHeaderOrder(1, 2, 0); } + @Test + public void testColumnReordering_reorderingTwiceBackForth_reordered() { + // given + openTestURL(); + assertColumnHeaderOrder(0, 1, 2); + toggleColumnReordering(); + + // when + dragDefaultColumnHeader(2, 0, 10); + + // then + assertColumnHeaderOrder(2, 0, 1, 3); + + // when + dragDefaultColumnHeader(1, 3, 110); + + // then + assertColumnHeaderOrder(2, 1, 3, 0); + } + @Test public void testColumnReordering_notEnabled_noReordering() { // given @@ -83,10 +103,47 @@ public class GridColumnReorderTest extends GridBasicFeaturesTest { assertColumnHeaderOrder(0, 1, 2); } + @Test + public void testColumnReordering_userChangesRevertedByServer_columnsAreUpdated() { + // given + openTestURL(); + assertColumnHeaderOrder(0, 1, 2); + toggleColumnReordering(); + + // when + dragDefaultColumnHeader(0, 2, 10); + assertColumnHeaderOrder(1, 0, 2); + moveColumnManuallyLeftByOne(0); + + // then + assertColumnHeaderOrder(0, 1, 2); + } + + @Test + public void testColumnReordering_concurrentUpdatesFromServer_columnOrderFromServerUsed() { + // given + openTestURL(); + assertColumnHeaderOrder(0, 1, 2); + toggleColumnReordering(); + + // when + selectMenuPath(new String[] { "Component", "Internals", + "Update column order without updating client" }); + dragDefaultColumnHeader(2, 0, 10); + + // then + assertColumnHeaderOrder(1, 0, 2); + } + private void toggleColumnReordering() { selectMenuPath(COLUMN_REORDERING_PATH); } + private void moveColumnManuallyLeftByOne(int index) { + selectMenuPath(new String[] { "Component", "Columns", + "Column " + index, "Move left" }); + } + private void assertColumnHeaderOrder(int... indices) { List headers = getGridHeaderRowCells(); for (int i = 0; i < indices.length; i++) { -- cgit v1.2.3 From ad7bcdc7d22cedf30d76dd6d1ba7a1c9bcabdd53 Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Thu, 19 Feb 2015 17:03:00 +0200 Subject: Fire server-side event when column order changes. (#16643) Event fired for both UI and programmatic changes. Change-Id: I043e7407ee11547cb3b6639dc14c26ff036a9d82 --- server/src/com/vaadin/ui/Grid.java | 86 +++++++++++++++++++++- .../grid/basicfeatures/GridBasicFeatures.java | 23 ++++++ .../server/GridColumnReorderTest.java | 68 +++++++++++++++++ 3 files changed, 176 insertions(+), 1 deletion(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 72a772713f..df64ee85ed 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -336,6 +336,58 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } + /** + * An event listener for column reorder events in the Grid. + * + * @since + */ + public interface ColumnReorderListener extends Serializable { + /** + * Called when the columns of the grid have been reordered. + * + * @param event + * An event providing more information + */ + void columnReorder(ColumnReorderEvent event); + } + + /** + * An event that is fired when the columns are reordered. + * + * @since + */ + public static class ColumnReorderEvent extends Component.Event { + + /** + * Is the column reorder related to this event initiated by the user + */ + private final boolean userOriginated; + + /** + * + * @param source + * the grid where the event originated from + * @param userOriginated + * true if event is a result of user + * interaction, false if from API call + */ + public ColumnReorderEvent(Grid source, boolean userOriginated) { + super(source); + this.userOriginated = userOriginated; + } + + /** + * Returns true if the column reorder was done by the user, + * false if not and it was triggered by server side code. + * + * @return true if event is a result of user interaction + */ + public boolean isUserOriginated() { + return userOriginated; + } + + } + /** * Default error handler for the editor * @@ -2896,6 +2948,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier, private static final Method SORT_ORDER_CHANGE_METHOD = ReflectTools .findMethod(SortListener.class, "sort", SortEvent.class); + private static final Method COLUMN_REORDER_METHOD = ReflectTools + .findMethod(ColumnReorderListener.class, "columnReorder", + ColumnReorderEvent.class); + /** * Creates a new Grid with a new {@link IndexedContainer} as the data * source. @@ -3120,7 +3176,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, connectorTracker); diffState.put(diffStateKey, encodeResult.getEncodedValue()); - // TODO fire column reorder event + fireColumnReorderEvent(true); } else { // make sure the client is reverted to the order that the // server thinks it is @@ -3632,6 +3688,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, columnOrder.addAll(stateColumnOrder); } getState().columnOrder = columnOrder; + fireColumnReorderEvent(false); } /** @@ -4098,6 +4155,33 @@ public class Grid extends AbstractComponent implements SelectionNotifier, removeListener(SelectionEvent.class, listener, SELECTION_CHANGE_METHOD); } + private void fireColumnReorderEvent(boolean userOriginated) { + fireEvent(new ColumnReorderEvent(this, userOriginated)); + } + + /** + * Registers a new column reorder listener. + * + * @since + * @param listener + * the listener to register + */ + public void addColumnReorderListener(ColumnReorderListener listener) { + addListener(ColumnReorderEvent.class, listener, COLUMN_REORDER_METHOD); + } + + /** + * Removes a previously registered column reorder listener. + * + * @since + * @param listener + * the listener to remove + */ + public void removeColumnReorderListener(ColumnReorderListener listener) { + removeListener(ColumnReorderEvent.class, listener, + COLUMN_REORDER_METHOD); + } + /** * Gets the * {@link com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index 792ef89d42..e9f6bfdbb9 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -50,6 +50,8 @@ import com.vaadin.ui.Grid; import com.vaadin.ui.Grid.CellReference; import com.vaadin.ui.Grid.CellStyleGenerator; import com.vaadin.ui.Grid.Column; +import com.vaadin.ui.Grid.ColumnReorderEvent; +import com.vaadin.ui.Grid.ColumnReorderListener; import com.vaadin.ui.Grid.FooterCell; import com.vaadin.ui.Grid.HeaderCell; import com.vaadin.ui.Grid.HeaderRow; @@ -109,6 +111,15 @@ public class GridBasicFeatures extends AbstractComponentTest { } }; + private ColumnReorderListener columnReorderListener = new ColumnReorderListener() { + + @Override + public void columnReorder(ColumnReorderEvent event) { + log("Columns reordered, userOriginated: " + + event.isUserOriginated()); + } + }; + @Override @SuppressWarnings("unchecked") protected Grid constructComponent() { @@ -509,6 +520,18 @@ public class GridBasicFeatures extends AbstractComponentTest { } }); + createBooleanAction("ColumnReorderListener", "State", false, + new Command() { + + @Override + public void execute(Grid grid, Boolean value, Object data) { + if (value) { + grid.addColumnReorderListener(columnReorderListener); + } else { + grid.removeColumnReorderListener(columnReorderListener); + } + } + }); createBooleanAction("Single select allow deselect", "State", singleSelectAllowDeselect, new Command() { diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java index 7f4e9bb7e9..2f00071351 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnReorderTest.java @@ -16,6 +16,8 @@ package com.vaadin.tests.components.grid.basicfeatures.server; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.util.List; @@ -36,6 +38,8 @@ public class GridColumnReorderTest extends GridBasicFeaturesTest { private static final String[] COLUMN_REORDERING_PATH = { "Component", "State", "Column Reordering Allowed" }; + private static final String[] COLUMN_REORDER_LISTENER_PATH = { "Component", + "State", "ColumnReorderListener" }; @Before public void setUp() { @@ -135,15 +139,79 @@ public class GridColumnReorderTest extends GridBasicFeaturesTest { assertColumnHeaderOrder(1, 0, 2); } + @Test + public void testColumnReordering_triggersReorderEvent_isUserInitiated() { + // given + openTestURL(); + toggleColumnReordering(); + + // when + toggleColumnReorderListener(); + dragDefaultColumnHeader(0, 2, 10); + + // then + assertColumnReorderEvent(true); + } + + @Test + public void testColumnReordering_addAndRemoveListener_registerUnRegisterWorks() { + // given + openTestURL(); + toggleColumnReordering(); + dragDefaultColumnHeader(0, 2, 10); + assertNoColumnReorderEvent(); + + // when + toggleColumnReorderListener(); + dragDefaultColumnHeader(0, 2, 110); + + // then + assertColumnReorderEvent(true); + + // when + toggleColumnReorderListener(); + dragDefaultColumnHeader(0, 3, 10); + + // then + assertNoColumnReorderEvent(); + } + + @Test + public void testColumnReorderingEvent_serverSideReorder_triggersReorderEvent() { + openTestURL(); + + // when + toggleColumnReorderListener(); + moveColumnManuallyLeftByOne(3); + + // then + assertColumnReorderEvent(false); + } + private void toggleColumnReordering() { selectMenuPath(COLUMN_REORDERING_PATH); } + private void toggleColumnReorderListener() { + selectMenuPath(COLUMN_REORDER_LISTENER_PATH); + } + private void moveColumnManuallyLeftByOne(int index) { selectMenuPath(new String[] { "Component", "Columns", "Column " + index, "Move left" }); } + private void assertColumnReorderEvent(boolean userOriginated) { + final String logRow = getLogRow(0); + assertTrue(logRow.contains("Columns reordered, userOriginated: " + + userOriginated)); + } + + private void assertNoColumnReorderEvent() { + final String logRow = getLogRow(0); + assertFalse(logRow.contains("Columns reordered")); + } + private void assertColumnHeaderOrder(int... indices) { List headers = getGridHeaderRowCells(); for (int i = 0; i < indices.length; i++) { -- cgit v1.2.3 From d6dbed15d626d0ae22cc711cfb08514be1c9a0db Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Wed, 4 Mar 2015 12:42:38 +0200 Subject: Fixes a bug with escalator.scrollToRow and spacers (#16644) Change-Id: I9e148cf81d393cc489c20bf0265f4a3bc6fed069 --- .../src/com/vaadin/client/widgets/Escalator.java | 6 ++--- server/src/com/vaadin/ui/Grid.java | 4 ++-- .../EscalatorBasicClientFeaturesTest.java | 26 +++++++++++++++------- .../escalator/EscalatorSpacerTest.java | 18 +++++++++++++++ .../grid/EscalatorBasicClientFeaturesWidget.java | 14 ++++++++++++ 5 files changed, 55 insertions(+), 13 deletions(-) (limited to 'server/src') diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index efa777111a..679b452fde 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -1150,9 +1150,9 @@ public class Escalator extends Widget implements RequiresResize, public void scrollToRow(final int rowIndex, final ScrollDestination destination, final double padding) { - getLogger().warning("[[spacers]] scrollToRow"); - - final double targetStartPx = body.getDefaultRowHeight() * rowIndex; + final double targetStartPx = (body.getDefaultRowHeight() * rowIndex) + + body.spacerContainer + .getSpacerHeightsSumUntilIndex(rowIndex); final double targetEndPx = targetStartPx + body.getDefaultRowHeight(); diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 6ab6c6b1a4..cf0e54156a 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -51,7 +51,6 @@ import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper; import com.vaadin.data.Validator.InvalidValueException; import com.vaadin.data.fieldgroup.DefaultFieldGroupFieldFactory; import com.vaadin.data.fieldgroup.FieldGroup; -import com.vaadin.data.fieldgroup.FieldGroup.BindException; import com.vaadin.data.fieldgroup.FieldGroup.CommitException; import com.vaadin.data.fieldgroup.FieldGroupFieldFactory; import com.vaadin.data.sort.Sort; @@ -2647,7 +2646,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * Getting a field before the editor has been opened depends on special * support from the {@link FieldGroup} in use. Using this method with a * user-provided FieldGroup might cause - * {@link BindException} to be thrown. + * {@link com.vaadin.data.fieldgroup.FieldGroup.BindException + * BindException} to be thrown. * * @return the bound field; or null if the respective * column is not editable diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java index 61c75b6162..04c0933866 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java @@ -33,6 +33,10 @@ import com.vaadin.tests.tb3.MultiBrowserTest; @TestCategory("escalator") public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest { + + private static final String LOGICAL_ROW_ATTRIBUTE_NAME = "vLogicalRow"; + private static final String SPACER_CSS_CLASS = "v-escalator-spacer"; + protected static final String COLUMNS_AND_ROWS = "Columns and Rows"; protected static final String COLUMNS = "Columns"; @@ -52,6 +56,8 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest protected static final String BODY_ROWS = "Body Rows"; protected static final String FOOTER_ROWS = "Footer Rows"; + protected static final String SCROLL_TO = "Scroll to..."; + protected static final String REMOVE_ALL_INSERT_SCROLL = "Remove all, insert 30 and scroll 40px"; protected static final String GENERAL = "General"; @@ -69,12 +75,15 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest protected static final String COLUMN_SPANNING = "Column spanning"; protected static final String COLSPAN_NORMAL = "Apply normal colspan"; protected static final String COLSPAN_NONE = "Apply no colspan"; + protected static final String SET_100PX = "Set 100px"; protected static final String SPACERS = "Spacers"; + protected static final String REMOVE = "Remove"; + protected static final String ROW_MINUS1 = "Row -1"; protected static final String ROW_1 = "Row 1"; + protected static final String ROW_25 = "Row 25"; + protected static final String ROW_75 = "Row 75"; protected static final String ROW_99 = "Row 99"; - protected static final String SET_100PX = "Set 100px"; - protected static final String REMOVE = "Remove"; @Override protected Class getUIClass() { @@ -173,15 +182,16 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest private TestBenchElement getRow(String sectionTag, int row) { TestBenchElement escalator = getEscalator(); WebElement tableSection = escalator.findElement(By.tagName(sectionTag)); - By xpath; + String xpathExpression = "tr[not(@class='" + SPACER_CSS_CLASS + "')]"; if (row >= 0) { int fromFirst = row + 1; - xpath = By.xpath("tr[" + fromFirst + "]"); + xpathExpression += "[" + fromFirst + "]"; } else { int fromLast = Math.abs(row + 1); - xpath = By.xpath("tr[last() - " + fromLast + "]"); + xpathExpression += "[last() - " + fromLast + "]"; } + By xpath = By.xpath(xpathExpression); if (tableSection != null && ((TestBenchElement) tableSection).isElementPresent(xpath)) { return (TestBenchElement) tableSection.findElement(xpath); @@ -281,7 +291,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest } private List getSpacers() { - return getEscalator().findElements(By.className("v-escalator-spacer")); + return getEscalator().findElements(By.className(SPACER_CSS_CLASS)); } protected boolean spacersAreFoundInDom() { @@ -295,8 +305,8 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest System.out.println("size: " + spacers.size()); for (WebElement spacer : spacers) { System.out.println(spacer + ", " + logicalRowIndex); - Boolean isInDom = (Boolean) executeScript( - "return arguments[0]['vLogicalRow'] === arguments[1]", + Boolean isInDom = (Boolean) executeScript("return arguments[0]['" + + LOGICAL_ROW_ATTRIBUTE_NAME + "'] === arguments[1]", spacer, logicalRowIndex); if (isInDom) { return spacer; diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java index da3472aebf..c3468b373e 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java @@ -267,6 +267,24 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest { + "-1 spacer", 0, getScrollTop()); } + @Test + public void scrollToRowWorksProperlyWithSpacers() throws Exception { + selectMenuPath(FEATURES, SPACERS, ROW_MINUS1, SET_100PX); + selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX); + + /* + * we check for row -2 instead of -1, because escalator has the one row + * buffered underneath the footer + */ + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_75); + Thread.sleep(500); + assertEquals("Row 75: 0,75", getBodyCell(-2, 0).getText()); + + selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_25); + Thread.sleep(500); + assertEquals("Row 25: 0,25", getBodyCell(0, 0).getText()); + } + private static double[] getElementDimensions(WebElement element) { /* * we need to parse the style attribute, since using getCssValue gets a diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java index dc86b89167..0d4aa305d9 100644 --- a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java +++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java @@ -16,6 +16,7 @@ import com.vaadin.client.widget.escalator.RowContainer.BodyRowContainer; import com.vaadin.client.widget.escalator.Spacer; import com.vaadin.client.widget.escalator.SpacerUpdater; import com.vaadin.client.widgets.Escalator; +import com.vaadin.shared.ui.grid.ScrollDestination; public class EscalatorBasicClientFeaturesWidget extends PureGWTTestApplication { @@ -571,6 +572,19 @@ public class EscalatorBasicClientFeaturesWidget extends escalator.setScrollTop(40); } }, menupath); + + String[] scrollToRowMenuPath = new String[menupath.length + 1]; + System.arraycopy(menupath, 0, scrollToRowMenuPath, 0, menupath.length); + scrollToRowMenuPath[scrollToRowMenuPath.length - 1] = "Scroll to..."; + for (int i = 0; i < 100; i += 25) { + final int rowIndex = i; + addMenuCommand("Row " + i, new ScheduledCommand() { + @Override + public void execute() { + escalator.scrollToRow(rowIndex, ScrollDestination.ANY, 0); + } + }, scrollToRowMenuPath); + } } private void createRowsMenu(final RowContainer container, String[] menupath) { -- cgit v1.2.3 From 84c143dd76ed1d27d03c0d695e7218b477d008fe Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Mon, 9 Mar 2015 14:31:37 +0200 Subject: Server side Grid can open details on the client side (#16644) Change-Id: Ibff5a83b3a09c7c530926dadae9138ba3823f27a --- .../vaadin/client/connectors/GridConnector.java | 26 ++++++- .../client/connectors/RpcDataSourceConnector.java | 53 +++++++++++-- .../client/widget/grid/DetailsGenerator.java | 1 + client/src/com/vaadin/client/widgets/Grid.java | 7 ++ .../com/vaadin/data/RpcDataProviderExtension.java | 72 ++++++++++++++++- server/src/com/vaadin/ui/Grid.java | 91 ++++++++++++++++++++++ .../src/com/vaadin/shared/ui/grid/GridState.java | 10 +++ .../grid/basicfeatures/GridBasicFeatures.java | 14 ++++ .../server/GridDetailsServerTest.java | 76 ++++++++++++++++++ 9 files changed, 341 insertions(+), 9 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 55f07ecf85..f476982c15 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -31,6 +31,7 @@ import java.util.logging.Logger; import com.google.gwt.core.client.Scheduler; import com.google.gwt.core.client.Scheduler.ScheduledCommand; import com.google.gwt.dom.client.NativeEvent; +import com.google.gwt.user.client.ui.Label; import com.google.gwt.user.client.ui.Widget; import com.vaadin.client.ComponentConnector; import com.vaadin.client.ConnectorHierarchyChangeEvent; @@ -45,6 +46,7 @@ import com.vaadin.client.ui.AbstractHasComponentsConnector; import com.vaadin.client.ui.SimpleManagedLayout; import com.vaadin.client.widget.grid.CellReference; import com.vaadin.client.widget.grid.CellStyleGenerator; +import com.vaadin.client.widget.grid.DetailsGenerator; import com.vaadin.client.widget.grid.EditorHandler; import com.vaadin.client.widget.grid.RowReference; import com.vaadin.client.widget.grid.RowStyleGenerator; @@ -101,7 +103,7 @@ import elemental.json.JsonValue; */ @Connect(com.vaadin.ui.Grid.class) public class GridConnector extends AbstractHasComponentsConnector implements - SimpleManagedLayout { + SimpleManagedLayout, RpcDataSourceConnector.DetailsListener { private static final class CustomCellStyleGenerator implements CellStyleGenerator { @@ -360,6 +362,14 @@ public class GridConnector extends AbstractHasComponentsConnector implements } } + private class CustomDetailsGenerator implements DetailsGenerator { + @Override + public Widget getDetails(int rowIndex) { + // TODO + return new Label("[todo]"); + } + } + /** * Maps a generated column id to a grid column instance */ @@ -501,7 +511,11 @@ public class GridConnector extends AbstractHasComponentsConnector implements }); getWidget().setEditorHandler(new CustomEditorHandler()); + + getWidget().setDetailsGenerator(new CustomDetailsGenerator()); + getLayoutManager().registerDependency(this, getWidget().getElement()); + layout(); } @@ -994,4 +1008,14 @@ public class GridConnector extends AbstractHasComponentsConnector implements public void layout() { getWidget().onResize(); } + + @Override + public void reapplyDetailsVisibility(int rowIndex, JsonObject row) { + if (row.hasKey(GridState.JSONKEY_DETAILS_VISIBLE) + && row.getBoolean(GridState.JSONKEY_DETAILS_VISIBLE)) { + getWidget().setDetailsVisible(rowIndex, true); + } else { + getWidget().setDetailsVisible(rowIndex, false); + } + } } diff --git a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java index f8d6ebcb62..ae4249de78 100644 --- a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java +++ b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java @@ -17,6 +17,7 @@ package com.vaadin.client.connectors; import java.util.ArrayList; +import java.util.List; import com.vaadin.client.ServerConnector; import com.vaadin.client.data.AbstractRemoteDataSource; @@ -43,6 +44,28 @@ import elemental.json.JsonObject; @Connect(com.vaadin.data.RpcDataProviderExtension.class) public class RpcDataSourceConnector extends AbstractExtensionConnector { + /** + * A callback interface to let {@link GridConnector} know that detail + * visibilities might have changed. + * + * @since + * @author Vaadin Ltd + */ + interface DetailsListener { + + /** + * A request to verify (and correct) the visibility for a row, given + * updated metadata. + * + * @param rowIndex + * the index of the row that should be checked + * @param row + * the row object to check visibility for + * @see GridState#JSONKEY_DETAILS_VISIBLE + */ + void reapplyDetailsVisibility(int rowIndex, JsonObject row); + } + public class RpcDataSource extends AbstractRemoteDataSource { protected RpcDataSource() { @@ -56,27 +79,28 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { rows.add(rowObject); } - dataSource.setRowData(firstRow, rows); + RpcDataSource.this.setRowData(firstRow, rows); } @Override public void removeRowData(int firstRow, int count) { - dataSource.removeRowData(firstRow, count); + RpcDataSource.this.removeRowData(firstRow, count); } @Override public void insertRowData(int firstRow, int count) { - dataSource.insertRowData(firstRow, count); + RpcDataSource.this.insertRowData(firstRow, count); } @Override public void resetDataAndSize(int size) { - dataSource.resetDataAndSize(size); + RpcDataSource.this.resetDataAndSize(size); } }); } private DataRequestRpc rpcProxy = getRpcProxy(DataRequestRpc.class); + private DetailsListener detailsListener; @Override protected void requestRows(int firstRowIndex, int numberOfRows, @@ -170,7 +194,24 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { if (!handle.isPinned()) { rpcProxy.setPinned(key, false); } + } + + void setDetailsListener(DetailsListener detailsListener) { + this.detailsListener = detailsListener; + } + @Override + protected void setRowData(int firstRowIndex, List rowData) { + super.setRowData(firstRowIndex, rowData); + + /* + * Intercepting details information from the data source, rerouting + * them back to the GridConnector (as a details listener) + */ + for (int i = 0; i < rowData.size(); i++) { + detailsListener.reapplyDetailsVisibility(firstRowIndex + i, + rowData.get(i)); + } } } @@ -178,6 +219,8 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { @Override protected void extend(ServerConnector target) { - ((GridConnector) target).setDataSource(dataSource); + GridConnector gridConnector = (GridConnector) target; + dataSource.setDetailsListener(gridConnector); + gridConnector.setDataSource(dataSource); } } diff --git a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java index 264aa4e614..309e3f1ea3 100644 --- a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java +++ b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java @@ -25,6 +25,7 @@ import com.google.gwt.user.client.ui.Widget; */ public interface DetailsGenerator { + /** A details generator that provides no details */ public static final DetailsGenerator NULL = new DetailsGenerator() { @Override public Widget getDetails(int rowIndex) { diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index b3906591c0..f4aaf798b7 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -6326,10 +6326,17 @@ public class Grid extends ResizeComposite implements * @since * @param detailsGenerator * the details generator to set + * @throws IllegalArgumentException + * if detailsGenerator is null; */ public void setDetailsGenerator(DetailsGenerator detailsGenerator) throws IllegalArgumentException { + if (detailsGenerator == null) { + throw new IllegalArgumentException( + "Details generator may not be null"); + } + this.detailsGenerator = detailsGenerator; // this will refresh all visible spacers diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index 991cb0537d..cf2284a62e 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -95,7 +95,7 @@ public class RpcDataProviderExtension extends AbstractExtension { // private implementation } - void setActiveRange(Range newActiveRange) { + public void setActiveRange(Range newActiveRange) { final Range[] removed = activeRange.partitionWith(newActiveRange); final Range[] added = newActiveRange.partitionWith(activeRange); @@ -163,7 +163,7 @@ public class RpcDataProviderExtension extends AbstractExtension { return String.valueOf(rollingIndex++); } - String getKey(Object itemId) { + public String getKey(Object itemId) { String key = itemIdToKey.get(itemId); if (key == null) { key = nextKey(); @@ -240,6 +240,20 @@ public class RpcDataProviderExtension extends AbstractExtension { return itemIds; } + /** + * Gets the row index for a given item. + * + * @since + * @param itemId + * the item id of the item for which to get the item + * @return the index of the item, or -1 if no such item could be found + */ + @SuppressWarnings("boxing") + public int getIndex(Object itemId) { + Integer integer = indexToItemId.inverse().get(itemId); + return integer != null ? integer : -1; + } + /** * Pin an item id to be cached indefinitely. *

@@ -304,7 +318,7 @@ public class RpcDataProviderExtension extends AbstractExtension { return pinnedItemIds.contains(itemId); } - Object itemIdAtIndex(int index) { + private Object itemIdAtIndex(int index) { return indexToItemId.get(Integer.valueOf(index)); } } @@ -727,6 +741,12 @@ public class RpcDataProviderExtension extends AbstractExtension { /** Size possibly changed with a bare ItemSetChangeEvent */ private boolean bareItemSetTriggeredSizeChange = false; + /** + * This map represents all the details that are user-defined as visible. + * This does not reflect the status in the DOM. + */ + private Set visibleDetails = new HashSet(); + /** * Creates a new data provider using the given container. * @@ -859,6 +879,10 @@ public class RpcDataProviderExtension extends AbstractExtension { rowObject.put(GridState.JSONKEY_DATA, rowData); rowObject.put(GridState.JSONKEY_ROWKEY, keyMapper.getKey(itemId)); + if (visibleDetails.contains(itemId)) { + rowObject.put(GridState.JSONKEY_DETAILS_VISIBLE, true); + } + rowReference.set(itemId); CellStyleGenerator cellStyleGenerator = grid.getCellStyleGenerator(); @@ -1116,4 +1140,46 @@ public class RpcDataProviderExtension extends AbstractExtension { return Logger.getLogger(RpcDataProviderExtension.class.getName()); } + /** + * Marks a row's details to be visible or hidden. + *

+ * If that row is currently in the client side's cache, this information + * will be sent over to the client. + * + * @since + * @param itemId + * the id of the item of which to change the details visibility + * @param visible + * true to show the details, false to + * hide + */ + public void setDetailsVisible(Object itemId, boolean visible) { + final boolean modified; + if (visible) { + modified = visibleDetails.add(itemId); + } else { + modified = visibleDetails.remove(itemId); + } + + int rowIndex = keyMapper.getIndex(itemId); + boolean modifiedRowIsActive = activeRowHandler.activeRange + .contains(rowIndex); + if (modified && modifiedRowIsActive) { + updateRowData(itemId); + } + } + + /** + * Checks whether the details for a row is marked as visible. + * + * @since + * @param itemId + * the id of the item of which to check the visibility + * @return true iff the detials are visible for the item. This + * might return true even if the row is not currently + * visible in the DOM + */ + public boolean isDetailsVisible(Object itemId) { + return visibleDetails.contains(itemId); + } } diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 22ef0333c2..b56bb0d036 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -164,6 +164,34 @@ import elemental.json.JsonValue; public class Grid extends AbstractComponent implements SelectionNotifier, SortNotifier, SelectiveRenderer, ItemClickNotifier { + /** + * A callback interface for generating details for a particular row in Grid. + * + * @since + * @author Vaadin Ltd + */ + public interface DetailsGenerator extends Serializable { + + /** A details generator that provides no details */ + public DetailsGenerator NULL = new DetailsGenerator() { + @Override + public Component getDetails(RowReference rowReference) { + return null; + } + }; + + /** + * This method is called for whenever a new details row needs to be + * generated. + * + * @param rowReference + * the reference for the row for which to generate details + * @return the details for the given row, or null to leave + * the details empty. + */ + Component getDetails(RowReference rowReference); + } + /** * Custom field group that allows finding property types before an item has * been bound. @@ -2888,6 +2916,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier, private EditorErrorHandler editorErrorHandler = new DefaultEditorErrorHandler(); + private DetailsGenerator detailsGenerator = DetailsGenerator.NULL; + private static final Method SELECTION_CHANGE_METHOD = ReflectTools .findMethod(SelectionListener.class, "select", SelectionEvent.class); @@ -5093,4 +5123,65 @@ public class Grid extends AbstractComponent implements SelectionNotifier, public void recalculateColumnWidths() { getRpcProxy(GridClientRpc.class).recalculateColumnWidths(); } + + /** + * Sets a new details generator for row details. + *

+ * The currently opened row details will be re-rendered. + * + * @since + * @param detailsGenerator + * the details generator to set + * @throws IllegalArgumentException + * if detailsGenerator is null; + */ + public void setDetailsGenerator(DetailsGenerator detailsGenerator) + throws IllegalArgumentException { + if (detailsGenerator == null) { + throw new IllegalArgumentException( + "Details generator may not be null"); + } else if (detailsGenerator == this.detailsGenerator) { + return; + } + + this.detailsGenerator = detailsGenerator; + + getLogger().warning("[[details]] update details on generator swap"); + } + + /** + * Gets the current details generator for row details. + * + * @since + * @return the detailsGenerator the current details generator + */ + public DetailsGenerator getDetailsGenerator() { + return detailsGenerator; + } + + /** + * Shows or hides the details for a specific item. + * + * @since + * @param itemId + * the id of the item for which to set details visibility + * @param visible + * true to show the details, or false + * to hide them + */ + public void setDetailsVisible(Object itemId, boolean visible) { + datasourceExtension.setDetailsVisible(itemId, visible); + } + + /** + * Checks whether details are visible for the given item. + * + * @since + * @param itemId + * the id of the item for which to check details visibility + * @return true iff the details are visible + */ + public boolean isDetailsVisible(Object itemId) { + return datasourceExtension.isDetailsVisible(itemId); + } } diff --git a/shared/src/com/vaadin/shared/ui/grid/GridState.java b/shared/src/com/vaadin/shared/ui/grid/GridState.java index 7018df1413..81e1827420 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridState.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridState.java @@ -102,6 +102,16 @@ public class GridState extends AbstractComponentState { */ public static final String JSONKEY_CELLSTYLES = "cs"; + /** + * The key that tells whether details are visible for the row + * + * @see com.vaadin.ui.Grid#setDetailsGenerator(com.vaadin.ui.Grid.DetailsGenerator) + * @see com.vaadin.ui.Grid#setDetailsVisible(Object, boolean) + * @see com.vaadin.shared.data.DataProviderRpc#setRowData(int, + * elemental.json.JsonArray) + * */ + public static final String JSONKEY_DETAILS_VISIBLE = "dv"; + /** * Columns in grid. */ diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index e5a46894b8..f0c4b3d9c0 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -248,6 +248,8 @@ public class GridBasicFeatures extends AbstractComponentTest { addFilterActions(); + createDetailsActions(); + this.grid = grid; return grid; } @@ -1051,6 +1053,18 @@ public class GridBasicFeatures extends AbstractComponentTest { }, null); } + private void createDetailsActions() { + createBooleanAction("firstItemId", "Details", false, + new Command() { + @Override + @SuppressWarnings("boxing") + public void execute(Grid g, Boolean visible, Object data) { + g.setDetailsVisible(g.getContainerDataSource() + .firstItemId(), visible); + } + }); + } + @Override protected Integer getTicketNumber() { return 12829; 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 new file mode 100644 index 0000000000..01d2ba55eb --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java @@ -0,0 +1,76 @@ +/* + * Copyright 2000-2014 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 + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.grid.basicfeatures.server; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; + +import org.junit.Before; +import org.junit.Test; +import org.openqa.selenium.NoSuchElementException; + +import com.vaadin.testbench.annotations.RunLocally; +import com.vaadin.testbench.parallel.Browser; +import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; + +@RunLocally(Browser.PHANTOMJS) +public class GridDetailsServerTest extends GridBasicFeaturesTest { + private static final String[] FIRST_ITEM_DETAILS = new String[] { + "Component", "Details", "firstItemId" }; + + @Before + public void setUp() { + openTestURL(); + } + + @Test + public void openVisibleDetails() { + try { + getGridElement().getDetails(0); + fail("Expected NoSuchElementException"); + } catch (NoSuchElementException ignore) { + // expected + } + selectMenuPath(FIRST_ITEM_DETAILS); + assertNotNull("details should've opened", getGridElement() + .getDetails(0)); + } + + @Test(expected = NoSuchElementException.class) + public void closeVisibleDetails() { + selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(FIRST_ITEM_DETAILS); + getGridElement().getDetails(0); + } + + @Test + public void openDetailsOutsideOfActiveRange() { + getGridElement().scroll(10000); + selectMenuPath(FIRST_ITEM_DETAILS); + getGridElement().scroll(0); + assertNotNull("details should've been opened", getGridElement() + .getDetails(0)); + } + + @Test(expected = NoSuchElementException.class) + public void closeDetailsOutsideOfActiveRange() { + selectMenuPath(FIRST_ITEM_DETAILS); + getGridElement().scroll(10000); + selectMenuPath(FIRST_ITEM_DETAILS); + getGridElement().scroll(0); + getGridElement().getDetails(0); + } +} -- cgit v1.2.3 From a1619ee73dc18eecda22056541826a3c8bb65a5c Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Tue, 10 Mar 2015 17:02:02 +0200 Subject: Grid's Details can now be Components (#16644) Change-Id: If67dd2e86cf41c57f208a3691e2cb7a5a29c133c --- .../vaadin/client/connectors/GridConnector.java | 154 +++++++++++++- .../client/data/AbstractRemoteDataSource.java | 1 + .../com/vaadin/data/RpcDataProviderExtension.java | 31 ++- server/src/com/vaadin/ui/Grid.java | 230 ++++++++++++++++++++- .../component/grid/DataProviderExtension.java | 2 +- .../shared/ui/grid/ConnectorIndexChange.java | 143 +++++++++++++ .../com/vaadin/shared/ui/grid/GridClientRpc.java | 15 ++ .../com/vaadin/shared/ui/grid/GridServerRpc.java | 17 ++ .../grid/basicfeatures/GridBasicFeatures.java | 74 +++++++ .../server/GridDetailsServerTest.java | 100 ++++++++- 10 files changed, 754 insertions(+), 13 deletions(-) create mode 100644 shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index f476982c15..70ad2504d8 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -29,12 +29,14 @@ import java.util.Set; import java.util.logging.Logger; import com.google.gwt.core.client.Scheduler; +import com.google.gwt.core.client.Scheduler.RepeatingCommand; import com.google.gwt.core.client.Scheduler.ScheduledCommand; import com.google.gwt.dom.client.NativeEvent; -import com.google.gwt.user.client.ui.Label; +import com.google.gwt.user.client.Timer; import com.google.gwt.user.client.ui.Widget; import com.vaadin.client.ComponentConnector; import com.vaadin.client.ConnectorHierarchyChangeEvent; +import com.vaadin.client.DeferredWorker; import com.vaadin.client.MouseEventDetailsBuilder; import com.vaadin.client.annotations.OnStateChange; import com.vaadin.client.communication.StateChangeEvent; @@ -72,8 +74,10 @@ import com.vaadin.client.widgets.Grid.FooterCell; import com.vaadin.client.widgets.Grid.FooterRow; import com.vaadin.client.widgets.Grid.HeaderCell; import com.vaadin.client.widgets.Grid.HeaderRow; +import com.vaadin.shared.Connector; import com.vaadin.shared.data.sort.SortDirection; import com.vaadin.shared.ui.Connect; +import com.vaadin.shared.ui.grid.ConnectorIndexChange; import com.vaadin.shared.ui.grid.EditorClientRpc; import com.vaadin.shared.ui.grid.EditorServerRpc; import com.vaadin.shared.ui.grid.GridClientRpc; @@ -103,7 +107,8 @@ import elemental.json.JsonValue; */ @Connect(com.vaadin.ui.Grid.class) public class GridConnector extends AbstractHasComponentsConnector implements - SimpleManagedLayout, RpcDataSourceConnector.DetailsListener { + SimpleManagedLayout, RpcDataSourceConnector.DetailsListener, + DeferredWorker { private static final class CustomCellStyleGenerator implements CellStyleGenerator { @@ -362,11 +367,119 @@ public class GridConnector extends AbstractHasComponentsConnector implements } } - private class CustomDetailsGenerator implements DetailsGenerator { + private static class CustomDetailsGenerator implements DetailsGenerator { + + private final Map indexToDetailsMap = new HashMap(); + @Override + @SuppressWarnings("boxing") public Widget getDetails(int rowIndex) { - // TODO - return new Label("[todo]"); + ComponentConnector componentConnector = indexToDetailsMap + .get(rowIndex); + if (componentConnector != null) { + return componentConnector.getWidget(); + } else { + return null; + } + } + + public void setDetailsConnectorChanges(Set changes) { + /* + * To avoid overwriting connectors while moving them about, we'll + * take all the affected connectors, first all remove those that are + * removed or moved, then we add back those that are moved or added. + */ + + /* Remove moved/removed connectors from bookkeeping */ + for (ConnectorIndexChange change : changes) { + Integer oldIndex = change.getOldIndex(); + Connector removedConnector = indexToDetailsMap.remove(oldIndex); + + Connector connector = change.getConnector(); + assert removedConnector == null || connector == null + || removedConnector.equals(connector) : "Index " + + oldIndex + " points to " + removedConnector + + " while " + connector + " was expected"; + } + + /* Add moved/added connectors to bookkeeping */ + for (ConnectorIndexChange change : changes) { + Integer newIndex = change.getNewIndex(); + ComponentConnector connector = (ComponentConnector) change + .getConnector(); + + if (connector != null) { + assert newIndex != null : "An existing connector has a missing new index."; + + ComponentConnector prevConnector = indexToDetailsMap.put( + newIndex, connector); + + assert prevConnector == null : "Connector collision at index " + + newIndex + + " between old " + + prevConnector + + " and new " + connector; + } + } + } + } + + @SuppressWarnings("boxing") + private class DetailsConnectorFetcher implements DeferredWorker { + + /** A flag making sure that we don't call scheduleFinally many times. */ + private boolean fetcherHasBeenCalled = false; + + /** A rolling counter for unique values. */ + private int detailsFetchCounter = 0; + + /** A collection that tracks the amount of requests currently underway. */ + private Set pendingFetches = new HashSet(5); + + private final ScheduledCommand lazyDetailsFetcher = new ScheduledCommand() { + @Override + public void execute() { + int currentFetchId = detailsFetchCounter++; + pendingFetches.add(currentFetchId); + getRpcProxy(GridServerRpc.class).sendDetailsComponents( + currentFetchId); + fetcherHasBeenCalled = false; + + assert assertRequestDoesNotTimeout(currentFetchId); + } + }; + + public void schedule() { + if (!fetcherHasBeenCalled) { + Scheduler.get().scheduleFinally(lazyDetailsFetcher); + fetcherHasBeenCalled = true; + } + } + + public void responseReceived(int fetchId) { + boolean success = pendingFetches.remove(fetchId); + assert success : "Received a response with an unidentified fetch id"; + } + + @Override + public boolean isWorkPending() { + return fetcherHasBeenCalled || !pendingFetches.isEmpty(); + } + + private boolean assertRequestDoesNotTimeout(final int fetchId) { + /* + * This method will not be compiled without asserts enabled. This + * only makes sure that any request does not time out. + * + * TODO Should this be an explicit check? Is it worth the overhead? + */ + new Timer() { + @Override + public void run() { + assert !pendingFetches.contains(fetchId); + } + }.schedule(1000); + return true; } } @@ -417,6 +530,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements private String lastKnownTheme = null; + private final CustomDetailsGenerator customDetailsGenerator = new CustomDetailsGenerator(); + + private final DetailsConnectorFetcher detailsConnectorFetcher = new DetailsConnectorFetcher(); + @Override @SuppressWarnings("unchecked") public Grid getWidget() { @@ -469,6 +586,24 @@ public class GridConnector extends AbstractHasComponentsConnector implements public void recalculateColumnWidths() { getWidget().recalculateColumnWidths(); } + + @Override + public void setDetailsConnectorChanges( + Set connectorChanges, int fetchId) { + customDetailsGenerator + .setDetailsConnectorChanges(connectorChanges); + + // refresh moved/added details rows + for (ConnectorIndexChange change : connectorChanges) { + Integer newIndex = change.getNewIndex(); + if (newIndex != null) { + int index = newIndex.intValue(); + getWidget().setDetailsVisible(index, false); + getWidget().setDetailsVisible(index, true); + } + } + detailsConnectorFetcher.responseReceived(fetchId); + } }); getWidget().addSelectionHandler(internalSelectionChangeHandler); @@ -512,7 +647,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements getWidget().setEditorHandler(new CustomEditorHandler()); - getWidget().setDetailsGenerator(new CustomDetailsGenerator()); + getWidget().setDetailsGenerator(customDetailsGenerator); getLayoutManager().registerDependency(this, getWidget().getElement()); @@ -1017,5 +1152,12 @@ public class GridConnector extends AbstractHasComponentsConnector implements } else { getWidget().setDetailsVisible(rowIndex, false); } + + detailsConnectorFetcher.schedule(); + } + + @Override + public boolean isWorkPending() { + return detailsConnectorFetcher.isWorkPending(); } } diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java index 1de271c646..0ac4c33c83 100644 --- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java +++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java @@ -570,6 +570,7 @@ public abstract class AbstractRemoteDataSource implements DataSource { Profiler.leave("AbstractRemoteDataSource.insertRowData"); } + @SuppressWarnings("boxing") private void moveRowFromIndexToIndex(int oldIndex, int newIndex) { T row = indexToRowMap.remove(oldIndex); if (indexToRowMap.containsKey(newIndex)) { diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index cf2284a62e..62b8214cbd 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -51,6 +51,7 @@ import com.vaadin.ui.Grid; import com.vaadin.ui.Grid.CellReference; import com.vaadin.ui.Grid.CellStyleGenerator; import com.vaadin.ui.Grid.Column; +import com.vaadin.ui.Grid.DetailComponentManager; import com.vaadin.ui.Grid.RowReference; import com.vaadin.ui.Grid.RowStyleGenerator; import com.vaadin.ui.renderers.Renderer; @@ -113,6 +114,7 @@ public class RpcDataProviderExtension extends AbstractExtension { final Object itemId = indexToItemId.get(ii); if (!isPinned(itemId)) { + detailComponentManager.destroyDetails(itemId); itemIdToKey.remove(itemId); indexToItemId.remove(ii); } @@ -154,6 +156,7 @@ public class RpcDataProviderExtension extends AbstractExtension { } indexToItemId.forcePut(index, itemId); + detailComponentManager.createDetails(itemId, index); } index++; } @@ -747,14 +750,18 @@ public class RpcDataProviderExtension extends AbstractExtension { */ private Set visibleDetails = new HashSet(); + private DetailComponentManager detailComponentManager; + /** * Creates a new data provider using the given container. * * @param container * the container to make available */ - public RpcDataProviderExtension(Indexed container) { + public RpcDataProviderExtension(Indexed container, + DetailComponentManager detailComponentManager) { this.container = container; + this.detailComponentManager = detailComponentManager; rpc = getRpcProxy(DataProviderRpc.class); registerRpc(new DataRequestRpc() { @@ -1018,6 +1025,10 @@ public class RpcDataProviderExtension extends AbstractExtension { JsonArray rowArray = Json.createArray(); rowArray.set(0, row); rpc.setRowData(index, rowArray); + + if (isDetailsVisible(itemId)) { + detailComponentManager.createDetails(itemId, index); + } } } @@ -1155,10 +1166,28 @@ public class RpcDataProviderExtension extends AbstractExtension { */ public void setDetailsVisible(Object itemId, boolean visible) { final boolean modified; + if (visible) { modified = visibleDetails.add(itemId); + + /* + * We don't want to create the component here, since the component + * might be out of view, and thus we don't know where the details + * should end up on the client side. This is also a great thing to + * optimize away, so that in case a lot of things would be opened at + * once, a huge chunk of data doesn't get sent over immediately. + */ + } else { modified = visibleDetails.remove(itemId); + + /* + * Here we can try to destroy the component no matter what. The + * component has been removed and should be detached from the + * component hierarchy. The details row will be closed on the client + * side automatically. + */ + detailComponentManager.destroyDetails(itemId); } int rowIndex = keyMapper.getIndex(itemId); diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index b56bb0d036..da5cedd999 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -36,6 +36,8 @@ import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import com.google.gwt.thirdparty.guava.common.collect.BiMap; +import com.google.gwt.thirdparty.guava.common.collect.HashBiMap; import com.google.gwt.thirdparty.guava.common.collect.Sets; import com.google.gwt.thirdparty.guava.common.collect.Sets.SetView; import com.vaadin.data.Container; @@ -75,6 +77,7 @@ import com.vaadin.server.KeyMapper; import com.vaadin.server.VaadinSession; import com.vaadin.shared.MouseEventDetails; import com.vaadin.shared.data.sort.SortDirection; +import com.vaadin.shared.ui.grid.ConnectorIndexChange; import com.vaadin.shared.ui.grid.EditorClientRpc; import com.vaadin.shared.ui.grid.EditorServerRpc; import com.vaadin.shared.ui.grid.GridClientRpc; @@ -183,6 +186,11 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * This method is called for whenever a new details row needs to be * generated. + *

+ * Note: If a component gets generated, it may not be manually + * attached anywhere, nor may it be a reused instance – each + * invocation of this method should produce a unique and isolated + * component instance. * * @param rowReference * the reference for the row for which to generate details @@ -2809,6 +2817,208 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } } + /** + * A class that makes detail component related internal communication + * possible between {@link RpcDataProviderExtension} and grid. + * + * @since + * @author Vaadin Ltd + */ + public final class DetailComponentManager implements Serializable { + /** + * This map represents all the components that have been requested for + * each item id. + *

+ * Normally this map is consistent with what is displayed in the + * component hierarchy (and thus the DOM). The only time this map is out + * of sync with the DOM is between the any calls to + * {@link #createDetails(Object, int)} or + * {@link #destroyDetails(Object)}, and + * {@link GridClientRpc#setDetailsConnectorChanges(Set)}. + *

+ * This is easily checked: if {@link #unattachedComponents} is + * {@link Collection#isEmpty() empty}, then this field is consistent + * with the connector hierarchy. + */ + private final Map visibleDetailsComponents = new HashMap(); + + /** A lookup map for which row contains which details component. */ + private BiMap rowIndexToDetails = HashBiMap + .create(); + + /** + * A copy of {@link #rowIndexToDetails} from its last stable state. Used + * for creating a diff against {@link #rowIndexToDetails}. + * + * @see #getAndResetConnectorChanges() + */ + private BiMap prevRowIndexToDetails = HashBiMap + .create(); + + /** + * A set keeping track on components that have been created, but not + * attached. They should be attached at some later point in time. + *

+ * This isn't strictly requried, but it's a handy explicit log. You + * could find out the same thing by taking out all the other components + * and checking whether Grid is their parent or not. + */ + private final Set unattachedComponents = new HashSet(); + + /** + * Creates a details component by the request of the client side, with + * the help of the user-defined {@link DetailsGenerator}. + *

+ * Also keeps internal bookkeeping up to date. + * + * @param itemId + * the item id for which to create the details component. + * Assumed not null and that a component is not + * currently present for this item previously + * @param rowIndex + * the row index for {@code itemId} + * @throws IllegalStateException + * if the current details generator provides a component + * that was manually attached, or if the same instance has + * already been provided + */ + public void createDetails(Object itemId, int rowIndex) + throws IllegalStateException { + assert itemId != null : "itemId was null"; + Integer newRowIndex = Integer.valueOf(rowIndex); + + assert !visibleDetailsComponents.containsKey(itemId) : "itemId already has a component. Should be destroyed first."; + + RowReference rowReference = new RowReference(Grid.this); + rowReference.set(itemId); + + Component details = getDetailsGenerator().getDetails(rowReference); + if (details != null) { + + if (details.getParent() != null) { + String generatorName = getDetailsGenerator().getClass() + .getName(); + throw new IllegalStateException(generatorName + + " generated a details component that already " + + "was attached. (itemId: " + itemId + ", row: " + + rowIndex + ", component: " + details); + } + + if (rowIndexToDetails.containsValue(details)) { + String generatorName = getDetailsGenerator().getClass() + .getName(); + throw new IllegalStateException(generatorName + + " provided a details component that already " + + "exists in Grid. (itemId: " + itemId + ", row: " + + rowIndex + ", component: " + details); + } + + visibleDetailsComponents.put(itemId, details); + rowIndexToDetails.put(newRowIndex, details); + unattachedComponents.add(details); + } + + /* + * Don't attach the components here. It's done by + * GridServerRpc.sendDetailsComponents in a separate roundtrip. + */ + } + + /** + * Destroys correctly a details component, by the request of the client + * side. + *

+ * Also keeps internal bookkeeping up to date. + * + * @param itemId + * the item id for which to destroy the details component + */ + public void destroyDetails(Object itemId) { + Component removedComponent = visibleDetailsComponents + .remove(itemId); + if (removedComponent == null) { + return; + } + + rowIndexToDetails.inverse().remove(removedComponent); + + removedComponent.setParent(null); + markAsDirty(); + } + + /** + * Gets all details components that are currently attached to the grid. + *

+ * Used internally by the Grid object. + * + * @return all details components that are currently attached to the + * grid + */ + Collection getComponents() { + Set components = new HashSet( + visibleDetailsComponents.values()); + components.removeAll(unattachedComponents); + return components; + } + + /** + * Gets information on how the connectors have changed. + *

+ * This method only returns the changes that have been made between two + * calls of this method. I.e. Calling this method once will reset the + * state for the next state. + *

+ * Used internally by the Grid object. + * + * @return information on how the connectors have changed + */ + Set getAndResetConnectorChanges() { + Set changes = new HashSet(); + + // populate diff with added/changed + for (Entry entry : rowIndexToDetails.entrySet()) { + Component component = entry.getValue(); + assert component != null : "rowIndexToDetails contains a null component"; + + Integer newIndex = entry.getKey(); + Integer oldIndex = prevRowIndexToDetails.inverse().get( + component); + + /* + * only attach components. Detaching already happened in + * destroyDetails. + */ + if (newIndex != null && oldIndex == null) { + assert unattachedComponents.contains(component) : "unattachedComponents does not contain component for index " + + newIndex + " (" + component + ")"; + component.setParent(Grid.this); + unattachedComponents.remove(component); + } + + if (!SharedUtil.equals(oldIndex, newIndex)) { + changes.add(new ConnectorIndexChange(component, oldIndex, + newIndex)); + } + } + + // populate diff with removed + for (Entry entry : prevRowIndexToDetails + .entrySet()) { + Integer oldIndex = entry.getKey(); + Component component = entry.getValue(); + Integer newIndex = rowIndexToDetails.inverse().get(component); + if (newIndex == null) { + changes.add(new ConnectorIndexChange(null, oldIndex, null)); + } + } + + // reset diff map + prevRowIndexToDetails = HashBiMap.create(rowIndexToDetails); + + return changes; + } + } + /** * The data source attached to the grid */ @@ -2916,8 +3126,15 @@ public class Grid extends AbstractComponent implements SelectionNotifier, private EditorErrorHandler editorErrorHandler = new DefaultEditorErrorHandler(); + /** + * The user-defined details generator. + * + * @see #setDetailsGenerator(DetailsGenerator) + */ private DetailsGenerator detailsGenerator = DetailsGenerator.NULL; + private final DetailComponentManager detailComponentManager = new DetailComponentManager(); + private static final Method SELECTION_CHANGE_METHOD = ReflectTools .findMethod(SelectionListener.class, "select", SelectionEvent.class); @@ -3118,6 +3335,13 @@ public class Grid extends AbstractComponent implements SelectionNotifier, fireEvent(new ItemClickEvent(Grid.this, item, itemId, propertyId, details)); } + + @Override + public void sendDetailsComponents(int fetchId) { + getRpcProxy(GridClientRpc.class).setDetailsConnectorChanges( + detailComponentManager.getAndResetConnectorChanges(), + fetchId); + } }); registerRpc(new EditorServerRpc() { @@ -3278,7 +3502,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier, sortOrder.clear(); } - datasourceExtension = new RpcDataProviderExtension(container); + datasourceExtension = new RpcDataProviderExtension(container, + detailComponentManager); datasourceExtension.extend(this, columnKeys); /* @@ -4607,6 +4832,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } componentList.addAll(getEditorFields()); + + componentList.addAll(detailComponentManager.getComponents()); + return componentList.iterator(); } diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java b/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java index 9ecf131c5b..54f5dcdbc7 100644 --- a/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java +++ b/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java @@ -47,7 +47,7 @@ public class DataProviderExtension { container = new IndexedContainer(); populate(container); - dataProvider = new RpcDataProviderExtension(container); + dataProvider = new RpcDataProviderExtension(container, null); keyMapper = dataProvider.getKeyMapper(); } diff --git a/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java b/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java new file mode 100644 index 0000000000..16be92007e --- /dev/null +++ b/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java @@ -0,0 +1,143 @@ +/* + * Copyright 2000-2014 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 + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.shared.ui.grid; + +import java.io.Serializable; + +import com.vaadin.shared.Connector; + +/** + * A description of an indexing modification for a connector. This is used by + * Grid by internal bookkeeping updates. + * + * @since + * @author Vaadin Ltd + */ +public class ConnectorIndexChange implements Serializable { + + private Connector connector; + private Integer oldIndex; + private Integer newIndex; + + /** Create a new connector index change */ + public ConnectorIndexChange() { + } + + /** + * Convenience constructor for setting all the fields in one line. + *

+ * Calling this constructor will also assert that the state of the pojo is + * consistent by internal assumptions. + * + * @param connector + * the changed connector + * @param oldIndex + * the old index + * @param newIndex + * the new index + */ + public ConnectorIndexChange(Connector connector, Integer oldIndex, + Integer newIndex) { + this.connector = connector; + this.oldIndex = oldIndex; + this.newIndex = newIndex; + + assert assertStateIsOk(); + } + + private boolean assertStateIsOk() { + assert (connector != null && newIndex != null) + || (connector == null && oldIndex != null && newIndex == null) : "connector: " + + nullityString(connector) + + ", oldIndex: " + + nullityString(oldIndex) + + ", newIndex: " + + nullityString(newIndex); + return true; + } + + private static String nullityString(Object object) { + return object == null ? "null" : "non-null"; + } + + /** + * Gets the old index for the connector. + *

+ * If null, the connector is recently added. This means that + * {@link #getConnector()} is expected not to return null. + * + * @return the old index for the connector + */ + public Integer getOldIndex() { + assert assertStateIsOk(); + return oldIndex; + } + + /** + * Gets the new index for the connector. + *

+ * If null, the connector should be removed. This means that + * {@link #getConnector()} is expected to return null as well. + * + * @return the new index for the connector + */ + public Integer getNewIndex() { + assert assertStateIsOk(); + return newIndex; + } + + /** + * Gets the changed connector. + * + * @return the changed connector. Might be null + */ + public Connector getConnector() { + assert assertStateIsOk(); + return connector; + } + + /** + * Sets the changed connector. + * + * @param connector + * the changed connector. May be null + */ + public void setConnector(Connector connector) { + this.connector = connector; + } + + /** + * Sets the old index + * + * @param oldIndex + * the old index. May be null if a new connector is + * being inserted + */ + public void setOldIndex(Integer oldIndex) { + this.oldIndex = oldIndex; + } + + /** + * Sets the new index + * + * @param newIndex + * the new index. May be null if a connector is + * being removed + */ + public void setNewIndex(Integer newIndex) { + this.newIndex = newIndex; + } +} diff --git a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java index 4ba081b5df..672c83ff53 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java @@ -15,6 +15,8 @@ */ package com.vaadin.shared.ui.grid; +import java.util.Set; + import com.vaadin.shared.communication.ClientRpc; /** @@ -55,4 +57,17 @@ public interface GridClientRpc extends ClientRpc { */ public void recalculateColumnWidths(); + /** + * Informs the GridConnector on how the indexing of details connectors has + * changed. + * + * @since + * @param connectorChanges + * the indexing changes of details connectors + * @param fetchId + * the id of the request for fetching the changes + */ + public void setDetailsConnectorChanges( + Set connectorChanges, int fetchId); + } diff --git a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java index c90a016383..a2ef7d0bb7 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java @@ -47,4 +47,21 @@ public interface GridServerRpc extends ServerRpc { * mouse event details */ void itemClick(String rowKey, String columnId, MouseEventDetails details); + + /** + * This is a trigger for Grid to send whatever has changed regarding the + * details components. + *

+ * The components can't be sent eagerly, since they are generated as a side + * effect in + * {@link com.vaadin.data.RpcDataProviderExtension#beforeClientResponse(boolean)} + * , and that is too late to change the hierarchy. So we need this + * round-trip to work around that limitation. + * + * @since + * @param fetchId + * an unique identifier for the request + * @see com.vaadin.ui.Grid#setDetailsVisible(Object, boolean) + */ + void sendDetailsComponents(int fetchId); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index f0c4b3d9c0..08f0d7d5d2 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -46,6 +46,7 @@ import com.vaadin.tests.components.AbstractComponentTest; import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.Component; import com.vaadin.ui.Grid; import com.vaadin.ui.Grid.CellReference; import com.vaadin.ui.Grid.CellStyleGenerator; @@ -58,6 +59,8 @@ import com.vaadin.ui.Grid.RowReference; import com.vaadin.ui.Grid.RowStyleGenerator; import com.vaadin.ui.Grid.SelectionMode; import com.vaadin.ui.Grid.SelectionModel; +import com.vaadin.ui.Label; +import com.vaadin.ui.Panel; import com.vaadin.ui.renderers.DateRenderer; import com.vaadin.ui.renderers.HtmlRenderer; import com.vaadin.ui.renderers.NumberRenderer; @@ -109,6 +112,8 @@ public class GridBasicFeatures extends AbstractComponentTest { } }; + private Panel detailsPanel; + @Override @SuppressWarnings("unchecked") protected Grid constructComponent() { @@ -1054,6 +1059,64 @@ public class GridBasicFeatures extends AbstractComponentTest { } private void createDetailsActions() { + createClickAction("custom details generator", "Details", + new Command() { + @Override + public void execute(Grid c, Void value, Object data) { + grid.setDetailsGenerator(new Grid.DetailsGenerator() { + private int seq = 0; + + @Override + public Component getDetails( + RowReference rowReference) { + return new Label("You are watching item id " + + rowReference.getItemId() + " (" + + (seq++) + ")"); + } + }); + } + }, null); + createClickAction("hierarchy details generator", "Details", + new Command() { + @Override + public void execute(Grid c, Void value, Object data) { + grid.setDetailsGenerator(new Grid.DetailsGenerator() { + @Override + public Component getDetails( + RowReference rowReference) { + detailsPanel = new Panel(); + detailsPanel.setContent(new Label("One")); + return detailsPanel; + } + }); + } + }, null); + + createClickAction("change hierarchy in generator", "Details", + new Command() { + @Override + public void execute(Grid c, Void value, Object data) { + Label label = (Label) detailsPanel.getContent(); + if (label.getValue().equals("One")) { + detailsPanel.setContent(new Label("Two")); + } else { + detailsPanel.setContent(new Label("One")); + } + } + }, null); + + createClickAction("toggle firstItemId", "Details", + new Command() { + @Override + public void execute(Grid g, Void value, Object data) { + Object firstItemId = g.getContainerDataSource() + .firstItemId(); + boolean toggle = g.isDetailsVisible(firstItemId); + g.setDetailsVisible(firstItemId, !toggle); + g.setDetailsVisible(firstItemId, toggle); + } + }, null); + createBooleanAction("firstItemId", "Details", false, new Command() { @Override @@ -1063,6 +1126,17 @@ public class GridBasicFeatures extends AbstractComponentTest { .firstItemId(), visible); } }); + + createBooleanAction("lastItemId-5", "Details", false, + new Command() { + @Override + @SuppressWarnings("boxing") + public void execute(Grid g, Boolean visible, Object data) { + Object fifthLastItemId = g.getContainerDataSource() + .getItemIds(ROWS - 6, 1).get(0); + g.setDetailsVisible(fifthLastItemId, visible); + } + }); } @Override 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 01d2ba55eb..e9e32cb1ca 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 @@ -15,21 +15,43 @@ */ package com.vaadin.tests.components.grid.basicfeatures.server; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; +import org.openqa.selenium.By; import org.openqa.selenium.NoSuchElementException; -import com.vaadin.testbench.annotations.RunLocally; -import com.vaadin.testbench.parallel.Browser; +import com.vaadin.testbench.TestBenchElement; +import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures; import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; -@RunLocally(Browser.PHANTOMJS) public class GridDetailsServerTest extends GridBasicFeaturesTest { + /** + * The reason to why last item details wasn't selected is that since it will + * exist only after the viewport has been scrolled into view, we wouldn't be + * able to scroll that particular details row into view, making tests + * awkward with two scroll commands back to back. + */ + private static final int ALMOST_LAST_ITEM_INDEX = GridBasicFeatures.ROWS - 5; + private static final String[] ALMOST_LAST_ITEM_DETAILS = new String[] { + "Component", "Details", "lastItemId-5" }; + private static final String[] FIRST_ITEM_DETAILS = new String[] { "Component", "Details", "firstItemId" }; + private static final String[] TOGGLE_FIRST_ITEM_DETAILS = new String[] { + "Component", "Details", "toggle firstItemId" }; + private static final String[] CUSTOM_DETAILS_GENERATOR = new String[] { + "Component", "Details", "custom details generator" }; + private static final String[] HIERARCHY_DETAILS_GENERATOR = new String[] { + "Component", "Details", "hierarchy details generator" }; + private static final String[] CHANGE_HIERARCHY = new String[] { + "Component", "Details", "change hierarchy in generator" }; @Before public void setUp() { @@ -53,7 +75,9 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { public void closeVisibleDetails() { selectMenuPath(FIRST_ITEM_DETAILS); selectMenuPath(FIRST_ITEM_DETAILS); - getGridElement().getDetails(0); + + // this will throw before assertNull + assertNull(getGridElement().getDetails(0)); } @Test @@ -73,4 +97,72 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { getGridElement().scroll(0); getGridElement().getDetails(0); } + + @Test + public void componentIsVisibleClientSide() { + selectMenuPath(CUSTOM_DETAILS_GENERATOR); + selectMenuPath(FIRST_ITEM_DETAILS); + + TestBenchElement details = getGridElement().getDetails(0); + assertNotNull("No widget detected inside details", + details.findElement(By.className("v-widget"))); + } + + @Test + public void togglingAVisibleDetailsRowWithSeparateRoundtrips() { + selectMenuPath(CUSTOM_DETAILS_GENERATOR); + selectMenuPath(FIRST_ITEM_DETAILS); // open + selectMenuPath(FIRST_ITEM_DETAILS); // close + selectMenuPath(FIRST_ITEM_DETAILS); // open + + TestBenchElement details = getGridElement().getDetails(0); + assertNotNull("No widget detected inside details", + details.findElement(By.className("v-widget"))); + } + + @Test + public void togglingAVisibleDetailsRowWithOneRoundtrip() { + selectMenuPath(CUSTOM_DETAILS_GENERATOR); + selectMenuPath(FIRST_ITEM_DETAILS); // open + + assertTrue("Unexpected generator content", + getGridElement().getDetails(0).getText().endsWith("(0)")); + selectMenuPath(TOGGLE_FIRST_ITEM_DETAILS); + assertTrue("New component was not displayed in the client", + getGridElement().getDetails(0).getText().endsWith("(1)")); + } + + @Test + @Ignore("This will be patched with https://dev.vaadin.com/review/#/c/7917/") + public void almosLastItemIdIsRendered() { + selectMenuPath(CUSTOM_DETAILS_GENERATOR); + selectMenuPath(ALMOST_LAST_ITEM_DETAILS); + scrollGridVerticallyTo(100000); + + TestBenchElement details = getGridElement().getDetails( + ALMOST_LAST_ITEM_INDEX); + assertNotNull(details); + assertTrue("Unexpected details content", + details.getText().endsWith(ALMOST_LAST_ITEM_INDEX + " (0)")); + } + + @Test + public void hierarchyChangesWorkInDetails() { + selectMenuPath(HIERARCHY_DETAILS_GENERATOR); + selectMenuPath(FIRST_ITEM_DETAILS); + assertEquals("One", getGridElement().getDetails(0).getText()); + selectMenuPath(CHANGE_HIERARCHY); + assertEquals("Two", getGridElement().getDetails(0).getText()); + } + + @Test + @Ignore("This will be patched with https://dev.vaadin.com/review/#/c/7917/") + public void hierarchyChangesWorkInDetailsWhileOutOfView() { + selectMenuPath(HIERARCHY_DETAILS_GENERATOR); + selectMenuPath(FIRST_ITEM_DETAILS); + scrollGridVerticallyTo(10000); + selectMenuPath(CHANGE_HIERARCHY); + scrollGridVerticallyTo(0); + assertEquals("Two", getGridElement().getDetails(0).getText()); + } } -- cgit v1.2.3 From 251ed2cbb633778d467e0a646ad6f87e3e8ca4bb Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Fri, 13 Mar 2015 11:32:30 +0200 Subject: Fixes a bug in Details being open on freshly retrieved Grid rows (#16644) Change-Id: Id337dd84ba0b0f09d55b3cdb0d8bfde67313ed21 --- .../com/vaadin/data/RpcDataProviderExtension.java | 5 +++- .../server/GridDetailsServerTest.java | 29 +++++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index 62b8214cbd..620933c379 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -156,7 +156,10 @@ public class RpcDataProviderExtension extends AbstractExtension { } indexToItemId.forcePut(index, itemId); - detailComponentManager.createDetails(itemId, index); + + if (visibleDetails.contains(itemId)) { + detailComponentManager.createDetails(itemId, index); + } } index++; } 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 e9e32cb1ca..e9b5b688d1 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 @@ -17,7 +17,6 @@ package com.vaadin.tests.components.grid.basicfeatures.server; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -76,8 +75,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { selectMenuPath(FIRST_ITEM_DETAILS); selectMenuPath(FIRST_ITEM_DETAILS); - // this will throw before assertNull - assertNull(getGridElement().getDetails(0)); + getGridElement().getDetails(0); } @Test @@ -120,6 +118,31 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { details.findElement(By.className("v-widget"))); } + @Test(expected = NoSuchElementException.class) + public void scrollingDoesNotCreateAFloodOfDetailsRows() { + selectMenuPath(CUSTOM_DETAILS_GENERATOR); + + // scroll somewhere to hit uncached rows + getGridElement().scrollToRow(101); + + // this should throw + getGridElement().getDetails(100); + } + + @Test + public void openingDetailsOutOfView() { + getGridElement().scrollToRow(500); + + selectMenuPath(CUSTOM_DETAILS_GENERATOR); + selectMenuPath(FIRST_ITEM_DETAILS); + + getGridElement().scrollToRow(0); + + // if this fails, it'll fail before the assertNotNull + assertNotNull("unexpected null details row", getGridElement() + .getDetails(0)); + } + @Test public void togglingAVisibleDetailsRowWithOneRoundtrip() { selectMenuPath(CUSTOM_DETAILS_GENERATOR); -- cgit v1.2.3 From b06b1d68469e49e7784de342f0dcf9de64b35f5a Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Mon, 16 Mar 2015 11:45:22 +0200 Subject: Adds details generator swap support for Grid (#16644) Change-Id: I741970a7bcebd27d3aa28d608d767b4b4f063ae8 --- .../vaadin/client/connectors/GridConnector.java | 32 ++-- client/src/com/vaadin/client/widgets/Grid.java | 5 +- .../com/vaadin/data/RpcDataProviderExtension.java | 7 + server/src/com/vaadin/ui/Grid.java | 61 ++++++-- .../shared/ui/grid/ConnectorIndexChange.java | 143 ------------------ .../shared/ui/grid/DetailsConnectorChange.java | 147 +++++++++++++++++++ .../com/vaadin/shared/ui/grid/GridClientRpc.java | 5 +- .../grid/basicfeatures/GridBasicFeatures.java | 131 ++++++++++------- .../server/GridDetailsServerTest.java | 162 +++++++++++++++------ 9 files changed, 429 insertions(+), 264 deletions(-) delete mode 100644 shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java create mode 100644 shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 1787dc5c97..e6b9c89483 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -77,7 +77,7 @@ import com.vaadin.client.widgets.Grid.HeaderRow; import com.vaadin.shared.Connector; import com.vaadin.shared.data.sort.SortDirection; import com.vaadin.shared.ui.Connect; -import com.vaadin.shared.ui.grid.ConnectorIndexChange; +import com.vaadin.shared.ui.grid.DetailsConnectorChange; import com.vaadin.shared.ui.grid.EditorClientRpc; import com.vaadin.shared.ui.grid.EditorServerRpc; import com.vaadin.shared.ui.grid.GridClientRpc; @@ -382,7 +382,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements } } - public void setDetailsConnectorChanges(Set changes) { + public void setDetailsConnectorChanges( + Set changes) { /* * To avoid overwriting connectors while moving them about, we'll * take all the affected connectors, first all remove those that are @@ -390,7 +391,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements */ /* Remove moved/removed connectors from bookkeeping */ - for (ConnectorIndexChange change : changes) { + for (DetailsConnectorChange change : changes) { Integer oldIndex = change.getOldIndex(); Connector removedConnector = indexToDetailsMap.remove(oldIndex); @@ -402,7 +403,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements } /* Add moved/added connectors to bookkeeping */ - for (ConnectorIndexChange change : changes) { + for (DetailsConnectorChange change : changes) { Integer newIndex = change.getNewIndex(); ComponentConnector connector = (ComponentConnector) change .getConnector(); @@ -456,8 +457,11 @@ public class GridConnector extends AbstractHasComponentsConnector implements } public void responseReceived(int fetchId) { - boolean success = pendingFetches.remove(fetchId); - assert success : "Received a response with an unidentified fetch id"; + /* Ignore negative fetchIds (they're pushed, not fetched) */ + if (fetchId >= 0) { + boolean success = pendingFetches.remove(fetchId); + assert success : "Received a response with an unidentified fetch id"; + } } @Override @@ -607,18 +611,20 @@ public class GridConnector extends AbstractHasComponentsConnector implements @Override public void setDetailsConnectorChanges( - Set connectorChanges, int fetchId) { + Set connectorChanges, int fetchId) { customDetailsGenerator .setDetailsConnectorChanges(connectorChanges); // refresh moved/added details rows - for (ConnectorIndexChange change : connectorChanges) { - Integer newIndex = change.getNewIndex(); - if (newIndex != null) { - int index = newIndex.intValue(); - getWidget().setDetailsVisible(index, false); - getWidget().setDetailsVisible(index, true); + for (DetailsConnectorChange change : connectorChanges) { + Integer index = change.getNewIndex(); + if (index == null) { + index = change.getOldIndex(); } + + int i = index.intValue(); + getWidget().setDetailsVisible(i, false); + getWidget().setDetailsVisible(i, true); } detailsConnectorFetcher.responseReceived(fetchId); } diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index f4aaf798b7..8243782c4e 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -6387,12 +6387,13 @@ public class Grid extends ResizeComposite implements * see GridSpacerUpdater.init for implementation details. */ - if (visible && !isDetailsVisible(rowIndex)) { + boolean isVisible = isDetailsVisible(rowIndex); + if (visible && !isVisible) { escalator.getBody().setSpacer(rowIndex, DETAILS_ROW_INITIAL_HEIGHT); visibleDetails.add(rowIndexInteger); } - else if (!visible && isDetailsVisible(rowIndex)) { + else if (!visible && isVisible) { escalator.getBody().setSpacer(rowIndex, -1); visibleDetails.remove(rowIndexInteger); } diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index 620933c379..97d141cd6e 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -30,6 +30,7 @@ import java.util.logging.Logger; import com.google.gwt.thirdparty.guava.common.collect.BiMap; import com.google.gwt.thirdparty.guava.common.collect.HashBiMap; +import com.google.gwt.thirdparty.guava.common.collect.ImmutableSet; import com.vaadin.data.Container.Indexed; import com.vaadin.data.Container.Indexed.ItemAddEvent; import com.vaadin.data.Container.Indexed.ItemRemoveEvent; @@ -1214,4 +1215,10 @@ public class RpcDataProviderExtension extends AbstractExtension { public boolean isDetailsVisible(Object itemId) { return visibleDetails.contains(itemId); } + + public void refreshDetails() { + for (Object itemId : ImmutableSet.copyOf(visibleDetails)) { + detailComponentManager.refresh(itemId); + } + } } diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index da5cedd999..ec1dd45536 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -38,6 +38,7 @@ import java.util.logging.Logger; import com.google.gwt.thirdparty.guava.common.collect.BiMap; import com.google.gwt.thirdparty.guava.common.collect.HashBiMap; +import com.google.gwt.thirdparty.guava.common.collect.Maps; import com.google.gwt.thirdparty.guava.common.collect.Sets; import com.google.gwt.thirdparty.guava.common.collect.Sets.SetView; import com.vaadin.data.Container; @@ -77,7 +78,7 @@ import com.vaadin.server.KeyMapper; import com.vaadin.server.VaadinSession; import com.vaadin.shared.MouseEventDetails; import com.vaadin.shared.data.sort.SortDirection; -import com.vaadin.shared.ui.grid.ConnectorIndexChange; +import com.vaadin.shared.ui.grid.DetailsConnectorChange; import com.vaadin.shared.ui.grid.EditorClientRpc; import com.vaadin.shared.ui.grid.EditorServerRpc; import com.vaadin.shared.ui.grid.GridClientRpc; @@ -2840,7 +2841,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * {@link Collection#isEmpty() empty}, then this field is consistent * with the connector hierarchy. */ - private final Map visibleDetailsComponents = new HashMap(); + private final Map visibleDetailsComponents = Maps + .newHashMap(); /** A lookup map for which row contains which details component. */ private BiMap rowIndexToDetails = HashBiMap @@ -2863,7 +2865,13 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * could find out the same thing by taking out all the other components * and checking whether Grid is their parent or not. */ - private final Set unattachedComponents = new HashSet(); + private final Set unattachedComponents = Sets.newHashSet(); + + /** + * Keeps tabs on all the details that did not get a component during + * {@link #createDetails(Object, int)}. + */ + private final Map emptyDetails = Maps.newHashMap(); /** * Creates a details component by the request of the client side, with @@ -2887,14 +2895,14 @@ public class Grid extends AbstractComponent implements SelectionNotifier, assert itemId != null : "itemId was null"; Integer newRowIndex = Integer.valueOf(rowIndex); - assert !visibleDetailsComponents.containsKey(itemId) : "itemId already has a component. Should be destroyed first."; + assert !visibleDetailsComponents.containsKey(itemId) : "itemId " + + "already has a component. Should be destroyed first."; RowReference rowReference = new RowReference(Grid.this); rowReference.set(itemId); Component details = getDetailsGenerator().getDetails(rowReference); if (details != null) { - if (details.getParent() != null) { String generatorName = getDetailsGenerator().getClass() .getName(); @@ -2916,6 +2924,20 @@ public class Grid extends AbstractComponent implements SelectionNotifier, visibleDetailsComponents.put(itemId, details); rowIndexToDetails.put(newRowIndex, details); unattachedComponents.add(details); + + assert !emptyDetails.containsKey(itemId) : "Bookeeping thinks " + + "itemId is empty even though we just created a " + + "component for it (" + itemId + ")"; + } else { + assert !emptyDetails.containsKey(itemId) : "Bookkeeping has " + + "already itemId marked as empty (itemId: " + itemId + + ", old index: " + emptyDetails.get(itemId) + + ", new index: " + newRowIndex + ")"; + assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping" + + " already had another itemId for this empty index " + + "(index: " + newRowIndex + ", new itemId: " + itemId + + ")"; + emptyDetails.put(itemId, newRowIndex); } /* @@ -2934,6 +2956,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * the item id for which to destroy the details component */ public void destroyDetails(Object itemId) { + emptyDetails.remove(itemId); + Component removedComponent = visibleDetailsComponents .remove(itemId); if (removedComponent == null) { @@ -2972,8 +2996,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * * @return information on how the connectors have changed */ - Set getAndResetConnectorChanges() { - Set changes = new HashSet(); + Set getAndResetConnectorChanges() { + Set changes = new HashSet(); // populate diff with added/changed for (Entry entry : rowIndexToDetails.entrySet()) { @@ -2996,7 +3020,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } if (!SharedUtil.equals(oldIndex, newIndex)) { - changes.add(new ConnectorIndexChange(component, oldIndex, + changes.add(new DetailsConnectorChange(component, oldIndex, newIndex)); } } @@ -3008,7 +3032,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, Component component = entry.getValue(); Integer newIndex = rowIndexToDetails.inverse().get(component); if (newIndex == null) { - changes.add(new ConnectorIndexChange(null, oldIndex, null)); + changes.add(new DetailsConnectorChange(null, oldIndex, null)); } } @@ -3017,6 +3041,21 @@ public class Grid extends AbstractComponent implements SelectionNotifier, return changes; } + + public void refresh(Object itemId) { + Component component = visibleDetailsComponents.get(itemId); + Integer rowIndex = null; + if (component != null) { + rowIndex = rowIndexToDetails.inverse().get(component); + destroyDetails(itemId); + } else { + rowIndex = emptyDetails.remove(itemId); + } + + assert rowIndex != null : "Given itemId does not map to an existing detail row (" + + itemId + ")"; + createDetails(itemId, rowIndex.intValue()); + } } /** @@ -5374,7 +5413,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, this.detailsGenerator = detailsGenerator; - getLogger().warning("[[details]] update details on generator swap"); + datasourceExtension.refreshDetails(); + getRpcProxy(GridClientRpc.class).setDetailsConnectorChanges( + detailComponentManager.getAndResetConnectorChanges(), -1); } /** diff --git a/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java b/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java deleted file mode 100644 index 16be92007e..0000000000 --- a/shared/src/com/vaadin/shared/ui/grid/ConnectorIndexChange.java +++ /dev/null @@ -1,143 +0,0 @@ -/* - * Copyright 2000-2014 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 - * License for the specific language governing permissions and limitations under - * the License. - */ -package com.vaadin.shared.ui.grid; - -import java.io.Serializable; - -import com.vaadin.shared.Connector; - -/** - * A description of an indexing modification for a connector. This is used by - * Grid by internal bookkeeping updates. - * - * @since - * @author Vaadin Ltd - */ -public class ConnectorIndexChange implements Serializable { - - private Connector connector; - private Integer oldIndex; - private Integer newIndex; - - /** Create a new connector index change */ - public ConnectorIndexChange() { - } - - /** - * Convenience constructor for setting all the fields in one line. - *

- * Calling this constructor will also assert that the state of the pojo is - * consistent by internal assumptions. - * - * @param connector - * the changed connector - * @param oldIndex - * the old index - * @param newIndex - * the new index - */ - public ConnectorIndexChange(Connector connector, Integer oldIndex, - Integer newIndex) { - this.connector = connector; - this.oldIndex = oldIndex; - this.newIndex = newIndex; - - assert assertStateIsOk(); - } - - private boolean assertStateIsOk() { - assert (connector != null && newIndex != null) - || (connector == null && oldIndex != null && newIndex == null) : "connector: " - + nullityString(connector) - + ", oldIndex: " - + nullityString(oldIndex) - + ", newIndex: " - + nullityString(newIndex); - return true; - } - - private static String nullityString(Object object) { - return object == null ? "null" : "non-null"; - } - - /** - * Gets the old index for the connector. - *

- * If null, the connector is recently added. This means that - * {@link #getConnector()} is expected not to return null. - * - * @return the old index for the connector - */ - public Integer getOldIndex() { - assert assertStateIsOk(); - return oldIndex; - } - - /** - * Gets the new index for the connector. - *

- * If null, the connector should be removed. This means that - * {@link #getConnector()} is expected to return null as well. - * - * @return the new index for the connector - */ - public Integer getNewIndex() { - assert assertStateIsOk(); - return newIndex; - } - - /** - * Gets the changed connector. - * - * @return the changed connector. Might be null - */ - public Connector getConnector() { - assert assertStateIsOk(); - return connector; - } - - /** - * Sets the changed connector. - * - * @param connector - * the changed connector. May be null - */ - public void setConnector(Connector connector) { - this.connector = connector; - } - - /** - * Sets the old index - * - * @param oldIndex - * the old index. May be null if a new connector is - * being inserted - */ - public void setOldIndex(Integer oldIndex) { - this.oldIndex = oldIndex; - } - - /** - * Sets the new index - * - * @param newIndex - * the new index. May be null if a connector is - * being removed - */ - public void setNewIndex(Integer newIndex) { - this.newIndex = newIndex; - } -} diff --git a/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java new file mode 100644 index 0000000000..40f4541fb1 --- /dev/null +++ b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java @@ -0,0 +1,147 @@ +/* + * Copyright 2000-2014 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 + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.shared.ui.grid; + +import java.io.Serializable; + +import com.vaadin.shared.Connector; + +/** + * A description of an indexing modification for a connector. This is used by + * Grid by internal bookkeeping updates. + * + * @since + * @author Vaadin Ltd + */ +public class DetailsConnectorChange implements Serializable { + + private Connector connector; + private Integer oldIndex; + private Integer newIndex; + + /** Create a new connector index change */ + public DetailsConnectorChange() { + } + + /** + * Convenience constructor for setting all the fields in one line. + *

+ * Calling this constructor will also assert that the state of the pojo is + * consistent by internal assumptions. + * + * @param connector + * the changed connector + * @param oldIndex + * the old index + * @param newIndex + * the new index + */ + public DetailsConnectorChange(Connector connector, Integer oldIndex, + Integer newIndex) { + this.connector = connector; + this.oldIndex = oldIndex; + this.newIndex = newIndex; + + assert assertStateIsOk(); + } + + private boolean assertStateIsOk() { + boolean connectorAndNewIndexIsNotNull = connector != null + && newIndex != null; + boolean connectorAndNewIndexIsNullThenOldIndexIsSet = connector == null + && newIndex == null && oldIndex != null; + + assert (connectorAndNewIndexIsNotNull || connectorAndNewIndexIsNullThenOldIndexIsSet) : "connector: " + + nullityString(connector) + + ", oldIndex: " + + nullityString(oldIndex) + + ", newIndex: " + + nullityString(newIndex); + return true; + } + + private static String nullityString(Object object) { + return object == null ? "null" : "non-null"; + } + + /** + * Gets the old index for the connector. + *

+ * If null, the connector is recently added. This means that + * {@link #getConnector()} is expected not to return null. + * + * @return the old index for the connector + */ + public Integer getOldIndex() { + assert assertStateIsOk(); + return oldIndex; + } + + /** + * Gets the new index for the connector. + *

+ * If null, the connector should be removed. This means that + * {@link #getConnector()} is expected to return null as well. + * + * @return the new index for the connector + */ + public Integer getNewIndex() { + assert assertStateIsOk(); + return newIndex; + } + + /** + * Gets the changed connector. + * + * @return the changed connector. Might be null + */ + public Connector getConnector() { + assert assertStateIsOk(); + return connector; + } + + /** + * Sets the changed connector. + * + * @param connector + * the changed connector. May be null + */ + public void setConnector(Connector connector) { + this.connector = connector; + } + + /** + * Sets the old index + * + * @param oldIndex + * the old index. May be null if a new connector is + * being inserted + */ + public void setOldIndex(Integer oldIndex) { + this.oldIndex = oldIndex; + } + + /** + * Sets the new index + * + * @param newIndex + * the new index. May be null if a connector is + * being removed + */ + public void setNewIndex(Integer newIndex) { + this.newIndex = newIndex; + } +} diff --git a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java index 672c83ff53..98e7fac567 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java @@ -65,9 +65,10 @@ public interface GridClientRpc extends ClientRpc { * @param connectorChanges * the indexing changes of details connectors * @param fetchId - * the id of the request for fetching the changes + * the id of the request for fetching the changes. A negative + * number indicates a push (not requested by the client side) */ public void setDetailsConnectorChanges( - Set connectorChanges, int fetchId); + Set connectorChanges, int fetchId); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index 08f0d7d5d2..63fb903f53 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -47,10 +47,12 @@ import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; import com.vaadin.ui.Button.ClickListener; import com.vaadin.ui.Component; +import com.vaadin.ui.CssLayout; import com.vaadin.ui.Grid; import com.vaadin.ui.Grid.CellReference; import com.vaadin.ui.Grid.CellStyleGenerator; import com.vaadin.ui.Grid.Column; +import com.vaadin.ui.Grid.DetailsGenerator; import com.vaadin.ui.Grid.FooterCell; import com.vaadin.ui.Grid.HeaderCell; import com.vaadin.ui.Grid.HeaderRow; @@ -60,6 +62,7 @@ import com.vaadin.ui.Grid.RowStyleGenerator; import com.vaadin.ui.Grid.SelectionMode; import com.vaadin.ui.Grid.SelectionModel; import com.vaadin.ui.Label; +import com.vaadin.ui.Notification; import com.vaadin.ui.Panel; import com.vaadin.ui.renderers.DateRenderer; import com.vaadin.ui.renderers.HtmlRenderer; @@ -114,6 +117,53 @@ public class GridBasicFeatures extends AbstractComponentTest { private Panel detailsPanel; + private final DetailsGenerator detailedDetailsGenerator = new DetailsGenerator() { + @Override + public Component getDetails(final RowReference rowReference) { + CssLayout cssLayout = new CssLayout(); + cssLayout.setHeight("200px"); + cssLayout.setWidth("100%"); + + Item item = rowReference.getItem(); + for (Object propertyId : item.getItemPropertyIds()) { + Property prop = item.getItemProperty(propertyId); + String string = prop.getValue().toString(); + cssLayout.addComponent(new Label(string)); + } + + final int rowIndex = grid.getContainerDataSource().indexOfId( + rowReference.getItemId()); + ClickListener clickListener = new ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + Notification.show("You clicked on the " + + "button in the details for " + "row " + rowIndex); + } + }; + cssLayout.addComponent(new Button("Press me", clickListener)); + return cssLayout; + } + }; + + private final DetailsGenerator watchingDetailsGenerator = new DetailsGenerator() { + private int id = 0; + + @Override + public Component getDetails(RowReference rowReference) { + return new Label("You are watching item id " + + rowReference.getItemId() + " (" + (id++) + ")"); + } + }; + + private final DetailsGenerator hierarchicalDetailsGenerator = new DetailsGenerator() { + @Override + public Component getDetails(RowReference rowReference) { + detailsPanel = new Panel(); + detailsPanel.setContent(new Label("One")); + return detailsPanel; + } + }; + @Override @SuppressWarnings("unchecked") protected Grid constructComponent() { @@ -1059,40 +1109,32 @@ public class GridBasicFeatures extends AbstractComponentTest { } private void createDetailsActions() { - createClickAction("custom details generator", "Details", - new Command() { - @Override - public void execute(Grid c, Void value, Object data) { - grid.setDetailsGenerator(new Grid.DetailsGenerator() { - private int seq = 0; + Command swapDetailsGenerator = new Command() { + @Override + public void execute(Grid c, DetailsGenerator generator, Object data) { + grid.setDetailsGenerator(generator); + } + }; - @Override - public Component getDetails( - RowReference rowReference) { - return new Label("You are watching item id " - + rowReference.getItemId() + " (" - + (seq++) + ")"); - } - }); - } - }, null); - createClickAction("hierarchy details generator", "Details", - new Command() { - @Override - public void execute(Grid c, Void value, Object data) { - grid.setDetailsGenerator(new Grid.DetailsGenerator() { - @Override - public Component getDetails( - RowReference rowReference) { - detailsPanel = new Panel(); - detailsPanel.setContent(new Label("One")); - return detailsPanel; - } - }); - } - }, null); + Command openOrCloseItemId = new Command() { + @Override + @SuppressWarnings("boxing") + public void execute(Grid g, Boolean visible, Object itemId) { + g.setDetailsVisible(itemId, visible); + } + }; - createClickAction("change hierarchy in generator", "Details", + createCategory("Generators", "Details"); + createClickAction("NULL", "Generators", swapDetailsGenerator, + DetailsGenerator.NULL); + createClickAction("\"Watching\"", "Generators", swapDetailsGenerator, + watchingDetailsGenerator); + createClickAction("Detailed", "Generators", swapDetailsGenerator, + detailedDetailsGenerator); + createClickAction("Hierarchical", "Generators", swapDetailsGenerator, + hierarchicalDetailsGenerator); + + createClickAction("- Change Component", "Generators", new Command() { @Override public void execute(Grid c, Void value, Object data) { @@ -1105,7 +1147,7 @@ public class GridBasicFeatures extends AbstractComponentTest { } }, null); - createClickAction("toggle firstItemId", "Details", + createClickAction("Toggle firstItemId", "Details", new Command() { @Override public void execute(Grid g, Void value, Object data) { @@ -1117,26 +1159,11 @@ public class GridBasicFeatures extends AbstractComponentTest { } }, null); - createBooleanAction("firstItemId", "Details", false, - new Command() { - @Override - @SuppressWarnings("boxing") - public void execute(Grid g, Boolean visible, Object data) { - g.setDetailsVisible(g.getContainerDataSource() - .firstItemId(), visible); - } - }); + createBooleanAction("Open firstItemId", "Details", false, + openOrCloseItemId, ds.firstItemId()); - createBooleanAction("lastItemId-5", "Details", false, - new Command() { - @Override - @SuppressWarnings("boxing") - public void execute(Grid g, Boolean visible, Object data) { - Object fifthLastItemId = g.getContainerDataSource() - .getItemIds(ROWS - 6, 1).get(0); - g.setDetailsVisible(fifthLastItemId, visible); - } - }); + createBooleanAction("Open 995", "Details", false, openOrCloseItemId, + ds.getIdByIndex(995)); } @Override 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 e9b5b688d1..f3f58b002e 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 @@ -16,6 +16,7 @@ package com.vaadin.tests.components.grid.basicfeatures.server; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -27,7 +28,7 @@ import org.openqa.selenium.By; import org.openqa.selenium.NoSuchElementException; import com.vaadin.testbench.TestBenchElement; -import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures; +import com.vaadin.testbench.elements.NotificationElement; import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; public class GridDetailsServerTest extends GridBasicFeaturesTest { @@ -37,20 +38,21 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { * able to scroll that particular details row into view, making tests * awkward with two scroll commands back to back. */ - private static final int ALMOST_LAST_ITEM_INDEX = GridBasicFeatures.ROWS - 5; - private static final String[] ALMOST_LAST_ITEM_DETAILS = new String[] { - "Component", "Details", "lastItemId-5" }; - - private static final String[] FIRST_ITEM_DETAILS = new String[] { - "Component", "Details", "firstItemId" }; + private static final int ALMOST_LAST_INDEX = 995; + private static final String[] OPEN_ALMOST_LAST_ITEM_DETAILS = new String[] { + "Component", "Details", "Open " + ALMOST_LAST_INDEX }; + private static final String[] OPEN_FIRST_ITEM_DETAILS = new String[] { + "Component", "Details", "Open firstItemId" }; private static final String[] TOGGLE_FIRST_ITEM_DETAILS = new String[] { - "Component", "Details", "toggle firstItemId" }; - private static final String[] CUSTOM_DETAILS_GENERATOR = new String[] { - "Component", "Details", "custom details generator" }; - private static final String[] HIERARCHY_DETAILS_GENERATOR = new String[] { - "Component", "Details", "hierarchy details generator" }; + "Component", "Details", "Toggle firstItemId" }; + private static final String[] DETAILS_GENERATOR_NULL = new String[] { + "Component", "Details", "Generators", "NULL" }; + private static final String[] DETAILS_GENERATOR_WATCHING = new String[] { + "Component", "Details", "Generators", "\"Watching\"" }; + private static final String[] DETAILS_GENERATOR_HIERARCHICAL = new String[] { + "Component", "Details", "Generators", "Hierarchical" }; private static final String[] CHANGE_HIERARCHY = new String[] { - "Component", "Details", "change hierarchy in generator" }; + "Component", "Details", "Generators", "- Change Component" }; @Before public void setUp() { @@ -65,41 +67,42 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { } catch (NoSuchElementException ignore) { // expected } - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); assertNotNull("details should've opened", getGridElement() .getDetails(0)); } @Test(expected = NoSuchElementException.class) public void closeVisibleDetails() { - selectMenuPath(FIRST_ITEM_DETAILS); - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); getGridElement().getDetails(0); } @Test - public void openDetailsOutsideOfActiveRange() { + public void openDetailsOutsideOfActiveRange() throws InterruptedException { getGridElement().scroll(10000); - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); getGridElement().scroll(0); + Thread.sleep(50); assertNotNull("details should've been opened", getGridElement() .getDetails(0)); } @Test(expected = NoSuchElementException.class) public void closeDetailsOutsideOfActiveRange() { - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); getGridElement().scroll(10000); - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); getGridElement().scroll(0); getGridElement().getDetails(0); } @Test public void componentIsVisibleClientSide() { - selectMenuPath(CUSTOM_DETAILS_GENERATOR); - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); TestBenchElement details = getGridElement().getDetails(0); assertNotNull("No widget detected inside details", @@ -107,11 +110,11 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { } @Test - public void togglingAVisibleDetailsRowWithSeparateRoundtrips() { - selectMenuPath(CUSTOM_DETAILS_GENERATOR); - selectMenuPath(FIRST_ITEM_DETAILS); // open - selectMenuPath(FIRST_ITEM_DETAILS); // close - selectMenuPath(FIRST_ITEM_DETAILS); // open + public void openingDetailsTwice() { + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // open + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // close + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // open TestBenchElement details = getGridElement().getDetails(0); assertNotNull("No widget detected inside details", @@ -120,7 +123,7 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { @Test(expected = NoSuchElementException.class) public void scrollingDoesNotCreateAFloodOfDetailsRows() { - selectMenuPath(CUSTOM_DETAILS_GENERATOR); + selectMenuPath(DETAILS_GENERATOR_WATCHING); // scroll somewhere to hit uncached rows getGridElement().scrollToRow(101); @@ -133,8 +136,8 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { public void openingDetailsOutOfView() { getGridElement().scrollToRow(500); - selectMenuPath(CUSTOM_DETAILS_GENERATOR); - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); getGridElement().scrollToRow(0); @@ -145,8 +148,8 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { @Test public void togglingAVisibleDetailsRowWithOneRoundtrip() { - selectMenuPath(CUSTOM_DETAILS_GENERATOR); - selectMenuPath(FIRST_ITEM_DETAILS); // open + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); // open assertTrue("Unexpected generator content", getGridElement().getDetails(0).getText().endsWith("(0)")); @@ -156,36 +159,111 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { } @Test - @Ignore("This will be patched with https://dev.vaadin.com/review/#/c/7917/") public void almosLastItemIdIsRendered() { - selectMenuPath(CUSTOM_DETAILS_GENERATOR); - selectMenuPath(ALMOST_LAST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_ALMOST_LAST_ITEM_DETAILS); scrollGridVerticallyTo(100000); TestBenchElement details = getGridElement().getDetails( - ALMOST_LAST_ITEM_INDEX); + ALMOST_LAST_INDEX); assertNotNull(details); assertTrue("Unexpected details content", - details.getText().endsWith(ALMOST_LAST_ITEM_INDEX + " (0)")); + details.getText().endsWith(ALMOST_LAST_INDEX + " (0)")); } @Test public void hierarchyChangesWorkInDetails() { - selectMenuPath(HIERARCHY_DETAILS_GENERATOR); - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); assertEquals("One", getGridElement().getDetails(0).getText()); selectMenuPath(CHANGE_HIERARCHY); assertEquals("Two", getGridElement().getDetails(0).getText()); } + @Ignore("This use case is not currently supported by Grid. If the detail " + + "is out of view, the component is detached from the UI and a " + + "new instance is generated when scrolled back. Support will " + + "maybe be incorporated at a later time") @Test - @Ignore("This will be patched with https://dev.vaadin.com/review/#/c/7917/") public void hierarchyChangesWorkInDetailsWhileOutOfView() { - selectMenuPath(HIERARCHY_DETAILS_GENERATOR); - selectMenuPath(FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); scrollGridVerticallyTo(10000); selectMenuPath(CHANGE_HIERARCHY); scrollGridVerticallyTo(0); assertEquals("Two", getGridElement().getDetails(0).getText()); } + + @Test + public void swappingDetailsGenerators_noDetailsShown() { + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(DETAILS_GENERATOR_NULL); + assertFalse("Got some errors", $(NotificationElement.class).exists()); + } + + @Test + public void swappingDetailsGenerators_shownDetails() { + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + assertTrue("Details should be empty at the start", getGridElement() + .getDetails(0).getText().isEmpty()); + + selectMenuPath(DETAILS_GENERATOR_WATCHING); + assertFalse("Details should not be empty after swapping generator", + getGridElement().getDetails(0).getText().isEmpty()); + } + + @Test + public void swappingDetailsGenerators_whileDetailsScrolledOut_showNever() { + scrollGridVerticallyTo(1000); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + assertFalse("Got some errors", $(NotificationElement.class).exists()); + } + + @Test + public void swappingDetailsGenerators_whileDetailsScrolledOut_showAfter() { + scrollGridVerticallyTo(1000); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + scrollGridVerticallyTo(0); + + assertFalse("Got some errors", $(NotificationElement.class).exists()); + assertNotNull("Could not find a details", getGridElement() + .getDetails(0)); + } + + @Test + public void swappingDetailsGenerators_whileDetailsScrolledOut_showBefore() { + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + scrollGridVerticallyTo(1000); + + assertFalse("Got some errors", $(NotificationElement.class).exists()); + assertNotNull("Could not find a details", getGridElement() + .getDetails(0)); + } + + @Test + public void swappingDetailsGenerators_whileDetailsScrolledOut_showBeforeAndAfter() { + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + scrollGridVerticallyTo(1000); + scrollGridVerticallyTo(0); + + assertFalse("Got some errors", $(NotificationElement.class).exists()); + assertNotNull("Could not find a details", getGridElement() + .getDetails(0)); + } + + @Test + public void nullDetailComponentToggling() { + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(DETAILS_GENERATOR_NULL); + assertTrue("Details should be empty with null component", + getGridElement().getDetails(0).getText().isEmpty()); + selectMenuPath(DETAILS_GENERATOR_WATCHING); + assertFalse("Details should be not empty with details component", + getGridElement().getDetails(0).getText().isEmpty()); + } } -- cgit v1.2.3 From 2be1e43d7081f0bc2c5f905d6b007fe597934ae3 Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Wed, 18 Mar 2015 10:05:47 +0200 Subject: Adds server side column hiding API to Grid (#17023) Change-Id: Ic00e873176f499dfc45976439e09d712932775da --- .../vaadin/client/connectors/GridConnector.java | 4 + server/src/com/vaadin/ui/Grid.java | 159 +++++++++++++++++++++ .../com/vaadin/shared/ui/grid/GridColumnState.java | 6 + .../grid/basicfeatures/GridBasicFeatures.java | 43 +++++- .../server/GridColumnVisibilityTest.java | 89 ++++++++++++ 5 files changed, 300 insertions(+), 1 deletion(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 495bcd0411..d3045ee13b 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -824,6 +824,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements column.setExpandRatio(state.expandRatio); column.setSortable(state.sortable); + + column.setHidden(state.hidden); + column.setHideable(state.hidable); + column.setEditable(state.editable); column.setEditorConnector((AbstractFieldConnector) state.editorConnector); } diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 12722b2596..31a25d8f8f 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -166,6 +166,67 @@ import elemental.json.JsonValue; public class Grid extends AbstractComponent implements SelectionNotifier, SortNotifier, SelectiveRenderer, ItemClickNotifier { + /** + * An event listener for column visibility change events in the Grid. + * + * @since + */ + public interface ColumnVisibilityChangeListener extends Serializable { + /** + * Called when a column has become hidden or unhidden. + * + * @param event + */ + void columnVisibilityChanged(ColumnVisibilityChangeEvent event); + } + + /** + * An event that is fired when a column becomes hidden or unhidden. + * + * @since + */ + public static class ColumnVisibilityChangeEvent extends Component.Event { + + private final Column column; + private final boolean userOriginated; + + /** + * Constructor for a column visibility change event. + * + * @param column + * the column that changed its visibility + * @param isUserOriginated + * true iff the event was triggered by an UI + * interaction + */ + public ColumnVisibilityChangeEvent(Component source, Column column, + boolean isUserOriginated) { + super(source); + this.column = column; + userOriginated = isUserOriginated; + } + + /** + * Gets the column that became hidden or unhidden. + * + * @return the column that became hidden or unhidden. + * @see Column#isHidden() + */ + public Column getColumn() { + return column; + } + + /** + * Returns true if the column reorder was done by the user, + * false if not and it was triggered by server side code. + * + * @return true if event is a result of user interaction + */ + public boolean isUserOriginated() { + return userOriginated; + } + } + /** * Custom field group that allows finding property types before an item has * been bound. @@ -2715,6 +2776,66 @@ public class Grid extends AbstractComponent implements SelectionNotifier, public Field getEditorField() { return grid.getEditorField(getPropertyId()); } + + /** + * Hides or shows the column. By default columns are visible before + * explicitly hiding them. + * + * @since + * @param hidden + * true to hide the column, false + * to show + */ + public void setHidden(boolean hidden) { + if (hidden != getState().hidden) { + getState().hidden = hidden; + grid.markAsDirty(); + grid.fireColumnVisibilityChangeEvent(this, false); + } + } + + /** + * Is this column hidden. Default is {@code false}. + * + * @since + * @return true if the column is currently hidden, + * false otherwise + */ + public boolean isHidden() { + return getState().hidden; + } + + /** + * Set whether it is possible for the user to hide this column or not. + * Default is {@code false}. + *

+ * Note: it is still possible to hide the column + * programmatically using {@link #setHidden(boolean)} + * + * @since + * @param hidable + * true iff the column may be hidable by the + * user via UI interaction + */ + public void setHidable(boolean hidable) { + getState().hidable = hidable; + grid.markAsDirty(); + } + + /** + * Is it possible for the the user to hide this column. Default is + * {@code false}. + *

+ * Note: the column can be programmatically hidden using + * {@link #setHidden(boolean)} regardless of the returned value. + * + * @since + * @return true if the user can hide the column, + * false if not + */ + public boolean isHidable() { + return getState().hidable; + } } /** @@ -2952,6 +3073,11 @@ public class Grid extends AbstractComponent implements SelectionNotifier, .findMethod(ColumnReorderListener.class, "columnReorder", ColumnReorderEvent.class); + private static final Method COLUMN_VISIBILITY_METHOD = ReflectTools + .findMethod(ColumnVisibilityChangeListener.class, + "columnVisibilityChanged", + ColumnVisibilityChangeEvent.class); + /** * Creates a new Grid with a new {@link IndexedContainer} as the data * source. @@ -5242,4 +5368,37 @@ public class Grid extends AbstractComponent implements SelectionNotifier, public void recalculateColumnWidths() { getRpcProxy(GridClientRpc.class).recalculateColumnWidths(); } + + /** + * Registers a new column visibility change listener + * + * @since + * @param listener + * the listener to register + */ + public void addColumnVisibilityChangeListener( + ColumnVisibilityChangeListener listener) { + addListener(ColumnVisibilityChangeEvent.class, listener, + COLUMN_VISIBILITY_METHOD); + } + + /** + * Removes a previously registered column visibility change listener + * + * @since + * @param listener + * the listener to remove + */ + public void removeColumnVisibilityChangeListener( + ColumnVisibilityChangeListener listener) { + removeListener(ColumnVisibilityChangeEvent.class, listener, + COLUMN_VISIBILITY_METHOD); + } + + private void fireColumnVisibilityChangeEvent(Column column, + boolean isUserOriginated) { + fireEvent(new ColumnVisibilityChangeEvent(this, column, + isUserOriginated)); + } + } diff --git a/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java b/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java index 4c5b2c3a02..b966c53352 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java @@ -76,4 +76,10 @@ public class GridColumnState implements Serializable { * minWidth is less than the calculated width, minWidth will win. */ public double minWidth = GridConstants.DEFAULT_MIN_WIDTH; + + /** Is the column currently hidden. */ + public boolean hidden = false; + + /** Can the column be hidden by the UI. */ + public boolean hidable = false; } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index e9f6bfdbb9..aeeaa25ac3 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -52,6 +52,8 @@ import com.vaadin.ui.Grid.CellStyleGenerator; import com.vaadin.ui.Grid.Column; import com.vaadin.ui.Grid.ColumnReorderEvent; import com.vaadin.ui.Grid.ColumnReorderListener; +import com.vaadin.ui.Grid.ColumnVisibilityChangeEvent; +import com.vaadin.ui.Grid.ColumnVisibilityChangeListener; import com.vaadin.ui.Grid.FooterCell; import com.vaadin.ui.Grid.HeaderCell; import com.vaadin.ui.Grid.HeaderRow; @@ -120,6 +122,16 @@ public class GridBasicFeatures extends AbstractComponentTest { } }; + private ColumnVisibilityChangeListener columnVisibilityListener = new ColumnVisibilityChangeListener() { + @Override + public void columnVisibilityChanged(ColumnVisibilityChangeEvent event) { + log("Visibility changed: "// + + "propertyId: " + event.getColumn().getPropertyId() // + + ", isHidden: " + event.getColumn().isHidden() // + + ", userOriginated: " + event.isUserOriginated()); + } + }; + @Override @SuppressWarnings("unchecked") protected Grid constructComponent() { @@ -532,6 +544,17 @@ public class GridBasicFeatures extends AbstractComponentTest { } } }); + createBooleanAction("ColumnVisibilityChangeListener", "State", false, + new Command() { + @Override + public void execute(Grid grid, Boolean value, Object data) { + if (value) { + grid.addColumnVisibilityChangeListener(columnVisibilityListener); + } else { + grid.removeColumnVisibilityChangeListener(columnVisibilityListener); + } + } + }); createBooleanAction("Single select allow deselect", "State", singleSelectAllowDeselect, new Command() { @@ -676,6 +699,7 @@ public class GridBasicFeatures extends AbstractComponentTest { }, null); } + @SuppressWarnings("boxing") protected void createColumnActions() { createCategory("Columns", null); @@ -736,6 +760,24 @@ public class GridBasicFeatures extends AbstractComponentTest { } }, c); + createBooleanAction("Hidable", getColumnProperty(c), false, + new Command() { + @Override + public void execute(Grid c, Boolean hidable, + Object propertyId) { + grid.getColumn(propertyId).setHidable(hidable); + } + }, getColumnProperty(c)); + + createBooleanAction("Hidden", getColumnProperty(c), false, + new Command() { + @Override + public void execute(Grid c, Boolean hidden, + Object propertyId) { + grid.getColumn(propertyId).setHidden(hidden); + } + }, getColumnProperty(c)); + createCategory("Column " + c + " Width", getColumnProperty(c)); createClickAction("Auto", "Column " + c + " Width", @@ -753,7 +795,6 @@ public class GridBasicFeatures extends AbstractComponentTest { createClickAction("25.5px", "Column " + c + " Width", new Command() { @Override - @SuppressWarnings("boxing") public void execute(Grid grid, Void value, Object columnIndex) { grid.getColumns().get((Integer) columnIndex) diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java new file mode 100644 index 0000000000..8fb733dfa0 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java @@ -0,0 +1,89 @@ +/* + * Copyright 2000-2014 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 + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.grid.basicfeatures.server; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Before; +import org.junit.Test; + +import com.vaadin.testbench.annotations.RunLocally; +import com.vaadin.testbench.parallel.Browser; +import com.vaadin.testbench.parallel.TestCategory; +import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; + +@TestCategory("grid") +@RunLocally(Browser.PHANTOMJS) +public class GridColumnVisibilityTest extends GridBasicFeaturesTest { + + private static final String[] TOGGLE_LISTENER = new String[] { "Component", + "State", "ColumnVisibilityChangeListener" }; + private static final String[] TOGGLE_HIDE_COLUMN_0 = new String[] { + "Component", "Columns", "Column 0", "Hidden" }; + + private static final String COLUMN_0_BECAME_HIDDEN_MSG = "Visibility " + + "changed: propertyId: Column 0, isHidden: true"; + private static final String COLUMN_0_BECAME_UNHIDDEN_MSG = "Visibility " + + "changed: propertyId: Column 0, isHidden: false"; + + @Before + public void setUp() { + openTestURL(); + } + + @Test + public void columnIsNotShownWhenHidden() { + assertEquals("column 0", getGridElement().getHeaderCell(0, 0).getText() + .toLowerCase()); + + selectMenuPath(TOGGLE_HIDE_COLUMN_0); + assertEquals("column 1", getGridElement().getHeaderCell(0, 0).getText() + .toLowerCase()); + } + + @Test + public void columnIsShownWhenUnhidden() { + selectMenuPath(TOGGLE_HIDE_COLUMN_0); + selectMenuPath(TOGGLE_HIDE_COLUMN_0); + assertEquals("column 0", getGridElement().getHeaderCell(0, 0).getText() + .toLowerCase()); + } + + @Test + public void registeringListener() { + assertFalse(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG)); + selectMenuPath(TOGGLE_LISTENER); + assertFalse(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG)); + + selectMenuPath(TOGGLE_HIDE_COLUMN_0); + assertTrue(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG)); + + selectMenuPath(TOGGLE_HIDE_COLUMN_0); + assertTrue(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG)); + } + + @Test + public void deregisteringListener() { + selectMenuPath(TOGGLE_LISTENER); + selectMenuPath(TOGGLE_HIDE_COLUMN_0); + + selectMenuPath(TOGGLE_LISTENER); + selectMenuPath(TOGGLE_HIDE_COLUMN_0); + assertFalse(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG)); + } +} -- cgit v1.2.3 From c77b94560588320b2b0b746c17b9240f433475dc Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Thu, 19 Mar 2015 17:59:47 +0200 Subject: Added some extra warnings for DetailsGenerator (#16644) Change-Id: Ie84d1aa3d8738d5988567331368f217abf104dd6 --- server/src/com/vaadin/ui/Grid.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 396e3c5a77..0e035ae524 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -254,7 +254,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * Note: If a component gets generated, it may not be manually * attached anywhere, nor may it be a reused instance – each * invocation of this method should produce a unique and isolated - * component instance. + * component instance. Essentially, this should mostly be a + * self-contained fire-and-forget method, as external references to the + * generated component might cause unexpected behavior. * * @param rowReference * the reference for the row for which to generate details -- cgit v1.2.3 From 4a52ec0b7ad95a4b44a70be9ce9386f6cde1192b Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Fri, 20 Mar 2015 11:18:35 +0200 Subject: Move DetailComponentManager from Grid to RDPE (#16644) Change-Id: I2b65a878bb50c2b1f62135a998207a41e82fe62f --- .../com/vaadin/data/RpcDataProviderExtension.java | 269 ++++++++++++++++++++- server/src/com/vaadin/ui/Grid.java | 253 +------------------ .../component/grid/DataProviderExtension.java | 2 +- 3 files changed, 271 insertions(+), 253 deletions(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index 8d7b654468..66c17c4afa 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -25,12 +25,15 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.logging.Logger; import com.google.gwt.thirdparty.guava.common.collect.BiMap; import com.google.gwt.thirdparty.guava.common.collect.HashBiMap; import com.google.gwt.thirdparty.guava.common.collect.ImmutableSet; +import com.google.gwt.thirdparty.guava.common.collect.Maps; +import com.google.gwt.thirdparty.guava.common.collect.Sets; import com.vaadin.data.Container.Indexed; import com.vaadin.data.Container.Indexed.ItemAddEvent; import com.vaadin.data.Container.Indexed.ItemRemoveEvent; @@ -46,13 +49,16 @@ import com.vaadin.server.ClientConnector; import com.vaadin.server.KeyMapper; import com.vaadin.shared.data.DataProviderRpc; import com.vaadin.shared.data.DataRequestRpc; +import com.vaadin.shared.ui.grid.DetailsConnectorChange; import com.vaadin.shared.ui.grid.GridState; import com.vaadin.shared.ui.grid.Range; +import com.vaadin.shared.util.SharedUtil; +import com.vaadin.ui.Component; import com.vaadin.ui.Grid; import com.vaadin.ui.Grid.CellReference; import com.vaadin.ui.Grid.CellStyleGenerator; import com.vaadin.ui.Grid.Column; -import com.vaadin.ui.Grid.DetailComponentManager; +import com.vaadin.ui.Grid.DetailsGenerator; import com.vaadin.ui.Grid.RowReference; import com.vaadin.ui.Grid.RowStyleGenerator; import com.vaadin.ui.renderers.Renderer; @@ -578,6 +584,253 @@ public class RpcDataProviderExtension extends AbstractExtension { } } + /** + * A class that makes detail component related internal communication + * possible between {@link RpcDataProviderExtension} and grid. + * + * @since + * @author Vaadin Ltd + */ + public static final class DetailComponentManager implements Serializable { + /** + * This map represents all the components that have been requested for + * each item id. + *

+ * Normally this map is consistent with what is displayed in the + * component hierarchy (and thus the DOM). The only time this map is out + * of sync with the DOM is between the any calls to + * {@link #createDetails(Object, int)} or + * {@link #destroyDetails(Object)}, and + * {@link GridClientRpc#setDetailsConnectorChanges(Set)}. + *

+ * This is easily checked: if {@link #unattachedComponents} is + * {@link Collection#isEmpty() empty}, then this field is consistent + * with the connector hierarchy. + */ + private final Map visibleDetailsComponents = Maps + .newHashMap(); + + /** A lookup map for which row contains which details component. */ + private BiMap rowIndexToDetails = HashBiMap + .create(); + + /** + * A copy of {@link #rowIndexToDetails} from its last stable state. Used + * for creating a diff against {@link #rowIndexToDetails}. + * + * @see #getAndResetConnectorChanges() + */ + private BiMap prevRowIndexToDetails = HashBiMap + .create(); + + /** + * A set keeping track on components that have been created, but not + * attached. They should be attached at some later point in time. + *

+ * This isn't strictly requried, but it's a handy explicit log. You + * could find out the same thing by taking out all the other components + * and checking whether Grid is their parent or not. + */ + private final Set unattachedComponents = Sets.newHashSet(); + + /** + * Keeps tabs on all the details that did not get a component during + * {@link #createDetails(Object, int)}. + */ + private final Map emptyDetails = Maps.newHashMap(); + + private Grid grid; + + /** + * Creates a details component by the request of the client side, with + * the help of the user-defined {@link DetailsGenerator}. + *

+ * Also keeps internal bookkeeping up to date. + * + * @param itemId + * the item id for which to create the details component. + * Assumed not null and that a component is not + * currently present for this item previously + * @param rowIndex + * the row index for {@code itemId} + * @throws IllegalStateException + * if the current details generator provides a component + * that was manually attached, or if the same instance has + * already been provided + */ + public void createDetails(Object itemId, int rowIndex) + throws IllegalStateException { + assert itemId != null : "itemId was null"; + Integer newRowIndex = Integer.valueOf(rowIndex); + + assert !visibleDetailsComponents.containsKey(itemId) : "itemId " + + "already has a component. Should be destroyed first."; + + RowReference rowReference = new RowReference(grid); + rowReference.set(itemId); + + DetailsGenerator detailsGenerator = grid.getDetailsGenerator(); + Component details = detailsGenerator.getDetails(rowReference); + if (details != null) { + String generatorName = detailsGenerator.getClass().getName(); + if (details.getParent() != null) { + throw new IllegalStateException(generatorName + + " generated a details component that already " + + "was attached. (itemId: " + itemId + ", row: " + + rowIndex + ", component: " + details); + } + + if (rowIndexToDetails.containsValue(details)) { + throw new IllegalStateException(generatorName + + " provided a details component that already " + + "exists in Grid. (itemId: " + itemId + ", row: " + + rowIndex + ", component: " + details); + } + + visibleDetailsComponents.put(itemId, details); + rowIndexToDetails.put(newRowIndex, details); + unattachedComponents.add(details); + + assert !emptyDetails.containsKey(itemId) : "Bookeeping thinks " + + "itemId is empty even though we just created a " + + "component for it (" + itemId + ")"; + } else { + assert !emptyDetails.containsKey(itemId) : "Bookkeeping has " + + "already itemId marked as empty (itemId: " + itemId + + ", old index: " + emptyDetails.get(itemId) + + ", new index: " + newRowIndex + ")"; + assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping" + + " already had another itemId for this empty index " + + "(index: " + newRowIndex + ", new itemId: " + itemId + + ")"; + emptyDetails.put(itemId, newRowIndex); + } + + /* + * Don't attach the components here. It's done by + * GridServerRpc.sendDetailsComponents in a separate roundtrip. + */ + } + + /** + * Destroys correctly a details component, by the request of the client + * side. + *

+ * Also keeps internal bookkeeping up to date. + * + * @param itemId + * the item id for which to destroy the details component + */ + public void destroyDetails(Object itemId) { + emptyDetails.remove(itemId); + + Component removedComponent = visibleDetailsComponents + .remove(itemId); + if (removedComponent == null) { + return; + } + + rowIndexToDetails.inverse().remove(removedComponent); + + removedComponent.setParent(null); + grid.markAsDirty(); + } + + /** + * Gets all details components that are currently attached to the grid. + *

+ * Used internally by the Grid object. + * + * @return all details components that are currently attached to the + * grid + */ + public Collection getComponents() { + Set components = new HashSet( + visibleDetailsComponents.values()); + components.removeAll(unattachedComponents); + return components; + } + + /** + * Gets information on how the connectors have changed. + *

+ * This method only returns the changes that have been made between two + * calls of this method. I.e. Calling this method once will reset the + * state for the next state. + *

+ * Used internally by the Grid object. + * + * @return information on how the connectors have changed + */ + public Set getAndResetConnectorChanges() { + Set changes = new HashSet(); + + // populate diff with added/changed + for (Entry entry : rowIndexToDetails.entrySet()) { + Component component = entry.getValue(); + assert component != null : "rowIndexToDetails contains a null component"; + + Integer newIndex = entry.getKey(); + Integer oldIndex = prevRowIndexToDetails.inverse().get( + component); + + /* + * only attach components. Detaching already happened in + * destroyDetails. + */ + if (newIndex != null && oldIndex == null) { + assert unattachedComponents.contains(component) : "unattachedComponents does not contain component for index " + + newIndex + " (" + component + ")"; + component.setParent(grid); + unattachedComponents.remove(component); + } + + if (!SharedUtil.equals(oldIndex, newIndex)) { + changes.add(new DetailsConnectorChange(component, oldIndex, + newIndex)); + } + } + + // populate diff with removed + for (Entry entry : prevRowIndexToDetails + .entrySet()) { + Integer oldIndex = entry.getKey(); + Component component = entry.getValue(); + Integer newIndex = rowIndexToDetails.inverse().get(component); + if (newIndex == null) { + changes.add(new DetailsConnectorChange(null, oldIndex, null)); + } + } + + // reset diff map + prevRowIndexToDetails = HashBiMap.create(rowIndexToDetails); + + return changes; + } + + public void refresh(Object itemId) { + Component component = visibleDetailsComponents.get(itemId); + Integer rowIndex = null; + if (component != null) { + rowIndex = rowIndexToDetails.inverse().get(component); + destroyDetails(itemId); + } else { + rowIndex = emptyDetails.remove(itemId); + } + + assert rowIndex != null : "Given itemId does not map to an " + + "existing detail row (" + itemId + ")"; + createDetails(itemId, rowIndex.intValue()); + } + + void setGrid(Grid grid) { + if (this.grid != null) { + throw new IllegalStateException("Grid may injected only once."); + } + this.grid = grid; + } + } + private final Indexed container; private final ActiveRowHandler activeRowHandler = new ActiveRowHandler(); @@ -685,7 +938,7 @@ public class RpcDataProviderExtension extends AbstractExtension { */ private Set visibleDetails = new HashSet(); - private DetailComponentManager detailComponentManager; + private final DetailComponentManager detailComponentManager = new DetailComponentManager(); /** * Creates a new data provider using the given container. @@ -693,10 +946,8 @@ public class RpcDataProviderExtension extends AbstractExtension { * @param container * the container to make available */ - public RpcDataProviderExtension(Indexed container, - DetailComponentManager detailComponentManager) { + public RpcDataProviderExtension(Indexed container) { this.container = container; - this.detailComponentManager = detailComponentManager; rpc = getRpcProxy(DataProviderRpc.class); registerRpc(new DataRequestRpc() { @@ -884,9 +1135,12 @@ public class RpcDataProviderExtension extends AbstractExtension { * * @param component * the remote data grid component to extend + * @param columnKeys + * the key mapper for columns */ public void extend(Grid component, KeyMapper columnKeys) { this.columnKeys = columnKeys; + detailComponentManager.setGrid(component); super.extend(component); } @@ -1171,4 +1425,9 @@ public class RpcDataProviderExtension extends AbstractExtension { */ return container.indexOfId(itemId); } + + /** Gets the detail component manager for this data provider */ + public DetailComponentManager getDetailComponentManager() { + return detailComponentManager; + } } diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 0e035ae524..4488789406 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -37,9 +37,6 @@ import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; -import com.google.gwt.thirdparty.guava.common.collect.BiMap; -import com.google.gwt.thirdparty.guava.common.collect.HashBiMap; -import com.google.gwt.thirdparty.guava.common.collect.Maps; import com.google.gwt.thirdparty.guava.common.collect.Sets; import com.google.gwt.thirdparty.guava.common.collect.Sets.SetView; import com.vaadin.data.Container; @@ -52,6 +49,7 @@ import com.vaadin.data.Item; import com.vaadin.data.Property; import com.vaadin.data.RpcDataProviderExtension; import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper; +import com.vaadin.data.RpcDataProviderExtension.DetailComponentManager; import com.vaadin.data.Validator.InvalidValueException; import com.vaadin.data.fieldgroup.DefaultFieldGroupFieldFactory; import com.vaadin.data.fieldgroup.FieldGroup; @@ -80,7 +78,6 @@ import com.vaadin.server.KeyMapper; import com.vaadin.server.VaadinSession; import com.vaadin.shared.MouseEventDetails; import com.vaadin.shared.data.sort.SortDirection; -import com.vaadin.shared.ui.grid.DetailsConnectorChange; import com.vaadin.shared.ui.grid.EditorClientRpc; import com.vaadin.shared.ui.grid.EditorServerRpc; import com.vaadin.shared.ui.grid.GridClientRpc; @@ -2995,246 +2992,6 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } } - /** - * A class that makes detail component related internal communication - * possible between {@link RpcDataProviderExtension} and grid. - * - * @since - * @author Vaadin Ltd - */ - public final class DetailComponentManager implements Serializable { - /** - * This map represents all the components that have been requested for - * each item id. - *

- * Normally this map is consistent with what is displayed in the - * component hierarchy (and thus the DOM). The only time this map is out - * of sync with the DOM is between the any calls to - * {@link #createDetails(Object, int)} or - * {@link #destroyDetails(Object)}, and - * {@link GridClientRpc#setDetailsConnectorChanges(Set)}. - *

- * This is easily checked: if {@link #unattachedComponents} is - * {@link Collection#isEmpty() empty}, then this field is consistent - * with the connector hierarchy. - */ - private final Map visibleDetailsComponents = Maps - .newHashMap(); - - /** A lookup map for which row contains which details component. */ - private BiMap rowIndexToDetails = HashBiMap - .create(); - - /** - * A copy of {@link #rowIndexToDetails} from its last stable state. Used - * for creating a diff against {@link #rowIndexToDetails}. - * - * @see #getAndResetConnectorChanges() - */ - private BiMap prevRowIndexToDetails = HashBiMap - .create(); - - /** - * A set keeping track on components that have been created, but not - * attached. They should be attached at some later point in time. - *

- * This isn't strictly requried, but it's a handy explicit log. You - * could find out the same thing by taking out all the other components - * and checking whether Grid is their parent or not. - */ - private final Set unattachedComponents = Sets.newHashSet(); - - /** - * Keeps tabs on all the details that did not get a component during - * {@link #createDetails(Object, int)}. - */ - private final Map emptyDetails = Maps.newHashMap(); - - /** - * Creates a details component by the request of the client side, with - * the help of the user-defined {@link DetailsGenerator}. - *

- * Also keeps internal bookkeeping up to date. - * - * @param itemId - * the item id for which to create the details component. - * Assumed not null and that a component is not - * currently present for this item previously - * @param rowIndex - * the row index for {@code itemId} - * @throws IllegalStateException - * if the current details generator provides a component - * that was manually attached, or if the same instance has - * already been provided - */ - public void createDetails(Object itemId, int rowIndex) - throws IllegalStateException { - assert itemId != null : "itemId was null"; - Integer newRowIndex = Integer.valueOf(rowIndex); - - assert !visibleDetailsComponents.containsKey(itemId) : "itemId " - + "already has a component. Should be destroyed first."; - - RowReference rowReference = new RowReference(Grid.this); - rowReference.set(itemId); - - Component details = getDetailsGenerator().getDetails(rowReference); - if (details != null) { - if (details.getParent() != null) { - String generatorName = getDetailsGenerator().getClass() - .getName(); - throw new IllegalStateException(generatorName - + " generated a details component that already " - + "was attached. (itemId: " + itemId + ", row: " - + rowIndex + ", component: " + details); - } - - if (rowIndexToDetails.containsValue(details)) { - String generatorName = getDetailsGenerator().getClass() - .getName(); - throw new IllegalStateException(generatorName - + " provided a details component that already " - + "exists in Grid. (itemId: " + itemId + ", row: " - + rowIndex + ", component: " + details); - } - - visibleDetailsComponents.put(itemId, details); - rowIndexToDetails.put(newRowIndex, details); - unattachedComponents.add(details); - - assert !emptyDetails.containsKey(itemId) : "Bookeeping thinks " - + "itemId is empty even though we just created a " - + "component for it (" + itemId + ")"; - } else { - assert !emptyDetails.containsKey(itemId) : "Bookkeeping has " - + "already itemId marked as empty (itemId: " + itemId - + ", old index: " + emptyDetails.get(itemId) - + ", new index: " + newRowIndex + ")"; - assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping" - + " already had another itemId for this empty index " - + "(index: " + newRowIndex + ", new itemId: " + itemId - + ")"; - emptyDetails.put(itemId, newRowIndex); - } - - /* - * Don't attach the components here. It's done by - * GridServerRpc.sendDetailsComponents in a separate roundtrip. - */ - } - - /** - * Destroys correctly a details component, by the request of the client - * side. - *

- * Also keeps internal bookkeeping up to date. - * - * @param itemId - * the item id for which to destroy the details component - */ - public void destroyDetails(Object itemId) { - emptyDetails.remove(itemId); - - Component removedComponent = visibleDetailsComponents - .remove(itemId); - if (removedComponent == null) { - return; - } - - rowIndexToDetails.inverse().remove(removedComponent); - - removedComponent.setParent(null); - markAsDirty(); - } - - /** - * Gets all details components that are currently attached to the grid. - *

- * Used internally by the Grid object. - * - * @return all details components that are currently attached to the - * grid - */ - Collection getComponents() { - Set components = new HashSet( - visibleDetailsComponents.values()); - components.removeAll(unattachedComponents); - return components; - } - - /** - * Gets information on how the connectors have changed. - *

- * This method only returns the changes that have been made between two - * calls of this method. I.e. Calling this method once will reset the - * state for the next state. - *

- * Used internally by the Grid object. - * - * @return information on how the connectors have changed - */ - Set getAndResetConnectorChanges() { - Set changes = new HashSet(); - - // populate diff with added/changed - for (Entry entry : rowIndexToDetails.entrySet()) { - Component component = entry.getValue(); - assert component != null : "rowIndexToDetails contains a null component"; - - Integer newIndex = entry.getKey(); - Integer oldIndex = prevRowIndexToDetails.inverse().get( - component); - - /* - * only attach components. Detaching already happened in - * destroyDetails. - */ - if (newIndex != null && oldIndex == null) { - assert unattachedComponents.contains(component) : "unattachedComponents does not contain component for index " - + newIndex + " (" + component + ")"; - component.setParent(Grid.this); - unattachedComponents.remove(component); - } - - if (!SharedUtil.equals(oldIndex, newIndex)) { - changes.add(new DetailsConnectorChange(component, oldIndex, - newIndex)); - } - } - - // populate diff with removed - for (Entry entry : prevRowIndexToDetails - .entrySet()) { - Integer oldIndex = entry.getKey(); - Component component = entry.getValue(); - Integer newIndex = rowIndexToDetails.inverse().get(component); - if (newIndex == null) { - changes.add(new DetailsConnectorChange(null, oldIndex, null)); - } - } - - // reset diff map - prevRowIndexToDetails = HashBiMap.create(rowIndexToDetails); - - return changes; - } - - public void refresh(Object itemId) { - Component component = visibleDetailsComponents.get(itemId); - Integer rowIndex = null; - if (component != null) { - rowIndex = rowIndexToDetails.inverse().get(component); - destroyDetails(itemId); - } else { - rowIndex = emptyDetails.remove(itemId); - } - - assert rowIndex != null : "Given itemId does not map to an existing detail row (" - + itemId + ")"; - createDetails(itemId, rowIndex.intValue()); - } - } - /** * The data source attached to the grid */ @@ -3349,7 +3106,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, */ private DetailsGenerator detailsGenerator = DetailsGenerator.NULL; - private final DetailComponentManager detailComponentManager = new DetailComponentManager(); + private DetailComponentManager detailComponentManager = null; private static final Method SELECTION_CHANGE_METHOD = ReflectTools .findMethod(SelectionListener.class, "select", SelectionEvent.class); @@ -3765,10 +3522,12 @@ public class Grid extends AbstractComponent implements SelectionNotifier, sortOrder.clear(); } - datasourceExtension = new RpcDataProviderExtension(container, - detailComponentManager); + datasourceExtension = new RpcDataProviderExtension(container); datasourceExtension.extend(this, columnKeys); + detailComponentManager = datasourceExtension + .getDetailComponentManager(); + /* * selectionModel == null when the invocation comes from the * constructor. diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java b/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java index 54f5dcdbc7..9ecf131c5b 100644 --- a/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java +++ b/server/tests/src/com/vaadin/tests/server/component/grid/DataProviderExtension.java @@ -47,7 +47,7 @@ public class DataProviderExtension { container = new IndexedContainer(); populate(container); - dataProvider = new RpcDataProviderExtension(container, null); + dataProvider = new RpcDataProviderExtension(container); keyMapper = dataProvider.getKeyMapper(); } -- cgit v1.2.3 From 27e0595fdb784891eda15a7b02f899d5d358b0d1 Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Fri, 20 Mar 2015 12:20:10 +0200 Subject: Fix various small issues discovered while reviewing API Change-Id: I196e490d5c5ae77ba895e0fca1b0d9160b6a7855 --- client/src/com/vaadin/client/widget/grid/AutoScroller.java | 8 ++++---- client/src/com/vaadin/client/widget/grid/DetailsGenerator.java | 1 - .../vaadin/client/widget/grid/events/ColumnReorderHandler.java | 1 - client/src/com/vaadin/client/widgets/Escalator.java | 2 +- client/src/com/vaadin/client/widgets/Grid.java | 2 +- server/src/com/vaadin/ui/Grid.java | 4 +++- shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) (limited to 'server/src') diff --git a/client/src/com/vaadin/client/widget/grid/AutoScroller.java b/client/src/com/vaadin/client/widget/grid/AutoScroller.java index f7c80df623..773dc012c8 100644 --- a/client/src/com/vaadin/client/widget/grid/AutoScroller.java +++ b/client/src/com/vaadin/client/widget/grid/AutoScroller.java @@ -75,7 +75,7 @@ public class AutoScroller { public enum ScrollAxis { VERTICAL, HORIZONTAL - }; + } /** The maximum number of pixels per second to autoscroll. */ private static final int SCROLL_TOP_SPEED_PX_SEC = 500; @@ -508,10 +508,10 @@ public class AutoScroller { * Defaults to 100px. * * @param px - * the height/width for the auto scroll area depending on + * the pixel height/width for the auto scroll area depending on * direction */ - public void setScrollAreaPX(int px) { + public void setScrollArea(int px) { scrollAreaPX = px; } @@ -522,7 +522,7 @@ public class AutoScroller { * * @return size in pixels */ - public int getScrollAreaPX() { + public int getScrollArea() { return scrollAreaPX; } diff --git a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java index 309e3f1ea3..103bf96291 100644 --- a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java +++ b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java @@ -42,6 +42,5 @@ public interface DetailsGenerator { * @return the details for the given row, or null to leave the * details empty. */ - // TODO: provide a row object instead of index (maybe, needs discussion?) Widget getDetails(int rowIndex); } diff --git a/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java b/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java index e4f258088f..4733ed8bc0 100644 --- a/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java +++ b/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java @@ -33,7 +33,6 @@ public interface ColumnReorderHandler extends EventHandler { * A column reorder event, fired by Grid when the columns of the Grid have * been reordered. * - * @since * @param event * column reorder event */ diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index 0d65dda611..75b797eb1f 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -4494,7 +4494,7 @@ public class Escalator extends Widget implements RequiresResize, * The meaning of each value may differ depending on the context it is being * used in. Check that particular method's JavaDoc. */ - public enum SpacerInclusionStrategy { + private enum SpacerInclusionStrategy { /** A representation of "the entire spacer". */ COMPLETE, diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index e9288a7ece..9821591b91 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -3385,7 +3385,7 @@ public class Grid extends ResizeComposite implements dragElement.addClassName("dragged-column-header"); // start the auto scroll handler - autoScroller.setScrollAreaPX(60); + autoScroller.setScrollArea(60); autoScroller.start(startingEvent, ScrollAxis.HORIZONTAL, autoScrollerCallback); return true; diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 4488789406..4a52dba173 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -194,13 +194,15 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Constructor for a column visibility change event. * + * @param source + * the grid from which this event originates * @param column * the column that changed its visibility * @param isUserOriginated * true iff the event was triggered by an UI * interaction */ - public ColumnVisibilityChangeEvent(Component source, Column column, + public ColumnVisibilityChangeEvent(Grid source, Column column, boolean isUserOriginated) { super(source); this.column = column; diff --git a/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java index 40f4541fb1..5b80f27b1e 100644 --- a/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java +++ b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java @@ -21,7 +21,7 @@ import com.vaadin.shared.Connector; /** * A description of an indexing modification for a connector. This is used by - * Grid by internal bookkeeping updates. + * Grid for internal bookkeeping updates. * * @since * @author Vaadin Ltd -- cgit v1.2.3 From 16c67cfab9b3dd2dbf324caa612fa3a2d15550d0 Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Sun, 22 Mar 2015 22:20:57 +0200 Subject: Grid column hiding info from client to server #(17023) Fixes mismatched client-server API regarding ColumnVisibilityChangeEvent Adds and removes the column hiding toggle as needed when columns added / removed. Known bug when a hidable column added, column toggle won't get the caption of column. Change-Id: I708e19432dc822f713bf11f5b8e6eadb528a3961 --- WebContent/VAADIN/themes/base/grid/grid.scss | 20 +++- .../vaadin/client/connectors/GridConnector.java | 35 +++++- .../grid/events/ColumnVisibilityChangeEvent.java | 2 +- client/src/com/vaadin/client/widgets/Grid.java | 37 +++++-- server/src/com/vaadin/ui/Grid.java | 65 +++++++++-- .../com/vaadin/shared/ui/grid/GridServerRpc.java | 15 +++ .../grid/basicfeatures/GridBasicFeatures.java | 6 + .../server/GridColumnVisibilityTest.java | 121 ++++++++++++++++++++- 8 files changed, 278 insertions(+), 23 deletions(-) (limited to 'server/src') diff --git a/WebContent/VAADIN/themes/base/grid/grid.scss b/WebContent/VAADIN/themes/base/grid/grid.scss index 87b936a1b9..ccb7043c50 100644 --- a/WebContent/VAADIN/themes/base/grid/grid.scss +++ b/WebContent/VAADIN/themes/base/grid/grid.scss @@ -84,16 +84,17 @@ $v-grid-editor-background-color: $v-grid-row-background-color !default; .#{$primaryStyleName}-sidebar { position: absolute; top: 1px; - right : 0; - + right : 1px; + background-color: $v-grid-header-background-color; border-left: $v-grid-header-border; border-bottom: $v-grid-header-border; z-index: 5; - + .#{$primaryStyleName}-sidebar-button { height: $v-grid-header-row-height; - + text-align: right; + &:after { content: "\f0c9"; font-family: FontAwesome, sans-serif; @@ -102,6 +103,17 @@ $v-grid-editor-background-color: $v-grid-row-background-color !default; padding: 0 $v-grid-cell-padding-horizontal; } } + + .#{$primaryStyleName}-sidebar-content { + + .column-hiding-panel { + display: block; + .column-hiding-toggle { + display: block; + padding: 3px 12px; + } + } + } } // Common cell styles diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 0807690023..7c568e02e5 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -56,6 +56,8 @@ import com.vaadin.client.widget.grid.events.BodyClickHandler; import com.vaadin.client.widget.grid.events.BodyDoubleClickHandler; import com.vaadin.client.widget.grid.events.ColumnReorderEvent; import com.vaadin.client.widget.grid.events.ColumnReorderHandler; +import com.vaadin.client.widget.grid.events.ColumnVisibilityChangeEvent; +import com.vaadin.client.widget.grid.events.ColumnVisibilityChangeHandler; import com.vaadin.client.widget.grid.events.GridClickEvent; import com.vaadin.client.widget.grid.events.GridDoubleClickEvent; import com.vaadin.client.widget.grid.events.SelectAllEvent; @@ -388,6 +390,33 @@ public class GridConnector extends AbstractHasComponentsConnector implements } }; + private ColumnVisibilityChangeHandler columnVisibilityChangeHandler = new ColumnVisibilityChangeHandler() { + + @Override + public void onVisibilityChange( + ColumnVisibilityChangeEvent event) { + if (!columnsUpdatedFromState) { + Column column = event.getColumn(); + if (column instanceof CustomGridColumn) { + getRpcProxy(GridServerRpc.class).columnVisibilityChanged( + ((CustomGridColumn) column).id, column.isHidden(), + event.isUserOriginated()); + for (GridColumnState state : getState().columns) { + if (state.id.equals(((CustomGridColumn) column).id)) { + state.hidden = event.isHidden(); + break; + } + } + } else { + getLogger().warning( + "Visibility changed for a unknown column type in Grid: " + + column.toString() + ", type " + + column.getClass()); + } + } + } + }; + private static class CustomDetailsGenerator implements DetailsGenerator { private final Map indexToDetailsMap = new HashMap(); @@ -713,6 +742,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements getWidget().setEditorHandler(new CustomEditorHandler()); getWidget().addColumnReorderHandler(columnReorderHandler); + getWidget().addColumnVisibilityChangeHandler( + columnVisibilityChangeHandler); getWidget().setDetailsGenerator(customDetailsGenerator); getLayoutManager().registerDependency(this, getWidget().getElement()); @@ -734,7 +765,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements if (!columnIdToColumn.containsKey(state.id)) { addColumnFromStateChangeEvent(state); } - updateColumnFromState(columnIdToColumn.get(state.id), state); + updateColumnFromStateChangeEvent(state); } } @@ -947,7 +978,9 @@ public class GridConnector extends AbstractHasComponentsConnector implements private void updateColumnFromStateChangeEvent(GridColumnState columnState) { CustomGridColumn column = columnIdToColumn.get(columnState.id); + columnsUpdatedFromState = true; updateColumnFromState(column, columnState); + columnsUpdatedFromState = false; if (columnState.rendererConnector != column.getRendererConnector()) { throw new UnsupportedOperationException( diff --git a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java index 10bfbfad68..4c25f7a61b 100644 --- a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java +++ b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java @@ -60,7 +60,7 @@ public class ColumnVisibilityChangeEvent extends } /** - * Is the column hidden or visible. + * Was the column set hidden or visible. * * @return true if the column was hidden false if * it was set visible diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 3e201277a0..3b26c8be57 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -68,7 +68,6 @@ import com.google.gwt.user.client.ui.HasEnabled; import com.google.gwt.user.client.ui.HasWidgets; import com.google.gwt.user.client.ui.ResizeComposite; import com.google.gwt.user.client.ui.ToggleButton; -import com.google.gwt.user.client.ui.VerticalPanel; import com.google.gwt.user.client.ui.Widget; import com.vaadin.client.BrowserInfo; import com.vaadin.client.DeferredWorker; @@ -1926,7 +1925,8 @@ public class Grid extends ResizeComposite implements */ private void setCellFocus(int rowIndex, int columnIndexDOM, RowContainer container) { - if (rowIndex == rowWithFocus && cellFocusRange.contains(columnIndexDOM) + if (rowIndex == rowWithFocus + && cellFocusRange.contains(columnIndexDOM) && container == this.containerWithFocus) { refreshRow(rowWithFocus); return; @@ -1955,10 +1955,12 @@ public class Grid extends ResizeComposite implements ++i; } while (cell != null); } - int columnIndex = getColumns().indexOf(getVisibleColumn(columnIndexDOM)); + int columnIndex = getColumns().indexOf( + getVisibleColumn(columnIndexDOM)); if (columnIndex >= escalator.getColumnConfiguration() .getFrozenColumnCount()) { - escalator.scrollToColumn(columnIndexDOM, ScrollDestination.ANY, 10); + escalator.scrollToColumn(columnIndexDOM, ScrollDestination.ANY, + 10); } if (this.containerWithFocus == container) { @@ -3048,7 +3050,7 @@ public class Grid extends ResizeComposite implements * UI and functionality related to hiding columns with toggles in the * sidebar. */ - private final class ColumnHider extends VerticalPanel { + private final class ColumnHider extends FlowPanel { ColumnHider() { setStyleName("column-hiding-panel"); @@ -3073,7 +3075,7 @@ public class Grid extends ResizeComposite implements } private ToggleButton createToggle(final Column column) { - ToggleButton toggle = new ToggleButton(column.headerCaption); + ToggleButton toggle = new ToggleButton(); toggle.addStyleName("column-hiding-toggle"); toggle.addValueChangeHandler(new ValueChangeHandler() { @@ -3082,6 +3084,7 @@ public class Grid extends ResizeComposite implements column.setHidden(!event.getValue(), true); } }); + updateColumnHidingToggleCaption(column, toggle); columnToHidingToggleMap.put(column, toggle); return toggle; } @@ -3119,7 +3122,19 @@ public class Grid extends ResizeComposite implements } private void updateColumnHidingToggleCaption(Column column) { - columnToHidingToggleMap.get(column).setText(column.headerCaption); + updateColumnHidingToggleCaption(column, + columnToHidingToggleMap.get(column)); + } + + private void updateColumnHidingToggleCaption(Column column, + ToggleButton toggle) { + String caption = column.headerCaption; + if (caption == null || caption.isEmpty()) { + // TODO what if the content is a widget? + HeaderCell cell = getDefaultHeaderRow().getCell(column); + caption = cell.getText(); + } + toggle.setText(caption); } } @@ -5001,6 +5016,10 @@ public class Grid extends ResizeComposite implements Set events = new HashSet(); events.addAll(getConsumedEventsForRenderer(column.getRenderer())); + if (column.isHidable()) { + columnHider.updateColumnHidable(column); + } + sinkEvents(events); } @@ -5061,6 +5080,10 @@ public class Grid extends ResizeComposite implements ((Column) column).setGrid(null); columns.remove(columnIndex); + + if (column.isHidable()) { + columnHider.updateColumnHidable(column); + } } /** diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 4a52dba173..e093a99159 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -182,7 +182,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } /** - * An event that is fired when a column becomes hidden or unhidden. + * An event that is fired when a column's visibility changes. * * @since */ @@ -190,6 +190,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, private final Column column; private final boolean userOriginated; + private final boolean hidden; /** * Constructor for a column visibility change event. @@ -198,27 +199,41 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * the grid from which this event originates * @param column * the column that changed its visibility + * @param hidden + * true if the column was hidden, + * false if it became visible * @param isUserOriginated * true iff the event was triggered by an UI * interaction */ public ColumnVisibilityChangeEvent(Grid source, Column column, - boolean isUserOriginated) { + boolean hidden, boolean isUserOriginated) { super(source); this.column = column; + this.hidden = hidden; userOriginated = isUserOriginated; } /** - * Gets the column that became hidden or unhidden. + * Gets the column that became hidden or visible. * - * @return the column that became hidden or unhidden. + * @return the column that became hidden or visible. * @see Column#isHidden() */ public Column getColumn() { return column; } + /** + * Was the column set hidden or visible. + * + * @return true if the column was hidden false + * if it was set visible + */ + public boolean isHidden() { + return hidden; + } + /** * Returns true if the column reorder was done by the user, * false if not and it was triggered by server side code. @@ -2828,7 +2843,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, if (hidden != getState().hidden) { getState().hidden = hidden; grid.markAsDirty(); - grid.fireColumnVisibilityChangeEvent(this, false); + grid.fireColumnVisibilityChangeEvent(this, hidden, false); } } @@ -3358,6 +3373,42 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } } + @Override + public void columnVisibilityChanged(String id, boolean hidden, + boolean userOriginated) { + final Column column = getColumnByColumnId(id); + final GridColumnState columnState = column.getState(); + + if (columnState.hidden != hidden) { + columnState.hidden = hidden; + + final String diffStateKey = "columns"; + ConnectorTracker connectorTracker = getUI() + .getConnectorTracker(); + JsonObject diffState = connectorTracker + .getDiffState(Grid.this); + + assert diffState.hasKey(diffStateKey) : "Field name has changed"; + Type type = null; + try { + type = (getState(false).getClass().getDeclaredField( + diffStateKey).getGenericType()); + } catch (NoSuchFieldException e) { + e.printStackTrace(); + } catch (SecurityException e) { + e.printStackTrace(); + } + EncodeResult encodeResult = JsonCodec.encode( + getState(false).columns, diffState, type, + connectorTracker); + + diffState.put(diffStateKey, encodeResult.getEncodedValue()); + + fireColumnVisibilityChangeEvent(column, hidden, + userOriginated); + } + } + @Override public void sendDetailsComponents(int fetchId) { getRpcProxy(GridClientRpc.class).setDetailsConnectorChanges( @@ -5455,9 +5506,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, COLUMN_VISIBILITY_METHOD); } - private void fireColumnVisibilityChangeEvent(Column column, + private void fireColumnVisibilityChangeEvent(Column column, boolean hidden, boolean isUserOriginated) { - fireEvent(new ColumnVisibilityChangeEvent(this, column, + fireEvent(new ColumnVisibilityChangeEvent(this, column, hidden, isUserOriginated)); } diff --git a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java index 28f59ea93a..2b2308fe84 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java @@ -76,4 +76,19 @@ public interface GridServerRpc extends ServerRpc { * @see com.vaadin.ui.Grid#setDetailsVisible(Object, boolean) */ void sendDetailsComponents(int fetchId); + + /** + * Informs the server that the column's visibility has been changed. + * + * @since + * @param id + * the id of the column + * @param hidden + * true if hidden, false if unhidden + * @param userOriginated + * true if triggered by user, false if + * by code + */ + void columnVisibilityChanged(String id, boolean hidden, + boolean userOriginated); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index c8c0e54123..d3b1237cf9 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -767,12 +767,18 @@ public class GridBasicFeatures extends AbstractComponentTest { createClickAction("Add / Remove", getColumnProperty(c), new Command() { + boolean wasHidable; + @Override public void execute(Grid grid, String value, Object data) { String columnProperty = getColumnProperty((Integer) data); if (grid.getColumn(columnProperty) == null) { grid.addColumn(columnProperty); + grid.getColumn(columnProperty).setHidable( + wasHidable); } else { + wasHidable = grid.getColumn(columnProperty) + .isHidable(); grid.removeColumn(columnProperty); } } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java index 8fb733dfa0..22a08d6748 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java @@ -17,18 +17,22 @@ package com.vaadin.tests.components.grid.basicfeatures.server; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import java.util.List; + import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; -import com.vaadin.testbench.annotations.RunLocally; -import com.vaadin.testbench.parallel.Browser; import com.vaadin.testbench.parallel.TestCategory; import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest; @TestCategory("grid") -@RunLocally(Browser.PHANTOMJS) public class GridColumnVisibilityTest extends GridBasicFeaturesTest { private static final String[] TOGGLE_LISTENER = new String[] { "Component", @@ -40,6 +44,8 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest { + "changed: propertyId: Column 0, isHidden: true"; private static final String COLUMN_0_BECAME_UNHIDDEN_MSG = "Visibility " + "changed: propertyId: Column 0, isHidden: false"; + private static final String USER_ORIGINATED_TRUE = "userOriginated: true"; + private static final String USER_ORIGINATED_FALSE = "userOriginated: false"; @Before public void setUp() { @@ -72,9 +78,11 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest { selectMenuPath(TOGGLE_HIDE_COLUMN_0); assertTrue(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG)); + assertTrue(logContainsText(USER_ORIGINATED_FALSE)); selectMenuPath(TOGGLE_HIDE_COLUMN_0); assertTrue(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG)); + assertTrue(logContainsText(USER_ORIGINATED_FALSE)); } @Test @@ -86,4 +94,111 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest { selectMenuPath(TOGGLE_HIDE_COLUMN_0); assertFalse(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG)); } + + @Test + public void testColumnHiding_userOriginated_correctParams() { + selectMenuPath(TOGGLE_LISTENER); + toggleColumnHidable(0); + assertColumnHeaderOrder(0, 1, 2, 3); + + getSidebarOpenButton().click(); + getColumnHidingToggle(0).click(); + getSidebarOpenButton().click(); + + assertColumnHeaderOrder(1, 2, 3); + assertTrue(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG)); + assertTrue(logContainsText(USER_ORIGINATED_TRUE)); + + getSidebarOpenButton().click(); + getColumnHidingToggle(0).click(); + getSidebarOpenButton().click(); + + assertColumnHeaderOrder(0, 1, 2, 3); + assertTrue(logContainsText(COLUMN_0_BECAME_UNHIDDEN_MSG)); + assertTrue(logContainsText(USER_ORIGINATED_TRUE)); + + getSidebarOpenButton().click(); + getColumnHidingToggle(0).click(); + getSidebarOpenButton().click(); + + assertColumnHeaderOrder(1, 2, 3); + assertTrue(logContainsText(COLUMN_0_BECAME_HIDDEN_MSG)); + assertTrue(logContainsText(USER_ORIGINATED_TRUE)); + } + + @Test + public void testColumnHiding_whenHidableColumnRemoved_toggleRemoved() { + toggleColumnHidable(0); + toggleColumnHidable(1); + getSidebarOpenButton().click(); + assertNotNull(getColumnHidingToggle(0)); + + addRemoveColumn(0); + + assertNull(getColumnHidingToggle(0)); + } + + @Test + @Ignore + // known issue, column caption not passed to toggle when added again + public void testColumnHiding_whenHidableColumnAdded_toggleAdded() { + selectMenuPath("Component", "Size", "Width", "100%"); + toggleColumnHidable(0); + toggleColumnHidable(1); + addRemoveColumn(0); + addRemoveColumn(4); + addRemoveColumn(5); + addRemoveColumn(6); + addRemoveColumn(7); + addRemoveColumn(8); + addRemoveColumn(9); + addRemoveColumn(10); + assertColumnHeaderOrder(1, 2, 3, 11); + + getSidebarOpenButton().click(); + assertNull(getColumnHidingToggle(0)); + getSidebarOpenButton().click(); + + addRemoveColumn(0); + assertColumnHeaderOrder(1, 2, 3, 11, 0); + + getSidebarOpenButton().click(); + assertNotNull(getColumnHidingToggle(0)); + } + + private void toggleColumnHidable(int index) { + selectMenuPath("Component", "Columns", "Column " + index, "Hidable"); + } + + private void addRemoveColumn(int index) { + selectMenuPath("Component", "Columns", "Column " + index, + "Add / Remove"); + } + + private WebElement getSidebar() { + List elements = findElements(By.className("v-grid-sidebar")); + return elements.isEmpty() ? null : elements.get(0); + } + + private WebElement getSidebarOpenButton() { + List elements = findElements(By + .className("v-grid-sidebar-button")); + return elements.isEmpty() ? null : elements.get(0); + } + + /** + * Returns the toggle inside the sidebar for hiding the column at the given + * index, or null if not found. + */ + private WebElement getColumnHidingToggle(int columnIndex) { + WebElement sidebar = getSidebar(); + List elements = sidebar.findElements(By + .className("column-hiding-toggle")); + for (WebElement e : elements) { + if (("Column " + columnIndex).equalsIgnoreCase(e.getText())) { + return e; + } + } + return null; + } } -- cgit v1.2.3 From 6a7437cc96da860e50297e064abe7aef387c9e2c Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Tue, 24 Mar 2015 15:11:23 +0200 Subject: Fixes edge case in null details generation for Grid (#17274) Change-Id: I1bf4c2f0600baea8b925bd31dcd42c1e901a7c8b --- .../vaadin/client/connectors/GridConnector.java | 28 +++++++++++--- .../com/vaadin/data/RpcDataProviderExtension.java | 5 ++- .../shared/ui/grid/DetailsConnectorChange.java | 45 +++++++++++++++++++++- .../server/GridDetailsServerTest.java | 26 ++++++++++++- 4 files changed, 93 insertions(+), 11 deletions(-) (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 7c568e02e5..51e986933c 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -19,6 +19,7 @@ package com.vaadin.client.connectors; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -677,8 +678,13 @@ public class GridConnector extends AbstractHasComponentsConnector implements customDetailsGenerator .setDetailsConnectorChanges(connectorChanges); + List removedFirst = new ArrayList( + connectorChanges); + Collections.sort(removedFirst, + DetailsConnectorChange.REMOVED_FIRST_COMPARATOR); + // refresh moved/added details rows - for (DetailsConnectorChange change : connectorChanges) { + for (DetailsConnectorChange change : removedFirst) { Integer oldIndex = change.getOldIndex(); Integer newIndex = change.getNewIndex(); @@ -689,13 +695,23 @@ public class GridConnector extends AbstractHasComponentsConnector implements + "invalid new index: " + newIndex + " (connector: " + change.getConnector() + ")"; - Integer index = newIndex; - if (index == null) { - index = oldIndex; + if (oldIndex != null) { + /* Close the old/removed index */ + getWidget().setDetailsVisible(oldIndex, false); + + if (change.isShouldStillBeVisible()) { + getWidget().setDetailsVisible(oldIndex, true); + } } - getWidget().setDetailsVisible(index, false); - getWidget().setDetailsVisible(index, true); + if (newIndex != null) { + /* + * Since the component was lazy loaded, we need to + * refresh the details by toggling it. + */ + getWidget().setDetailsVisible(newIndex, false); + getWidget().setDetailsVisible(newIndex, true); + } } detailsConnectorFetcher.responseReceived(fetchId); } diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index 66c17c4afa..a21a81244a 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -787,7 +787,7 @@ public class RpcDataProviderExtension extends AbstractExtension { if (!SharedUtil.equals(oldIndex, newIndex)) { changes.add(new DetailsConnectorChange(component, oldIndex, - newIndex)); + newIndex, emptyDetails.containsKey(component))); } } @@ -798,7 +798,8 @@ public class RpcDataProviderExtension extends AbstractExtension { Component component = entry.getValue(); Integer newIndex = rowIndexToDetails.inverse().get(component); if (newIndex == null) { - changes.add(new DetailsConnectorChange(null, oldIndex, null)); + changes.add(new DetailsConnectorChange(null, oldIndex, + null, emptyDetails.containsValue(oldIndex))); } } diff --git a/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java index 5b80f27b1e..171a7738a6 100644 --- a/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java +++ b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java @@ -16,6 +16,7 @@ package com.vaadin.shared.ui.grid; import java.io.Serializable; +import java.util.Comparator; import com.vaadin.shared.Connector; @@ -28,9 +29,25 @@ import com.vaadin.shared.Connector; */ public class DetailsConnectorChange implements Serializable { + public static final Comparator REMOVED_FIRST_COMPARATOR = new Comparator() { + @Override + public int compare(DetailsConnectorChange a, DetailsConnectorChange b) { + boolean deleteA = a.getNewIndex() == null; + boolean deleteB = b.getNewIndex() == null; + if (deleteA && !deleteB) { + return -1; + } else if (!deleteA && deleteB) { + return 1; + } else { + return 0; + } + } + }; + private Connector connector; private Integer oldIndex; private Integer newIndex; + private boolean shouldStillBeVisible; /** Create a new connector index change */ public DetailsConnectorChange() { @@ -48,12 +65,15 @@ public class DetailsConnectorChange implements Serializable { * the old index * @param newIndex * the new index + * @param shouldStillBeVisible + * details should be visible regardless of {@code connector} */ public DetailsConnectorChange(Connector connector, Integer oldIndex, - Integer newIndex) { + Integer newIndex, boolean shouldStillBeVisible) { this.connector = connector; this.oldIndex = oldIndex; this.newIndex = newIndex; + this.shouldStillBeVisible = shouldStillBeVisible; assert assertStateIsOk(); } @@ -144,4 +164,27 @@ public class DetailsConnectorChange implements Serializable { public void setNewIndex(Integer newIndex) { this.newIndex = newIndex; } + + /** + * Checks whether whether the details should remain open, even if connector + * might be null. + * + * @return true iff the details should remain open, even if + * connector might be null + */ + public boolean isShouldStillBeVisible() { + return shouldStillBeVisible; + } + + /** + * Sets whether the details should remain open, even if connector might be + * null. + * + * @param shouldStillBeVisible + * true iff the details should remain open, even if + * connector might be null + */ + public void setShouldStillBeVisible(boolean shouldStillBeVisible) { + this.shouldStillBeVisible = shouldStillBeVisible; + } } 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 7ddd903161..cfaf79ea05 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 @@ -80,6 +80,22 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { getGridElement().getDetails(0); } + @Test + public void openVisiblePopulatedDetails() { + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + assertNotNull("details should've populated", getGridElement() + .getDetails(0).findElement(By.className("v-widget"))); + } + + @Test(expected = NoSuchElementException.class) + public void closeVisiblePopulatedDetails() { + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + getGridElement().getDetails(0); + } + @Test public void openDetailsOutsideOfActiveRange() throws InterruptedException { getGridElement().scroll(10000); @@ -260,8 +276,14 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { selectMenuPath(OPEN_FIRST_ITEM_DETAILS); selectMenuPath(DETAILS_GENERATOR_WATCHING); selectMenuPath(DETAILS_GENERATOR_NULL); - assertTrue("Details should be empty with null component", - getGridElement().getDetails(0).getText().isEmpty()); + + 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()); -- cgit v1.2.3 From da95f395add4916d0544858c87b448ddc5d03881 Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Tue, 24 Mar 2015 15:19:35 +0200 Subject: Fix Grid's frozen column count with hidden columns #17273 Change-Id: I4f8a893eec3cf7c32da34cb364a4d56589cbf3e2 --- client/src/com/vaadin/client/widgets/Grid.java | 28 +++++++- server/src/com/vaadin/ui/Grid.java | 3 + .../grid/basicfeatures/GridColumnHidingTest.java | 75 ++++++++++++++++++++++ .../server/GridColumnVisibilityTest.java | 69 ++++++++++++++++++++ 4 files changed, 174 insertions(+), 1 deletion(-) (limited to 'server/src') diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 15dd45ec21..5ed2f2f949 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -4279,8 +4279,23 @@ public class Grid extends ResizeComposite implements this.hidden = hidden; } else { this.hidden = hidden; + + final int columnIndex = grid.getVisibleColumns().indexOf( + this); grid.escalator.getColumnConfiguration().insertColumns( - grid.getVisibleColumns().indexOf(this), 1); + columnIndex, 1); + + // make sure column is set to frozen if it needs to be, + // escalator doesn't handle situation where the added column + // would be the last frozen column + int gridFrozenColumns = grid.getFrozenColumnCount(); + int escalatorFrozenColumns = grid.escalator + .getColumnConfiguration().getFrozenColumnCount(); + if (gridFrozenColumns > escalatorFrozenColumns + && escalatorFrozenColumns == columnIndex) { + grid.escalator.getColumnConfiguration() + .setFrozenColumnCount(++escalatorFrozenColumns); + } } grid.columnHider.updateToggleValue(this); scheduleColumnWidthRecalculator(); @@ -5826,6 +5841,14 @@ public class Grid extends ResizeComposite implements private void updateFrozenColumns() { int numberOfColumns = frozenColumnCount; + // for the escalator the hidden columns are not in the frozen column + // count, but for grid they are. thus need to convert the index + for (int i = 0; i < frozenColumnCount; i++) { + if (getColumn(i).isHidden()) { + numberOfColumns--; + } + } + if (numberOfColumns == -1) { numberOfColumns = 0; } else if (selectionColumn != null) { @@ -5842,6 +5865,9 @@ public class Grid extends ResizeComposite implements * columns will be frozen, but the built-in selection checkbox column will * still be frozen if it's in use. -1 means that not even the selection * column is frozen. + *

+ * NOTE: This includes {@link Column#isHidden() hidden columns} in + * the count. * * @return the number of frozen columns */ diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index e093a99159..62b3a74f05 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -3954,6 +3954,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * columns will be frozen, but the built-in selection checkbox column will * still be frozen if it's in use. -1 means that not even the selection * column is frozen. + *

+ * NOTE: this count includes {@link Column#isHidden() hidden + * columns} in the count. * * @see #setFrozenColumnCount(int) * 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 aca7689a0a..c12c769c63 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java @@ -556,6 +556,81 @@ public class GridColumnHidingTest extends GridBasicClientFeaturesTest { assertTrue(cell.isFocused()); } + @Test + public void testFrozenColumnHiding_lastFrozenColumnHidden_isFrozenWhenMadeVisible() { + toggleFrozenColumns(2); + toggleHidableColumnAPI(0); + toggleHidableColumnAPI(1); + getSidebarOpenButton().click(); + verifyColumnIsFrozen(0); + verifyColumnIsFrozen(1); + verifyColumnIsNotFrozen(2); + assertColumnHeaderOrder(0, 1, 2, 3); + + getColumnHidingToggle(1).click(); + verifyColumnIsFrozen(0); + // the grid element indexing doesn't take hidden columns into account! + verifyColumnIsNotFrozen(1); + assertColumnHeaderOrder(0, 2, 3); + + getColumnHidingToggle(0).click(); + verifyColumnIsNotFrozen(0); + assertColumnHeaderOrder(2, 3, 4); + + getColumnHidingToggle(0).click(); + assertColumnHeaderOrder(0, 2, 3); + verifyColumnIsFrozen(0); + verifyColumnIsNotFrozen(1); + + getColumnHidingToggle(1).click(); + assertColumnHeaderOrder(0, 1, 2, 3); + verifyColumnIsFrozen(0); + verifyColumnIsFrozen(1); + verifyColumnIsNotFrozen(2); + } + + @Test + public void testFrozenColumnHiding_columnHiddenFrozenCountChanged_columnIsFrozenWhenVisible() { + toggleHidableColumnAPI(1); + toggleHidableColumnAPI(2); + getSidebarOpenButton().click(); + getColumnHidingToggle(1).click(); + getColumnHidingToggle(2).click(); + assertColumnHeaderOrder(0, 3, 4); + + toggleFrozenColumns(3); + verifyColumnIsFrozen(0); + // the grid element indexing doesn't take hidden columns into account! + verifyColumnIsNotFrozen(1); + verifyColumnIsNotFrozen(2); + + getColumnHidingToggle(2).click(); + verifyColumnIsFrozen(0); + verifyColumnIsFrozen(1); + verifyColumnIsNotFrozen(2); + verifyColumnIsNotFrozen(3); + + getColumnHidingToggle(1).click(); + verifyColumnIsFrozen(0); + verifyColumnIsFrozen(1); + verifyColumnIsFrozen(2); + verifyColumnIsNotFrozen(3); + verifyColumnIsNotFrozen(4); + } + + private void toggleFrozenColumns(int count) { + selectMenuPath("Component", "State", "Frozen column count", count + + " columns"); + } + + private void verifyColumnIsFrozen(int index) { + assertTrue(getGridElement().getHeaderCell(0, index).isFrozen()); + } + + private void verifyColumnIsNotFrozen(int index) { + assertFalse(getGridElement().getHeaderCell(0, index).isFrozen()); + } + private void verifyColumnHidingTogglesOrder(int... indices) { WebElement sidebar = getSidebar(); List elements = sidebar.findElements(By diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java index 22a08d6748..37eda1c28f 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java @@ -166,6 +166,75 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest { assertNotNull(getColumnHidingToggle(0)); } + @Test + public void testFrozenColumnHiding_hiddenColumnMadeFrozen_frozenWhenMadeVisible() { + selectMenuPath("Component", "Size", "Width", "100%"); + toggleColumnHidable(0); + toggleColumnHidable(1); + getSidebarOpenButton().click(); + getColumnHidingToggle(0).click(); + getColumnHidingToggle(1).click(); + + assertColumnHeaderOrder(2, 3, 4, 5); + + setFrozenColumns(2); + verifyColumnNotFrozen(0); + verifyColumnNotFrozen(1); + + getColumnHidingToggle(0).click(); + assertColumnHeaderOrder(0, 2, 3, 4, 5); + verifyColumnFrozen(0); + verifyColumnNotFrozen(1); + + getColumnHidingToggle(1).click(); + assertColumnHeaderOrder(0, 1, 2, 3, 4, 5); + verifyColumnFrozen(0); + verifyColumnFrozen(1); + verifyColumnNotFrozen(2); + } + + @Test + public void testFrozenColumnHiding_hiddenFrozenColumnUnfrozen_notFrozenWhenMadeVisible() { + selectMenuPath("Component", "Size", "Width", "100%"); + toggleColumnHidable(0); + toggleColumnHidable(1); + setFrozenColumns(2); + verifyColumnFrozen(0); + verifyColumnFrozen(1); + verifyColumnNotFrozen(2); + verifyColumnNotFrozen(3); + + getSidebarOpenButton().click(); + getColumnHidingToggle(0).click(); + getColumnHidingToggle(1).click(); + assertColumnHeaderOrder(2, 3, 4, 5); + verifyColumnNotFrozen(0); + verifyColumnNotFrozen(1); + + setFrozenColumns(0); + verifyColumnNotFrozen(0); + verifyColumnNotFrozen(1); + + getColumnHidingToggle(0).click(); + assertColumnHeaderOrder(0, 2, 3, 4, 5); + verifyColumnNotFrozen(0); + verifyColumnNotFrozen(1); + + getColumnHidingToggle(1).click(); + assertColumnHeaderOrder(0, 1, 2, 3, 4, 5); + verifyColumnNotFrozen(0); + verifyColumnNotFrozen(1); + verifyColumnNotFrozen(2); + } + + private void verifyColumnFrozen(int index) { + assertTrue(getGridElement().getHeaderCell(0, index).isFrozen()); + } + + private void verifyColumnNotFrozen(int index) { + assertFalse(getGridElement().getHeaderCell(0, index).isFrozen()); + } + private void toggleColumnHidable(int index) { selectMenuPath("Component", "Columns", "Column " + index, "Hidable"); } -- cgit v1.2.3 From eb3406247e397c23d447bf4fd84a5052a0488876 Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Thu, 26 Mar 2015 11:27:21 +0200 Subject: Update all empty since tags published in alpha1 Change-Id: I1afce7e69beb9a61354fd82fcda194d4277dfd36 --- .../client/connectors/RpcDataSourceConnector.java | 2 +- .../client/data/AbstractRemoteDataSource.java | 2 +- .../client/widget/escalator/RowContainer.java | 2 +- .../com/vaadin/client/widget/escalator/Spacer.java | 2 +- .../client/widget/escalator/SpacerUpdater.java | 2 +- .../vaadin/client/widget/grid/AutoScroller.java | 2 +- .../vaadin/client/widget/grid/CellReference.java | 2 +- .../client/widget/grid/DetailsGenerator.java | 2 +- .../widget/grid/events/ColumnReorderEvent.java | 2 +- .../widget/grid/events/ColumnReorderHandler.java | 2 +- .../grid/events/ColumnVisibilityChangeEvent.java | 2 +- .../grid/events/ColumnVisibilityChangeHandler.java | 2 +- .../src/com/vaadin/client/widgets/Escalator.java | 4 +-- client/src/com/vaadin/client/widgets/Grid.java | 38 +++++++++++----------- .../com/vaadin/data/RpcDataProviderExtension.java | 6 ++-- server/src/com/vaadin/ui/Grid.java | 38 +++++++++++----------- .../shared/ui/grid/DetailsConnectorChange.java | 2 +- .../com/vaadin/shared/ui/grid/GridClientRpc.java | 2 +- .../com/vaadin/shared/ui/grid/GridServerRpc.java | 6 ++-- 19 files changed, 60 insertions(+), 60 deletions(-) (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java index e8c7ee5286..627ee74eca 100644 --- a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java +++ b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java @@ -48,7 +48,7 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { * A callback interface to let {@link GridConnector} know that detail * visibilities might have changed. * - * @since + * @since 7.5.0 * @author Vaadin Ltd */ interface DetailsListener { diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java index 152b66f2ca..88977d85ec 100644 --- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java +++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java @@ -341,7 +341,7 @@ public abstract class AbstractRemoteDataSource implements DataSource { * A hook that can be overridden to do something whenever a row is dropped * from the cache. * - * @since + * @since 7.5.0 * @param rowIndex * the index of the dropped row */ diff --git a/client/src/com/vaadin/client/widget/escalator/RowContainer.java b/client/src/com/vaadin/client/widget/escalator/RowContainer.java index b7aae1f4f3..abab25046c 100644 --- a/client/src/com/vaadin/client/widget/escalator/RowContainer.java +++ b/client/src/com/vaadin/client/widget/escalator/RowContainer.java @@ -39,7 +39,7 @@ public interface RowContainer { *

* The body section can contain both rows and spacers. * - * @since + * @since 7.5.0 * @author Vaadin Ltd * @see com.vaadin.client.widgets.Escalator#getBody() */ diff --git a/client/src/com/vaadin/client/widget/escalator/Spacer.java b/client/src/com/vaadin/client/widget/escalator/Spacer.java index 1e9985d6c1..6ccef88f0d 100644 --- a/client/src/com/vaadin/client/widget/escalator/Spacer.java +++ b/client/src/com/vaadin/client/widget/escalator/Spacer.java @@ -21,7 +21,7 @@ import com.google.gwt.dom.client.Element; * A representation of a spacer element in a * {@link com.vaadin.client.widget.escalator.RowContainer.BodyRowContainer}. * - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public interface Spacer { diff --git a/client/src/com/vaadin/client/widget/escalator/SpacerUpdater.java b/client/src/com/vaadin/client/widget/escalator/SpacerUpdater.java index 01d715fdd0..49adefd536 100644 --- a/client/src/com/vaadin/client/widget/escalator/SpacerUpdater.java +++ b/client/src/com/vaadin/client/widget/escalator/SpacerUpdater.java @@ -23,7 +23,7 @@ import com.vaadin.client.widget.escalator.RowContainer.BodyRowContainer; * The updater is responsible for making sure all elements are properly * constructed and cleaned up. * - * @since + * @since 7.5.0 * @author Vaadin Ltd * @see Spacer * @see BodyRowContainer diff --git a/client/src/com/vaadin/client/widget/grid/AutoScroller.java b/client/src/com/vaadin/client/widget/grid/AutoScroller.java index 83d8bf0b62..f2e44196ec 100644 --- a/client/src/com/vaadin/client/widget/grid/AutoScroller.java +++ b/client/src/com/vaadin/client/widget/grid/AutoScroller.java @@ -34,7 +34,7 @@ import com.vaadin.client.widgets.Grid; * Grid when the cursor is close enough the edge of the body of the grid, * depending on the scroll direction chosen. * - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public class AutoScroller { diff --git a/client/src/com/vaadin/client/widget/grid/CellReference.java b/client/src/com/vaadin/client/widget/grid/CellReference.java index 2e50480646..e783cb92ae 100644 --- a/client/src/com/vaadin/client/widget/grid/CellReference.java +++ b/client/src/com/vaadin/client/widget/grid/CellReference.java @@ -105,7 +105,7 @@ public class CellReference { * Gets the index of the cell in the DOM. The difference to * {@link #getColumnIndex()} is caused by hidden columns. * - * @since + * @since 7.5.0 * @return the index of the column in the DOM */ public int getColumnIndexDOM() { diff --git a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java index 103bf96291..b9427091a7 100644 --- a/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java +++ b/client/src/com/vaadin/client/widget/grid/DetailsGenerator.java @@ -20,7 +20,7 @@ import com.google.gwt.user.client.ui.Widget; /** * A callback interface for generating details for a particular row in Grid. * - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public interface DetailsGenerator { diff --git a/client/src/com/vaadin/client/widget/grid/events/ColumnReorderEvent.java b/client/src/com/vaadin/client/widget/grid/events/ColumnReorderEvent.java index c72da0c48e..1712871089 100644 --- a/client/src/com/vaadin/client/widget/grid/events/ColumnReorderEvent.java +++ b/client/src/com/vaadin/client/widget/grid/events/ColumnReorderEvent.java @@ -23,7 +23,7 @@ import com.google.gwt.event.shared.GwtEvent; * @param * The row type of the grid. The row type is the POJO type from where * the data is retrieved into the column cells. - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public class ColumnReorderEvent extends GwtEvent> { diff --git a/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java b/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java index 4733ed8bc0..29c476058e 100644 --- a/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java +++ b/client/src/com/vaadin/client/widget/grid/events/ColumnReorderHandler.java @@ -24,7 +24,7 @@ import com.google.gwt.event.shared.EventHandler; * @param * The row type of the grid. The row type is the POJO type from where * the data is retrieved into the column cells. - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public interface ColumnReorderHandler extends EventHandler { diff --git a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java index 4c25f7a61b..63b788bcf2 100644 --- a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java +++ b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeEvent.java @@ -25,7 +25,7 @@ import com.vaadin.client.widgets.Grid.Column; * @param * The row type of the grid. The row type is the POJO type from where * the data is retrieved into the column cells. - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public class ColumnVisibilityChangeEvent extends diff --git a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeHandler.java b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeHandler.java index 10a7660954..542fe4e3c1 100644 --- a/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeHandler.java +++ b/client/src/com/vaadin/client/widget/grid/events/ColumnVisibilityChangeHandler.java @@ -23,7 +23,7 @@ import com.google.gwt.event.shared.EventHandler; * * @param The row type of the grid. The row type is the POJO type from where * the data is retrieved into the column cells. - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public interface ColumnVisibilityChangeHandler extends EventHandler { diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index 87f19d2ded..2b3a254b52 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -5824,7 +5824,7 @@ public class Escalator extends Widget implements RequiresResize, * Returns the scroll width for the escalator. Note that this is not * necessary the same as {@code Element.scrollWidth} in the DOM. * - * @since + * @since 7.5.0 * @return the scroll width in pixels */ public double getScrollWidth() { @@ -5835,7 +5835,7 @@ public class Escalator extends Widget implements RequiresResize, * Returns the scroll height for the escalator. Note that this is not * necessary the same as {@code Element.scrollHeight} in the DOM. * - * @since + * @since 7.5.0 * @return the scroll height in pixels */ public double getScrollHeight() { diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 5ed2f2f949..8316cb8b45 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -653,7 +653,7 @@ public class Grid extends ResizeComposite implements /** * Returns true if this row contains spanned cells. * - * @since + * @since 7.5.0 * @return does this row contain spanned cells */ public boolean hasSpannedCells() { @@ -3121,7 +3121,7 @@ public class Grid extends ResizeComposite implements * column hiding toggles and custom widgets become visible once the sidebar * has been opened. * - * @since + * @since 7.5.0 */ private static class Sidebar extends Composite { @@ -4262,7 +4262,7 @@ public class Grid extends ResizeComposite implements * Hides or shows the column. By default columns are visible before * explicitly hiding them. * - * @since + * @since 7.5.0 * @param hidden * true to hide the column, false * to show @@ -4307,7 +4307,7 @@ public class Grid extends ResizeComposite implements /** * Is this column hidden. Default is {@code false}. * - * @since + * @since 7.5.0 * @return true if the column is currently hidden, * false otherwise */ @@ -4322,7 +4322,7 @@ public class Grid extends ResizeComposite implements * Note: it is still possible to hide the column * programmatically using {@link #setHidden(boolean)}. * - * @since + * @since 7.5.0 * @param hidable * true if the user can hide this column, * false if not @@ -4341,7 +4341,7 @@ public class Grid extends ResizeComposite implements * Note: the column can be programmatically hidden using * {@link #setHidden(boolean)} regardless of the returned value. * - * @since + * @since 7.5.0 * @return true if the user can hide the column, * false if not */ @@ -5349,7 +5349,7 @@ public class Grid extends ResizeComposite implements *

* No {@link Column#isHidden() hidden} columns included. * - * @since + * @since 7.5.0 * @return A unmodifiable list of the currently visible columns in the grid */ public List> getVisibleColumns() { @@ -5989,7 +5989,7 @@ public class Grid extends ResizeComposite implements /** * Sets the horizontal scroll offset * - * @since + * @since 7.5.0 * @param px * the number of pixels this grid should be scrolled right */ @@ -6009,7 +6009,7 @@ public class Grid extends ResizeComposite implements /** * Returns the height of the scrollable area in pixels. * - * @since + * @since 7.5.0 * @return the height of the scrollable area in pixels */ public double getScrollHeight() { @@ -6019,7 +6019,7 @@ public class Grid extends ResizeComposite implements /** * Returns the width of the scrollable area in pixels. * - * @since + * @since 7.5.0 * @return the width of the scrollable area in pixels. */ public double getScrollWidth() { @@ -7051,7 +7051,7 @@ public class Grid extends ResizeComposite implements * Register a column reorder handler to this Grid. The event for this * handler is fired when the Grid's columns are reordered. * - * @since + * @since 7.5.0 * @param handler * the handler for the event * @return the registration for the event @@ -7065,7 +7065,7 @@ public class Grid extends ResizeComposite implements * Register a column visibility change handler to this Grid. The event for * this handler is fired when the Grid's columns change visibility. * - * @since + * @since 7.5.0 * @param handler * the handler for the event * @return the registration for the event @@ -7129,7 +7129,7 @@ public class Grid extends ResizeComposite implements /** * Returns whether columns can be reordered with drag and drop. * - * @since + * @since 7.5.0 * @return true if columns can be reordered, false otherwise */ public boolean isColumnReorderingAllowed() { @@ -7139,7 +7139,7 @@ public class Grid extends ResizeComposite implements /** * Sets whether column reordering with drag and drop is allowed or not. * - * @since + * @since 7.5.0 * @param columnReorderingAllowed * specifies whether column reordering is allowed */ @@ -7572,7 +7572,7 @@ public class Grid extends ResizeComposite implements *

* The currently opened row details will be re-rendered. * - * @since + * @since 7.5.0 * @param detailsGenerator * the details generator to set * @throws IllegalArgumentException @@ -7595,7 +7595,7 @@ public class Grid extends ResizeComposite implements /** * Gets the current details generator for row details. * - * @since + * @since 7.5.0 * @return the detailsGenerator the current details generator */ public DetailsGenerator getDetailsGenerator() { @@ -7608,7 +7608,7 @@ public class Grid extends ResizeComposite implements * This method does nothing if trying to set show already-visible details, * or hide already-hidden details. * - * @since + * @since 7.5.0 * @param rowIndex * the index of the affected row * @param visible @@ -7651,7 +7651,7 @@ public class Grid extends ResizeComposite implements /** * Check whether the details for a row is visible or not. * - * @since + * @since 7.5.0 * @param rowIndex * the index of the row for which to check details * @return true iff the details for the given row is visible @@ -7680,7 +7680,7 @@ public class Grid extends ResizeComposite implements * The grid's sidebar shows the column hiding options for those columns that * have been set as {@link Column#setHidable(boolean) hidable}. * - * @since + * @since 7.5.0 * @return the sidebar widget for this grid */ private Sidebar getSidebar() { diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index a21a81244a..217b2fe392 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -588,7 +588,7 @@ public class RpcDataProviderExtension extends AbstractExtension { * A class that makes detail component related internal communication * possible between {@link RpcDataProviderExtension} and grid. * - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public static final class DetailComponentManager implements Serializable { @@ -1357,7 +1357,7 @@ public class RpcDataProviderExtension extends AbstractExtension { * If that row is currently in the client side's cache, this information * will be sent over to the client. * - * @since + * @since 7.5.0 * @param itemId * the id of the item of which to change the details visibility * @param visible @@ -1401,7 +1401,7 @@ public class RpcDataProviderExtension extends AbstractExtension { /** * Checks whether the details for a row is marked as visible. * - * @since + * @since 7.5.0 * @param itemId * the id of the item of which to check the visibility * @return true iff the detials are visible for the item. This diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 62b3a74f05..69fbb41fc2 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -170,7 +170,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * An event listener for column visibility change events in the Grid. * - * @since + * @since 7.5.0 */ public interface ColumnVisibilityChangeListener extends Serializable { /** @@ -184,7 +184,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * An event that is fired when a column's visibility changes. * - * @since + * @since 7.5.0 */ public static class ColumnVisibilityChangeEvent extends Component.Event { @@ -248,7 +248,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * A callback interface for generating details for a particular row in Grid. * - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public interface DetailsGenerator extends Serializable { @@ -452,7 +452,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * An event listener for column reorder events in the Grid. * - * @since + * @since 7.5.0 */ public interface ColumnReorderListener extends Serializable { /** @@ -467,7 +467,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * An event that is fired when the columns are reordered. * - * @since + * @since 7.5.0 */ public static class ColumnReorderEvent extends Component.Event { @@ -2834,7 +2834,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * Hides or shows the column. By default columns are visible before * explicitly hiding them. * - * @since + * @since 7.5.0 * @param hidden * true to hide the column, false * to show @@ -2850,7 +2850,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Is this column hidden. Default is {@code false}. * - * @since + * @since 7.5.0 * @return true if the column is currently hidden, * false otherwise */ @@ -2865,7 +2865,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * Note: it is still possible to hide the column * programmatically using {@link #setHidden(boolean)} * - * @since + * @since 7.5.0 * @param hidable * true iff the column may be hidable by the * user via UI interaction @@ -2882,7 +2882,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * Note: the column can be programmatically hidden using * {@link #setHidden(boolean)} regardless of the returned value. * - * @since + * @since 7.5.0 * @return true if the user can hide the column, * false if not */ @@ -3803,7 +3803,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * Returns whether column reordering is allowed. Default value is * false. * - * @since + * @since 7.5.0 * @return true if reordering is allowed */ public boolean isColumnReorderingAllowed() { @@ -3814,7 +3814,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * Sets whether or not column reordering is allowed. Default value is * false. * - * @since + * @since 7.5.0 * @param columnReorderingAllowed * specifies whether column reordering is allowed */ @@ -4399,7 +4399,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Registers a new column reorder listener. * - * @since + * @since 7.5.0 * @param listener * the listener to register */ @@ -4410,7 +4410,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Removes a previously registered column reorder listener. * - * @since + * @since 7.5.0 * @param listener * the listener to remove */ @@ -5486,7 +5486,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Registers a new column visibility change listener * - * @since + * @since 7.5.0 * @param listener * the listener to register */ @@ -5499,7 +5499,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Removes a previously registered column visibility change listener * - * @since + * @since 7.5.0 * @param listener * the listener to remove */ @@ -5520,7 +5520,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, *

* The currently opened row details will be re-rendered. * - * @since + * @since 7.5.0 * @param detailsGenerator * the details generator to set * @throws IllegalArgumentException @@ -5545,7 +5545,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Gets the current details generator for row details. * - * @since + * @since 7.5.0 * @return the detailsGenerator the current details generator */ public DetailsGenerator getDetailsGenerator() { @@ -5555,7 +5555,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Shows or hides the details for a specific item. * - * @since + * @since 7.5.0 * @param itemId * the id of the item for which to set details visibility * @param visible @@ -5569,7 +5569,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Checks whether details are visible for the given item. * - * @since + * @since 7.5.0 * @param itemId * the id of the item for which to check details visibility * @return true iff the details are visible diff --git a/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java index 171a7738a6..8b64d22423 100644 --- a/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java +++ b/shared/src/com/vaadin/shared/ui/grid/DetailsConnectorChange.java @@ -24,7 +24,7 @@ import com.vaadin.shared.Connector; * A description of an indexing modification for a connector. This is used by * Grid for internal bookkeeping updates. * - * @since + * @since 7.5.0 * @author Vaadin Ltd */ public class DetailsConnectorChange implements Serializable { diff --git a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java index 98e7fac567..d4707f9ba5 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java @@ -61,7 +61,7 @@ public interface GridClientRpc extends ClientRpc { * Informs the GridConnector on how the indexing of details connectors has * changed. * - * @since + * @since 7.5.0 * @param connectorChanges * the indexing changes of details connectors * @param fetchId diff --git a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java index 2b2308fe84..dca55c11c4 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java @@ -51,7 +51,7 @@ public interface GridServerRpc extends ServerRpc { /** * Informs the server that the columns of the Grid have been reordered. * - * @since + * @since 7.5.0 * @param newColumnOrder * a list of column ids in the new order * @param oldColumnOrder @@ -70,7 +70,7 @@ public interface GridServerRpc extends ServerRpc { * , and that is too late to change the hierarchy. So we need this * round-trip to work around that limitation. * - * @since + * @since 7.5.0 * @param fetchId * an unique identifier for the request * @see com.vaadin.ui.Grid#setDetailsVisible(Object, boolean) @@ -80,7 +80,7 @@ public interface GridServerRpc extends ServerRpc { /** * Informs the server that the column's visibility has been changed. * - * @since + * @since 7.5.0 * @param id * the id of the column * @param hidden -- cgit v1.2.3 From e191abb0da9e4660e925991490e967c178380aef Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Wed, 25 Mar 2015 18:04:45 +0200 Subject: Fixed some faulty asserts in Grid's detail row creation (#17293) Change-Id: I8e9998524c02ca1e2f9d3391fa27bacc53655c7f --- .../com/vaadin/data/RpcDataProviderExtension.java | 36 ++++++++++++++++------ .../server/GridDetailsServerTest.java | 15 +++++++++ 2 files changed, 41 insertions(+), 10 deletions(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java index 217b2fe392..e645ec60f7 100644 --- a/server/src/com/vaadin/data/RpcDataProviderExtension.java +++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java @@ -663,8 +663,10 @@ public class RpcDataProviderExtension extends AbstractExtension { assert itemId != null : "itemId was null"; Integer newRowIndex = Integer.valueOf(rowIndex); - assert !visibleDetailsComponents.containsKey(itemId) : "itemId " - + "already has a component. Should be destroyed first."; + if (visibleDetailsComponents.containsKey(itemId)) { + // Don't overwrite existing components + return; + } RowReference rowReference = new RowReference(grid); rowReference.set(itemId); @@ -695,14 +697,8 @@ public class RpcDataProviderExtension extends AbstractExtension { + "itemId is empty even though we just created a " + "component for it (" + itemId + ")"; } else { - assert !emptyDetails.containsKey(itemId) : "Bookkeeping has " - + "already itemId marked as empty (itemId: " + itemId - + ", old index: " + emptyDetails.get(itemId) - + ", new index: " + newRowIndex + ")"; - assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping" - + " already had another itemId for this empty index " - + "(index: " + newRowIndex + ", new itemId: " + itemId - + ")"; + assert assertItemIdHasNotMovedAndNothingIsOverwritten(itemId, + newRowIndex); emptyDetails.put(itemId, newRowIndex); } @@ -712,6 +708,26 @@ public class RpcDataProviderExtension extends AbstractExtension { */ } + private boolean assertItemIdHasNotMovedAndNothingIsOverwritten( + Object itemId, Integer newRowIndex) { + + Integer oldRowIndex = emptyDetails.get(itemId); + if (!SharedUtil.equals(oldRowIndex, newRowIndex)) { + + assert !emptyDetails.containsKey(itemId) : "Unexpected " + + "change of empty details row index for itemId " + + itemId + " from " + oldRowIndex + " to " + + newRowIndex; + + assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping" + + " already had another itemId for this empty index " + + "(index: " + newRowIndex + ", new itemId: " + itemId + + ")"; + } + + return true; + } + /** * Destroys correctly a details component, by the request of the client * side. 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 cfaf79ea05..4ea64073f3 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 @@ -288,4 +288,19 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest { 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); + assertFalse(logContainsText("AssertionError")); + } + + @Test + public void noAssertErrorsOnPopulatedDetailsAndScrollDown() { + selectMenuPath(DETAILS_GENERATOR_WATCHING); + selectMenuPath(OPEN_FIRST_ITEM_DETAILS); + scrollGridVerticallyTo(500); + assertFalse(logContainsText("AssertionError")); + } } -- cgit v1.2.3 From 2080f86e03552c56d52f488e4dcd72282cd64f62 Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Mon, 30 Mar 2015 15:28:41 +0300 Subject: Server Grid.scrollToRow takes details into account (#17270) Change-Id: I7b6d67aeb4d625a53e6fe370b729016a84e33214 --- .../vaadin/client/connectors/GridConnector.java | 161 ++++++++++++++- .../src/com/vaadin/client/widgets/Escalator.java | 2 +- server/src/com/vaadin/ui/Grid.java | 7 + .../com/vaadin/shared/ui/grid/GridClientRpc.java | 3 +- .../grid/GridScrollToRowWithDetails.java | 133 ++++++++++++ .../grid/GridScrollToRowWithDetailsTest.java | 224 +++++++++++++++++++++ .../src/com/vaadin/tests/util/PersonContainer.java | 6 +- 7 files changed, 523 insertions(+), 13 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetails.java create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetailsTest.java (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 51e986933c..d17f378611 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -477,7 +477,15 @@ public class GridConnector extends AbstractHasComponentsConnector implements } @SuppressWarnings("boxing") - private class DetailsConnectorFetcher implements DeferredWorker { + private static class DetailsConnectorFetcher implements DeferredWorker { + + private static final int FETCH_TIMEOUT_MS = 5000; + + public interface Listener { + void fetchHasBeenScheduled(int id); + + void fetchHasReturned(int id); + } /** A flag making sure that we don't call scheduleFinally many times. */ private boolean fetcherHasBeenCalled = false; @@ -493,14 +501,26 @@ public class GridConnector extends AbstractHasComponentsConnector implements public void execute() { int currentFetchId = detailsFetchCounter++; pendingFetches.add(currentFetchId); - getRpcProxy(GridServerRpc.class).sendDetailsComponents( - currentFetchId); + rpc.sendDetailsComponents(currentFetchId); fetcherHasBeenCalled = false; + if (listener != null) { + listener.fetchHasBeenScheduled(currentFetchId); + } + assert assertRequestDoesNotTimeout(currentFetchId); } }; + private DetailsConnectorFetcher.Listener listener = null; + + private final GridServerRpc rpc; + + public DetailsConnectorFetcher(GridServerRpc rpc) { + assert rpc != null : "RPC was null"; + this.rpc = rpc; + } + public void schedule() { if (!fetcherHasBeenCalled) { Scheduler.get().scheduleFinally(lazyDetailsFetcher); @@ -509,10 +529,17 @@ public class GridConnector extends AbstractHasComponentsConnector implements } public void responseReceived(int fetchId) { - /* Ignore negative fetchIds (they're pushed, not fetched) */ - if (fetchId >= 0) { - boolean success = pendingFetches.remove(fetchId); - assert success : "Received a response with an unidentified fetch id"; + + if (fetchId < 0) { + /* Ignore negative fetchIds (they're pushed, not fetched) */ + return; + } + + boolean success = pendingFetches.remove(fetchId); + assert success : "Received a response with an unidentified fetch id"; + + if (listener != null) { + listener.fetchHasReturned(fetchId); } } @@ -534,9 +561,113 @@ public class GridConnector extends AbstractHasComponentsConnector implements assert !pendingFetches.contains(fetchId) : "Fetch id " + fetchId + " timed out."; } - }.schedule(1000); + }.schedule(FETCH_TIMEOUT_MS); return true; } + + public void setListener(DetailsConnectorFetcher.Listener listener) { + // if more are needed, feel free to convert this into a collection. + this.listener = listener; + } + } + + /** + * The functionality that makes sure that the scroll position is still kept + * up-to-date even if more details are being fetched lazily. + */ + private class LazyDetailsScrollAdjuster implements DeferredWorker { + + private static final int SCROLL_TO_END_ID = -2; + private static final int NO_SCROLL_SCHEDULED = -1; + + private class ScrollStopChecker implements DeferredWorker { + private final ScheduledCommand checkCommand = new ScheduledCommand() { + @Override + public void execute() { + isScheduled = false; + if (queuedFetches.isEmpty()) { + currentRow = NO_SCROLL_SCHEDULED; + destination = null; + } + } + }; + + private boolean isScheduled = false; + + public void schedule() { + if (isScheduled) { + return; + } + Scheduler.get().scheduleDeferred(checkCommand); + isScheduled = true; + } + + @Override + public boolean isWorkPending() { + return isScheduled; + } + } + + private DetailsConnectorFetcher.Listener fetcherListener = new DetailsConnectorFetcher.Listener() { + @Override + @SuppressWarnings("boxing") + public void fetchHasBeenScheduled(int id) { + if (currentRow != NO_SCROLL_SCHEDULED) { + queuedFetches.add(id); + } + } + + @Override + @SuppressWarnings("boxing") + public void fetchHasReturned(int id) { + if (currentRow == NO_SCROLL_SCHEDULED + || queuedFetches.isEmpty()) { + return; + } + + queuedFetches.remove(id); + if (currentRow == SCROLL_TO_END_ID) { + getWidget().scrollToEnd(); + } else { + getWidget().scrollToRow(currentRow, destination); + } + + /* + * Schedule a deferred call whether we should stop adjusting for + * scrolling. + * + * This is done deferredly just because we can't be absolutely + * certain whether this most recent scrolling won't cascade into + * further lazy details loading (perhaps deferredly). + */ + scrollStopChecker.schedule(); + } + }; + + private int currentRow = NO_SCROLL_SCHEDULED; + private final Set queuedFetches = new HashSet(); + private final ScrollStopChecker scrollStopChecker = new ScrollStopChecker(); + private ScrollDestination destination; + + public LazyDetailsScrollAdjuster() { + detailsConnectorFetcher.setListener(fetcherListener); + } + + public void adjustForEnd() { + currentRow = SCROLL_TO_END_ID; + } + + public void adjustFor(int row, ScrollDestination destination) { + currentRow = row; + this.destination = destination; + } + + @Override + public boolean isWorkPending() { + return currentRow != NO_SCROLL_SCHEDULED + || !queuedFetches.isEmpty() + || scrollStopChecker.isWorkPending(); + } } /** @@ -597,7 +728,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements private final CustomDetailsGenerator customDetailsGenerator = new CustomDetailsGenerator(); - private final DetailsConnectorFetcher detailsConnectorFetcher = new DetailsConnectorFetcher(); + private final DetailsConnectorFetcher detailsConnectorFetcher = new DetailsConnectorFetcher( + getRpcProxy(GridServerRpc.class)); private final DetailsListener detailsListener = new DetailsListener() { @Override @@ -618,6 +750,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements } }; + private final LazyDetailsScrollAdjuster lazyDetailsScrollAdjuster = new LazyDetailsScrollAdjuster(); + @Override @SuppressWarnings("unchecked") public Grid getWidget() { @@ -637,6 +771,10 @@ public class GridConnector extends AbstractHasComponentsConnector implements registerRpc(GridClientRpc.class, new GridClientRpc() { @Override public void scrollToStart() { + /* + * no need for lazyDetailsScrollAdjuster, because the start is + * always 0, won't change a bit. + */ Scheduler.get().scheduleFinally(new ScheduledCommand() { @Override public void execute() { @@ -647,6 +785,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements @Override public void scrollToEnd() { + lazyDetailsScrollAdjuster.adjustForEnd(); Scheduler.get().scheduleFinally(new ScheduledCommand() { @Override public void execute() { @@ -658,6 +797,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements @Override public void scrollToRow(final int row, final ScrollDestination destination) { + lazyDetailsScrollAdjuster.adjustFor(row, destination); Scheduler.get().scheduleFinally(new ScheduledCommand() { @Override public void execute() { @@ -1266,7 +1406,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements @Override public boolean isWorkPending() { - return detailsConnectorFetcher.isWorkPending(); + return detailsConnectorFetcher.isWorkPending() + || lazyDetailsScrollAdjuster.isWorkPending(); } public DetailsListener getDetailsListener() { diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index e02b4e9f83..6d9b8ee70a 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -2042,7 +2042,7 @@ public class Escalator extends Widget implements RequiresResize, TableCellElement cellOriginal = rowElement.getCells().getItem( colIndex); - if (cellIsPartOfSpan(cellOriginal)) { + if (cellOriginal == null || cellIsPartOfSpan(cellOriginal)) { continue; } diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 69fbb41fc2..2cb7ab352a 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -250,6 +250,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * * @since 7.5.0 * @author Vaadin Ltd + * @see DetailsGenerator#NULL */ public interface DetailsGenerator extends Serializable { @@ -3968,6 +3969,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Scrolls to a certain item, using {@link ScrollDestination#ANY}. + *

+ * If the item has visible details, its size will also be taken into + * account. * * @param itemId * id of item to scroll to. @@ -3980,6 +3984,9 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Scrolls to a certain item, using user-specified scroll destination. + *

+ * If the item has visible details, its size will also be taken into + * account. * * @param itemId * id of item to scroll to. diff --git a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java index d4707f9ba5..3c6d993482 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java @@ -28,7 +28,8 @@ import com.vaadin.shared.communication.ClientRpc; public interface GridClientRpc extends ClientRpc { /** - * Command client Grid to scroll to a specific data row. + * Command client Grid to scroll to a specific data row and its (optional) + * details. * * @param row * zero-based row index. If the row index is below zero or above diff --git a/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetails.java b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetails.java new file mode 100644 index 0000000000..5659f01bdd --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetails.java @@ -0,0 +1,133 @@ +/* + * Copyright 2000-2014 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 + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.grid; + +import com.vaadin.annotations.Theme; +import com.vaadin.data.Property.ValueChangeEvent; +import com.vaadin.data.Property.ValueChangeListener; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.util.Person; +import com.vaadin.tests.util.PersonContainer; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.CheckBox; +import com.vaadin.ui.Component; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.DetailsGenerator; +import com.vaadin.ui.Grid.RowReference; +import com.vaadin.ui.Grid.SelectionMode; +import com.vaadin.ui.Label; +import com.vaadin.ui.Layout; +import com.vaadin.ui.TextField; +import com.vaadin.ui.UI; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.themes.ValoTheme; + +@Theme(ValoTheme.THEME_NAME) +public class GridScrollToRowWithDetails extends UI { + + private final DetailsGenerator detailsGenerator = new DetailsGenerator() { + @Override + public Component getDetails(RowReference rowReference) { + Person person = (Person) rowReference.getItemId(); + Label label = new Label(person.getFirstName() + " " + + person.getLastName()); + label.setHeight("30px"); + return label; + } + }; + + private TextField numberTextField; + private Grid grid; + + @Override + protected void init(VaadinRequest request) { + + Layout layout = new VerticalLayout(); + + grid = new Grid(PersonContainer.createWithTestData(1000)); + grid.setSelectionMode(SelectionMode.NONE); + layout.addComponent(grid); + + final CheckBox checkbox = new CheckBox("Details generator"); + checkbox.addValueChangeListener(new ValueChangeListener() { + @Override + @SuppressWarnings("boxing") + public void valueChange(ValueChangeEvent event) { + if (checkbox.getValue()) { + grid.setDetailsGenerator(detailsGenerator); + } else { + grid.setDetailsGenerator(DetailsGenerator.NULL); + } + } + }); + layout.addComponent(checkbox); + + numberTextField = new TextField("Row"); + numberTextField.setImmediate(false); + layout.addComponent(numberTextField); + + layout.addComponent(new Button("Toggle", new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + toggle(); + } + })); + + layout.addComponent(new Button("Scroll to", new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + scrollTo(); + } + })); + + layout.addComponent(new Button("Toggle and scroll", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + toggle(); + scrollTo(); + } + })); + layout.addComponent(new Button("Scroll and toggle", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + scrollTo(); + toggle(); + } + })); + + setContent(layout); + } + + private void toggle() { + Object itemId = getItemId(); + boolean isVisible = grid.isDetailsVisible(itemId); + grid.setDetailsVisible(itemId, !isVisible); + } + + private void scrollTo() { + grid.scrollTo(getItemId()); + } + + private Object getItemId() { + int row = Integer.parseInt(numberTextField.getValue()); + Object itemId = grid.getContainerDataSource().getIdByIndex(row); + return itemId; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetailsTest.java b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetailsTest.java new file mode 100644 index 0000000000..b6ecd3f6e2 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridScrollToRowWithDetailsTest.java @@ -0,0 +1,224 @@ +/* + * Copyright 2000-2014 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 + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.grid; + +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.JavascriptExecutor; +import org.openqa.selenium.WebElement; + +import com.vaadin.shared.ui.grid.Range; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.CheckBoxElement; +import com.vaadin.testbench.elements.TextFieldElement; +import com.vaadin.tests.components.grid.basicfeatures.element.CustomGridElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridScrollToRowWithDetailsTest extends MultiBrowserTest { + + private static class Param { + private final int rowIndex; + private final boolean useGenerator; + private final boolean scrollFirstToBottom; + private final int scrollTarget; + + public Param(int rowIndex, boolean useGenerator, + boolean scrollFirstToBottom, int scrollTarget) { + this.rowIndex = rowIndex; + this.useGenerator = useGenerator; + this.scrollFirstToBottom = scrollFirstToBottom; + this.scrollTarget = Math.max(0, scrollTarget); + } + + public int getRowIndex() { + return rowIndex; + } + + public boolean useGenerator() { + return useGenerator; + } + + public boolean scrollFirstToBottom() { + return scrollFirstToBottom; + } + + public int getScrollTarget() { + return scrollTarget; + } + + @Override + public String toString() { + return "Param [rowIndex=" + getRowIndex() + ", useGenerator=" + + useGenerator() + ", scrollFirstToBottom=" + + scrollFirstToBottom() + ", scrollTarget=" + + getScrollTarget() + "]"; + } + } + + public static Collection parameters() { + List data = new ArrayList(); + + int[][] params = new int[][] {// @formatter:off + // row, top+noGen, top+gen, bot+noGen, bot+gen + { 0, 0, 0, 0, 0 }, + { 500, 18741, 18723, 19000, 19000 }, + { 999, 37703, 37685, 37703, 37685 }, + }; + // @formatter:on + + for (int i[] : params) { + int rowIndex = i[0]; + int targetTopScrollWithoutGenerator = i[1]; + int targetTopScrollWithGenerator = i[2]; + int targetBottomScrollWithoutGenerator = i[3]; + int targetBottomScrollWithGenerator = i[4]; + + data.add(new Param(rowIndex, false, false, + targetTopScrollWithoutGenerator)); + data.add(new Param(rowIndex, true, false, + targetTopScrollWithGenerator)); + data.add(new Param(rowIndex, false, true, + targetBottomScrollWithoutGenerator)); + data.add(new Param(rowIndex, true, true, + targetBottomScrollWithGenerator)); + } + + return data; + } + + @Before + public void setUp() { + setDebug(true); + } + + @Test + public void toggleAndScroll() throws Throwable { + for (Param param : parameters()) { + try { + openTestURL(); + useGenerator(param.useGenerator()); + scrollToBottom(param.scrollFirstToBottom()); + + // the tested method + toggleAndScroll(param.getRowIndex()); + + Range allowedRange = Range.withLength( + param.getScrollTarget() - 5, 10); + assertTrue( + allowedRange + " does not contain " + getScrollTop(), + allowedRange.contains(getScrollTop())); + } catch (Throwable t) { + throw new Throwable("" + param, t); + } + } + } + + @Test + public void scrollAndToggle() throws Throwable { + for (Param param : parameters()) { + try { + openTestURL(); + useGenerator(param.useGenerator()); + scrollToBottom(param.scrollFirstToBottom()); + + // the tested method + scrollAndToggle(param.getRowIndex()); + + Range allowedRange = Range.withLength( + param.getScrollTarget() - 5, 10); + assertTrue( + allowedRange + " does not contain " + getScrollTop(), + allowedRange.contains(getScrollTop())); + } catch (Throwable t) { + throw new Throwable("" + param, t); + } + } + } + + private void scrollToBottom(boolean scrollFirstToBottom) { + if (scrollFirstToBottom) { + executeScript("arguments[0].scrollTop = 9999999", + getVerticalScrollbar()); + } + } + + private void useGenerator(boolean use) { + CheckBoxElement checkBox = $(CheckBoxElement.class).first(); + boolean isChecked = isCheckedValo(checkBox); + if (use != isChecked) { + clickValo(checkBox); + } + } + + @SuppressWarnings("boxing") + private boolean isCheckedValo(CheckBoxElement checkBoxElement) { + WebElement checkbox = checkBoxElement.findElement(By.tagName("input")); + Object value = executeScript("return arguments[0].checked;", checkbox); + return (Boolean) value; + } + + private void clickValo(CheckBoxElement checkBoxElement) { + checkBoxElement.findElement(By.tagName("label")).click(); + } + + private Object executeScript(String string, Object... param) { + return ((JavascriptExecutor) getDriver()).executeScript(string, param); + } + + private void scrollAndToggle(int row) { + setRow(row); + getScrollAndToggle().click(); + } + + private void toggleAndScroll(int row) { + setRow(row); + getToggleAndScroll().click(); + } + + private ButtonElement getScrollAndToggle() { + return $(ButtonElement.class).caption("Scroll and toggle").first(); + } + + private ButtonElement getToggleAndScroll() { + return $(ButtonElement.class).caption("Toggle and scroll").first(); + } + + private void setRow(int row) { + $(TextFieldElement.class).first().setValue(String.valueOf(row)); + } + + private CustomGridElement getGrid() { + return $(CustomGridElement.class).first(); + } + + private int getScrollTop() { + return ((Long) executeScript("return arguments[0].scrollTop;", + getVerticalScrollbar())).intValue(); + } + + private WebElement getVerticalScrollbar() { + WebElement scrollBar = getGrid().findElement( + By.className("v-grid-scroller-vertical")); + return scrollBar; + } +} diff --git a/uitest/src/com/vaadin/tests/util/PersonContainer.java b/uitest/src/com/vaadin/tests/util/PersonContainer.java index 611e5d3adb..709086be29 100644 --- a/uitest/src/com/vaadin/tests/util/PersonContainer.java +++ b/uitest/src/com/vaadin/tests/util/PersonContainer.java @@ -32,10 +32,14 @@ public class PersonContainer extends BeanItemContainer implements } public static PersonContainer createWithTestData() { + return createWithTestData(100); + } + + public static PersonContainer createWithTestData(int size) { PersonContainer c = null; Random r = new Random(0); c = new PersonContainer(); - for (int i = 0; i < 100; i++) { + for (int i = 0; i < size; i++) { Person p = new Person(); p.setFirstName(TestDataGenerator.getFirstName(r)); p.setLastName(TestDataGenerator.getLastName(r)); -- cgit v1.2.3 From 0e7755958b46434185cb1e6e2ec8aa6932b32f34 Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Mon, 23 Mar 2015 13:42:07 +0200 Subject: API for column hiding toggle caption in Grid (#17272) Fixes column toggle not getting a caption when a hidable column is added. Fixes column toggle not getting a caption on columns with widget in header. Change-Id: Ie10ada793a3635302603f684f232cadaef74a982 --- .../vaadin/client/connectors/GridConnector.java | 1 + client/src/com/vaadin/client/widgets/Grid.java | 52 +++++++++++++++++----- server/src/com/vaadin/ui/Grid.java | 46 ++++++++++++++++++- .../com/vaadin/shared/ui/grid/GridColumnState.java | 3 ++ .../grid/basicfeatures/GridBasicFeatures.java | 31 ++++++++++--- .../grid/basicfeatures/GridColumnReorderTest.java | 2 +- .../server/GridColumnVisibilityTest.java | 50 ++++++++++++++++++--- 7 files changed, 159 insertions(+), 26 deletions(-) (limited to 'server/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 0e2ee0046b..5554664566 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -1197,6 +1197,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements column.setHidden(state.hidden); column.setHidable(state.hidable); + column.setHidingToggleCaption(state.hidingToggleCaption); column.setEditable(state.editable); column.setEditorConnector((AbstractFieldConnector) state.editorConnector); diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 20b8844623..f9c6ed28fe 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -3096,7 +3096,7 @@ public class Grid extends ResizeComposite implements column.setHidden(!event.getValue(), true); } }); - updateColumnHidingToggleCaption(column, toggle); + updateHidingToggleCaption(column, toggle); columnToHidingToggleMap.put(column, toggle); return toggle; } @@ -3133,22 +3133,20 @@ public class Grid extends ResizeComposite implements hasValue.setStyleName("hidden", hidden); } - private void updateColumnHidingToggleCaption(Column column) { - updateColumnHidingToggleCaption(column, + private void updateHidingToggleCaption(Column column) { + updateHidingToggleCaption(column, columnToHidingToggleMap.get(column)); } - private void updateColumnHidingToggleCaption(Column column, + private void updateHidingToggleCaption(Column column, ToggleButton toggle) { - String caption = column.headerCaption; - if (caption == null || caption.isEmpty()) { - // TODO what if the content is a widget? - HeaderCell cell = getDefaultHeaderRow().getCell(column); - caption = cell.getText(); + String caption = column.getHidingToggleCaption(); + if (caption == null) { + caption = column.headerCaption; + // the caption might still be null, but that is the users fault } toggle.setText(caption); } - } /** @@ -3782,6 +3780,8 @@ public class Grid extends ResizeComposite implements private String headerCaption = ""; + private String hidingToggleCaption = null; + private double minimumWidthPx = GridConstants.DEFAULT_MIN_WIDTH; private double maximumWidthPx = GridConstants.DEFAULT_MAX_WIDTH; private int expandRatio = GridConstants.DEFAULT_EXPAND_RATIO; @@ -3891,7 +3891,7 @@ public class Grid extends ResizeComposite implements if (row != null) { row.getCell(this).setText(headerCaption); if (isHidable()) { - grid.columnHider.updateColumnHidingToggleCaption(this); + grid.columnHider.updateHidingToggleCaption(this); } } } @@ -4143,6 +4143,36 @@ public class Grid extends ResizeComposite implements return hidable; } + /** + * Sets the hiding toggle's caption for this column. Shown in the toggle + * for this column in the grid's sidebar when the column is + * {@link #isHidable() hidable}. + *

+ * Defaults to null, when will use whatever is set with + * {@link #setHeaderCaption(String)}. + * + * @since + * @param hidingToggleCaption + * the caption for the hiding toggle for this column + */ + public void setHidingToggleCaption(String hidingToggleCaption) { + this.hidingToggleCaption = hidingToggleCaption; + if (isHidable()) { + grid.columnHider.updateHidingToggleCaption(this); + } + } + + /** + * Gets the hiding toggle caption for this column. + * + * @since + * @see #setHidingToggleCaption(String) + * @return the hiding toggle's caption for this column + */ + public String getHidingToggleCaption() { + return hidingToggleCaption; + } + @Override public String toString() { String details = ""; diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 2cb7ab352a..cd97e90e2e 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -2285,6 +2285,46 @@ public class Grid extends AbstractComponent implements SelectionNotifier, return this; } + /** + * Gets the caption of the hiding toggle for this column. + * + * @since + * @see #setHidingToggleCaption(String) + * @return the caption for the hiding toggle for this column + * @throws IllegalStateException + * if the column is no longer attached to any grid + */ + public String getHidingToggleCaption() throws IllegalStateException { + checkColumnIsAttached(); + return state.hidingToggleCaption; + } + + /** + * Sets the caption of the hiding toggle for this column. Shown in the + * toggle for this column in the grid's sidebar when the column is + * {@link #isHidable() hidable}. + *

+ * By default, before triggering this setter, a user friendly version of + * the column's {@link #getPropertyId() property id} is used. + *

+ * NOTE: setting this to null or empty string + * might cause the hiding toggle to not render correctly. + * + * @since + * @param hidingToggleCaption + * the text to show in the column hiding toggle + * @return the column itself + * @throws IllegalStateException + * if the column is no longer attached to any grid + */ + public Column setHidingToggleCaption(String hidingToggleCaption) + throws IllegalStateException { + checkColumnIsAttached(); + state.hidingToggleCaption = hidingToggleCaption; + grid.markAsDirty(); + return this; + } + /** * Returns the width (in pixels). By default a column is 100px wide. * @@ -3860,8 +3900,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier, header.addColumn(datasourcePropertyId); footer.addColumn(datasourcePropertyId); - column.setHeaderCaption(SharedUtil.propertyIdToHumanFriendly(String - .valueOf(datasourcePropertyId))); + String humanFriendlyPropertyId = SharedUtil + .propertyIdToHumanFriendly(String.valueOf(datasourcePropertyId)); + column.setHeaderCaption(humanFriendlyPropertyId); + column.setHidingToggleCaption(humanFriendlyPropertyId); return column; } diff --git a/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java b/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java index b966c53352..5aa9ea9b65 100644 --- a/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java +++ b/shared/src/com/vaadin/shared/ui/grid/GridColumnState.java @@ -82,4 +82,7 @@ public class GridColumnState implements Serializable { /** Can the column be hidden by the UI. */ public boolean hidable = false; + + /** The caption for the column hiding toggle. */ + public String hidingToggleCaption; } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java index d3b1237cf9..6f4c7df38c 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java @@ -768,17 +768,23 @@ public class GridBasicFeatures extends AbstractComponentTest { new Command() { boolean wasHidable; + boolean wasHidden; + String wasColumnHidingToggleCaption; @Override public void execute(Grid grid, String value, Object data) { String columnProperty = getColumnProperty((Integer) data); - if (grid.getColumn(columnProperty) == null) { - grid.addColumn(columnProperty); - grid.getColumn(columnProperty).setHidable( - wasHidable); + Column column = grid.getColumn(columnProperty); + if (column == null) { + column = grid.addColumn(columnProperty); + column.setHidable(wasHidable); + column.setHidden(wasHidden); + column.setHidingToggleCaption(wasColumnHidingToggleCaption); } else { - wasHidable = grid.getColumn(columnProperty) - .isHidable(); + wasHidable = column.isHidable(); + wasHidden = column.isHidden(); + wasColumnHidingToggleCaption = column + .getHidingToggleCaption(); grid.removeColumn(columnProperty); } } @@ -840,6 +846,19 @@ public class GridBasicFeatures extends AbstractComponentTest { grid.getColumn(propertyId).setHidden(hidden); } }, getColumnProperty(c)); + createClickAction("Change hiding toggle caption", + getColumnProperty(c), new Command() { + int count = 0; + + @Override + public void execute(Grid grid, String value, Object data) { + final String columnProperty = getColumnProperty((Integer) data); + grid.getColumn(columnProperty) + .setHidingToggleCaption( + columnProperty + " caption " + + count++); + } + }, null, c); createCategory("Column " + c + " Width", getColumnProperty(c)); diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnReorderTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnReorderTest.java index 68ba5f5c54..d779a5c81a 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnReorderTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnReorderTest.java @@ -369,7 +369,7 @@ public class GridColumnReorderTest extends GridBasicClientFeaturesTest { assertColumnHeaderOrder(1, 3, 4, 5, 2); // when then - dragAndDropColumnHeader(0, 1, 3, CellSide.RIGHT); + dragAndDropColumnHeader(0, 1, 4, CellSide.LEFT); assertColumnHeaderOrder(1, 4, 3, 5, 2); dragAndDropColumnHeader(0, 2, 4, CellSide.LEFT); diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java index 37eda1c28f..7942650576 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnVisibilityTest.java @@ -24,7 +24,6 @@ import static org.junit.Assert.assertTrue; import java.util.List; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.openqa.selenium.By; import org.openqa.selenium.WebElement; @@ -139,12 +138,15 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest { } @Test - @Ignore - // known issue, column caption not passed to toggle when added again - public void testColumnHiding_whenHidableColumnAdded_toggleAdded() { + public void testColumnHiding_whenHidableColumnAdded_toggleWithCorrectCaptionAdded() { selectMenuPath("Component", "Size", "Width", "100%"); toggleColumnHidable(0); toggleColumnHidable(1); + toggleColumnHidingToggleCaptionChange(0); + getSidebarOpenButton().click(); + assertEquals("Column 0 caption 0", getColumnHidingToggle(0).getText()); + getSidebarOpenButton().click(); + addRemoveColumn(0); addRemoveColumn(4); addRemoveColumn(5); @@ -163,7 +165,43 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest { assertColumnHeaderOrder(1, 2, 3, 11, 0); getSidebarOpenButton().click(); - assertNotNull(getColumnHidingToggle(0)); + assertEquals("Column 0 caption 0", getColumnHidingToggle(0).getText()); + } + + @Test + public void testColumnHidingToggleCaption_settingToggleCaption_updatesToggle() { + toggleColumnHidable(1); + getSidebarOpenButton().click(); + assertEquals("column 1", getGridElement().getHeaderCell(0, 1).getText() + .toLowerCase()); + assertEquals("Column 1", getColumnHidingToggle(1).getText()); + + toggleColumnHidingToggleCaptionChange(1); + assertEquals("column 1", getGridElement().getHeaderCell(0, 1).getText() + .toLowerCase()); + assertEquals("Column 1 caption 0", getColumnHidingToggle(1).getText()); + + toggleColumnHidingToggleCaptionChange(1); + assertEquals("Column 1 caption 1", getColumnHidingToggle(1).getText()); + } + + @Test + public void testColumnHidingToggleCaption_settingWidgetToHeader_toggleCaptionStays() { + toggleColumnHidable(1); + getSidebarOpenButton().click(); + assertEquals("column 1", getGridElement().getHeaderCell(0, 1).getText() + .toLowerCase()); + assertEquals("Column 1", getColumnHidingToggle(1).getText()); + + selectMenuPath("Component", "Columns", "Column 1", "Header Type", + "Widget Header"); + + assertEquals("Column 1", getColumnHidingToggle(1).getText()); + } + + private void toggleColumnHidingToggleCaptionChange(int index) { + selectMenuPath("Component", "Columns", "Column " + index, + "Change hiding toggle caption"); } @Test @@ -264,7 +302,7 @@ public class GridColumnVisibilityTest extends GridBasicFeaturesTest { List elements = sidebar.findElements(By .className("column-hiding-toggle")); for (WebElement e : elements) { - if (("Column " + columnIndex).equalsIgnoreCase(e.getText())) { + if ((e.getText().toLowerCase()).startsWith("column " + columnIndex)) { return e; } } -- cgit v1.2.3 From 4c8ac1b008a9221ecd9e9133dd598435ae4dd8fd Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Thu, 16 Apr 2015 09:50:30 +0300 Subject: Grid.Column setHidable and setHidden return the Column #17478 Change-Id: I720b66d14a151964cf071deee380704c73f88744 --- server/src/com/vaadin/ui/Grid.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 1d7eac7b3c..ed526bc63c 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -3117,13 +3117,15 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * @param hidden * true to hide the column, false * to show + * @return this column */ - public void setHidden(boolean hidden) { + public Column setHidden(boolean hidden) { if (hidden != getState().hidden) { getState().hidden = hidden; grid.markAsDirty(); grid.fireColumnVisibilityChangeEvent(this, hidden, false); } + return this; } /** @@ -3148,10 +3150,14 @@ public class Grid extends AbstractComponent implements SelectionNotifier, * @param hidable * true iff the column may be hidable by the * user via UI interaction + * @return this column */ - public void setHidable(boolean hidable) { - getState().hidable = hidable; - grid.markAsDirty(); + public Column setHidable(boolean hidable) { + if (hidable != getState().hidable) { + getState().hidable = hidable; + grid.markAsDirty(); + } + return this; } /** -- cgit v1.2.3 From ebd18795aab03617a1ae39926d8569f4daef59d8 Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Thu, 16 Apr 2015 10:48:59 +0300 Subject: Declarative support and tests for Grid's 7.5 features (#17481) - Grid.Column.hidden and Grid.Column.hidable - Grid.columnReorderingAllowed Change-Id: Iee2e3ff7472bceef314403b750549c99e26a9546 --- server/src/com/vaadin/ui/Grid.java | 13 ++++++++++++- .../grid/declarative/GridColumnDeclarativeTest.java | 4 ++++ .../grid/declarative/GridDeclarativeAttributeTest.java | 3 ++- 3 files changed, 18 insertions(+), 2 deletions(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index ed526bc63c..9346248dd9 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -3200,6 +3200,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier, getMaximumWidth(), def.maxWidth, Double.class); DesignAttributeHandler.writeAttribute("expand", attributes, getExpandRatio(), def.expandRatio, Integer.class); + DesignAttributeHandler.writeAttribute("hidable", attributes, + isHidable(), def.hidable, boolean.class); + DesignAttributeHandler.writeAttribute("hidden", attributes, + isHidden(), def.hidden, boolean.class); DesignAttributeHandler.writeAttribute("property-id", attributes, getPropertyId(), null, Object.class); } @@ -3225,7 +3229,14 @@ public class Grid extends AbstractComponent implements SelectionNotifier, setEditable(DesignAttributeHandler.readAttribute("editable", attributes, boolean.class)); } - + if (design.hasAttr("hidable")) { + setHidable(DesignAttributeHandler.readAttribute("hidable", + attributes, boolean.class)); + } + if (design.hasAttr("hidden")) { + setHidden(DesignAttributeHandler.readAttribute("hidden", + attributes, boolean.class)); + } // Read size info where necessary. if (design.hasAttr("width")) { setWidth(DesignAttributeHandler.readAttribute("width", diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java index 1c22a69571..798988bb0d 100644 --- a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java @@ -28,6 +28,8 @@ public class GridColumnDeclarativeTest extends GridDeclarativeTestBase { + " " + " " + " " + + " " + + " " + "" // + "" // + ""; @@ -37,6 +39,8 @@ public class GridColumnDeclarativeTest extends GridDeclarativeTestBase { .setExpandRatio(2).setSortable(false); grid.addColumn("Column3", String.class).setMinimumWidth(15) .setExpandRatio(1).setEditable(false); + grid.addColumn("Column4", String.class).setHidable(true); + grid.addColumn("Column5", String.class).setHidden(true); // Remove the default header grid.removeHeaderRow(grid.getDefaultHeaderRow()); diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeAttributeTest.java b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeAttributeTest.java index e17f3ee0be..d27968cb07 100644 --- a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeAttributeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeAttributeTest.java @@ -38,7 +38,7 @@ public class GridDeclarativeAttributeTest extends DeclarativeTestBase { public void testBasicAttributes() { String design = ""; + + "editor-save-caption='Tallenna' editor-cancel-caption='Peruuta' column-reordering-allowed=true>"; Grid grid = new Grid(); grid.setEditorEnabled(true); @@ -47,6 +47,7 @@ public class GridDeclarativeAttributeTest extends DeclarativeTestBase { grid.setFrozenColumnCount(-1); grid.setEditorSaveCaption("Tallenna"); grid.setEditorCancelCaption("Peruuta"); + grid.setColumnReorderingAllowed(true); testRead(design, grid); testWrite(design, grid); -- cgit v1.2.3 From 6ed28680346c648a6b4e974568a56f6d4d0e000b Mon Sep 17 00:00:00 2001 From: Pekka Hyvönen Date: Thu, 16 Apr 2015 14:25:55 +0300 Subject: Declarative support and test for Grid.Column.hidingToggleCaption #17481 Change-Id: Ic20686649b28530160498742e78f69074e32e596 --- server/src/com/vaadin/ui/Grid.java | 8 ++++++++ .../component/grid/declarative/GridColumnDeclarativeTest.java | 8 ++++++-- .../component/grid/declarative/GridDeclarativeTestBase.java | 7 ++++++- 3 files changed, 20 insertions(+), 3 deletions(-) (limited to 'server/src') diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 9346248dd9..a7ff15c8dd 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -3204,6 +3204,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier, isHidable(), def.hidable, boolean.class); DesignAttributeHandler.writeAttribute("hidden", attributes, isHidden(), def.hidden, boolean.class); + DesignAttributeHandler.writeAttribute("hiding-toggle-caption", + attributes, getHidingToggleCaption(), + SharedUtil.propertyIdToHumanFriendly(getPropertyId()), + String.class); DesignAttributeHandler.writeAttribute("property-id", attributes, getPropertyId(), null, Object.class); } @@ -3237,6 +3241,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier, setHidden(DesignAttributeHandler.readAttribute("hidden", attributes, boolean.class)); } + if (design.hasAttr("hiding-toggle-caption")) { + setHidingToggleCaption(DesignAttributeHandler.readAttribute( + "hiding-toggle-caption", attributes, String.class)); + } // Read size info where necessary. if (design.hasAttr("width")) { setWidth(DesignAttributeHandler.readAttribute("width", diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java index 798988bb0d..6cf9ef55ad 100644 --- a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java +++ b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridColumnDeclarativeTest.java @@ -28,7 +28,7 @@ public class GridColumnDeclarativeTest extends GridDeclarativeTestBase { + " " + " " + " " - + " " + + " " + " " + "" // + "" // @@ -39,7 +39,8 @@ public class GridColumnDeclarativeTest extends GridDeclarativeTestBase { .setExpandRatio(2).setSortable(false); grid.addColumn("Column3", String.class).setMinimumWidth(15) .setExpandRatio(1).setEditable(false); - grid.addColumn("Column4", String.class).setHidable(true); + grid.addColumn("Column4", String.class).setHidable(true) + .setHidingToggleCaption("col 4"); grid.addColumn("Column5", String.class).setHidden(true); // Remove the default header @@ -57,6 +58,7 @@ public class GridColumnDeclarativeTest extends GridDeclarativeTestBase { + " " + " " // property-id="property-1" + " " + + " " // property-id="property-3" + "" // + ""; Grid grid = new Grid(); @@ -65,6 +67,8 @@ public class GridColumnDeclarativeTest extends GridDeclarativeTestBase { .setExpandRatio(2); grid.addColumn("Column3", String.class).setMinimumWidth(15) .setExpandRatio(1); + grid.addColumn("property-3", String.class).setHidable(true) + .setHidden(true).setHidingToggleCaption("col 4"); testRead(design, grid); } diff --git a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeTestBase.java b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeTestBase.java index 2a4b3f01fc..9424d89ecf 100644 --- a/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeTestBase.java +++ b/server/tests/src/com/vaadin/tests/server/component/grid/declarative/GridDeclarativeTestBase.java @@ -147,7 +147,12 @@ public class GridDeclarativeTestBase extends DeclarativeTestBase { col2.isSortable()); assertEquals(baseError + "Editable", col1.isEditable(), col2.isEditable()); - + assertEquals(baseError + "Hidable", col1.isHidable(), + col2.isHidable()); + assertEquals(baseError + "Hidden", col1.isHidden(), col2.isHidden()); + assertEquals(baseError + "HidingToggleCaption", + col1.getHidingToggleCaption(), + col2.getHidingToggleCaption()); } } } -- cgit v1.2.3