From: Henrik Paul Date: Tue, 10 Mar 2015 15:02:02 +0000 (+0200) Subject: Grid's Details can now be Components (#16644) X-Git-Tag: 7.5.0.alpha1~18^2~3 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a1619ee73dc18eecda22056541826a3c8bb65a5c;p=vaadin-framework.git Grid's Details can now be Components (#16644) Change-Id: If67dd2e86cf41c57f208a3691e2cb7a5a29c133c --- 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()); + } }