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 +++ 4 files changed, 81 insertions(+), 6 deletions(-) (limited to 'client/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 -- 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 'client/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 5c2da23e72e17d04e3cafc67ff1166dc313b9712 Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Wed, 18 Mar 2015 10:16:47 +0200 Subject: Fixes a bug when scrolling a Grid with details open (#16644) If the a row with an open details row was pushed out of the active row range, the component would be removed from the connector hierarchy on the server side but not on the client side. Vaadin gave a warning for this. This patch makes sure that the widget is properly deregistered when it gets outside of the active range pre-emptively. Change-Id: I2145e82a990ded31e4426e85e59edad9d4d4194f --- .../vaadin/client/connectors/GridConnector.java | 40 ++++++++++++++-------- .../client/connectors/RpcDataSourceConnector.java | 15 +++++++- .../client/data/AbstractRemoteDataSource.java | 14 ++++++++ 3 files changed, 53 insertions(+), 16 deletions(-) (limited to 'client/src') diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 70ad2504d8..1787dc5c97 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -29,7 +29,6 @@ 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.Timer; @@ -40,6 +39,7 @@ import com.vaadin.client.DeferredWorker; import com.vaadin.client.MouseEventDetailsBuilder; import com.vaadin.client.annotations.OnStateChange; import com.vaadin.client.communication.StateChangeEvent; +import com.vaadin.client.connectors.RpcDataSourceConnector.DetailsListener; import com.vaadin.client.connectors.RpcDataSourceConnector.RpcDataSource; import com.vaadin.client.data.DataSource.RowHandle; import com.vaadin.client.renderers.Renderer; @@ -107,8 +107,7 @@ import elemental.json.JsonValue; */ @Connect(com.vaadin.ui.Grid.class) public class GridConnector extends AbstractHasComponentsConnector implements - SimpleManagedLayout, RpcDataSourceConnector.DetailsListener, - DeferredWorker { + SimpleManagedLayout, DeferredWorker { private static final class CustomCellStyleGenerator implements CellStyleGenerator { @@ -534,6 +533,25 @@ public class GridConnector extends AbstractHasComponentsConnector implements private final DetailsConnectorFetcher detailsConnectorFetcher = new DetailsConnectorFetcher(); + private final DetailsListener detailsListener = new DetailsListener() { + @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); + } + + detailsConnectorFetcher.schedule(); + } + + @Override + public void closeDetails(int rowIndex) { + getWidget().setDetailsVisible(rowIndex, false); + } + }; + @Override @SuppressWarnings("unchecked") public Grid getWidget() { @@ -1144,20 +1162,12 @@ public class GridConnector extends AbstractHasComponentsConnector implements 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); - } - - detailsConnectorFetcher.schedule(); - } - @Override public boolean isWorkPending() { return detailsConnectorFetcher.isWorkPending(); } + + public DetailsListener getDetailsListener() { + return detailsListener; + } } diff --git a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java index ae4249de78..e8c7ee5286 100644 --- a/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java +++ b/client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java @@ -64,6 +64,14 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { * @see GridState#JSONKEY_DETAILS_VISIBLE */ void reapplyDetailsVisibility(int rowIndex, JsonObject row); + + /** + * Closes details for a row. + * + * @param rowIndex + * the index of the row for which to close details + */ + void closeDetails(int rowIndex); } public class RpcDataSource extends AbstractRemoteDataSource { @@ -213,6 +221,11 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { rowData.get(i)); } } + + @Override + protected void onDropFromCache(int rowIndex) { + detailsListener.closeDetails(rowIndex); + } } private final RpcDataSource dataSource = new RpcDataSource(); @@ -220,7 +233,7 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector { @Override protected void extend(ServerConnector target) { GridConnector gridConnector = (GridConnector) target; - dataSource.setDetailsListener(gridConnector); + dataSource.setDetailsListener(gridConnector.getDetailsListener()); gridConnector.setDataSource(dataSource); } } diff --git a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java index 0ac4c33c83..152b66f2ca 100644 --- a/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java +++ b/client/src/com/vaadin/client/data/AbstractRemoteDataSource.java @@ -332,9 +332,23 @@ public abstract class AbstractRemoteDataSource implements DataSource { for (int i = range.getStart(); i < range.getEnd(); i++) { T removed = indexToRowMap.remove(Integer.valueOf(i)); keyToIndexMap.remove(getRowKey(removed)); + + onDropFromCache(i); } } + /** + * A hook that can be overridden to do something whenever a row is dropped + * from the cache. + * + * @since + * @param rowIndex + * the index of the dropped row + */ + protected void onDropFromCache(int rowIndex) { + // noop + } + private void handleMissingRows(Range range) { if (range.isEmpty()) { return; -- 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 'client/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