From f805482c6013cebdb401341da37614fd664f85c7 Mon Sep 17 00:00:00 2001 From: Aleksi Hietanen Date: Tue, 21 Nov 2017 14:25:42 +0200 Subject: Fix issues from API review for 8.2 (#10342) * Rename HierarchicalDataCommunicator#getMapper to getHierarchyMapper * Make rpc field in Notification private * Change DropIndexCalculator.ALWAYS_DROP_TO_END to a generic static method * Move EditorImpl#editRow documentation to the interface level * Correct GridDragEndEvent, GridDragStartEvent constructor javadocs * Revert SharedState.registeredEventListeners to a Set * Rename GridDropTarget dropAllowedOnSortedGridRows * Rename ColumnState.contentMode to tooltipContentMode --- .../provider/HierarchicalDataCommunicator.java | 6 ++--- .../main/java/com/vaadin/event/EventRouter.java | 6 ++--- server/src/main/java/com/vaadin/ui/Grid.java | 6 ++--- .../src/main/java/com/vaadin/ui/Notification.java | 16 ++++++------- .../ui/components/grid/DropIndexCalculator.java | 11 +++++---- .../java/com/vaadin/ui/components/grid/Editor.java | 16 ++++++++++--- .../com/vaadin/ui/components/grid/EditorImpl.java | 16 ------------- .../ui/components/grid/GridDragEndEvent.java | 2 +- .../ui/components/grid/GridDragStartEvent.java | 2 +- .../vaadin/ui/components/grid/GridDropTarget.java | 18 +++++++-------- .../vaadin/ui/components/grid/GridRowDragger.java | 2 +- .../tests/components/grid/GridDropTargetTest.java | 26 +++++++++++----------- .../component/grid/GridRowDraggerOneGridTest.java | 6 ++--- .../component/grid/GridRowDraggerTwoGridsTest.java | 6 ++--- 14 files changed, 68 insertions(+), 71 deletions(-) (limited to 'server') diff --git a/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java b/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java index f45bae9fea..ffdcf042c9 100644 --- a/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java +++ b/server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java @@ -323,11 +323,11 @@ public class HierarchicalDataCommunicator extends DataCommunicator { } /** - * Returns active {@code HierarchyMapper} + * Returns the {@code HierarchyMapper} used by this data communicator. * - * @return the mapper + * @return the hierarchy mapper used by this data communicator */ - protected HierarchyMapper getMapper() { + protected HierarchyMapper getHierarchyMapper() { return mapper; } } diff --git a/server/src/main/java/com/vaadin/event/EventRouter.java b/server/src/main/java/com/vaadin/event/EventRouter.java index b4dd43d9f4..f381e1875d 100644 --- a/server/src/main/java/com/vaadin/event/EventRouter.java +++ b/server/src/main/java/com/vaadin/event/EventRouter.java @@ -120,9 +120,10 @@ public class EventRouter implements MethodEventSource { .addRegisteredEventListener(state, eventIdentifier); return () -> { - registration.remove(); - listenerList.remove(listenerMethod); + if (!hasListeners(eventType)) { + registration.remove(); + } }; } @@ -313,5 +314,4 @@ public class EventRouter implements MethodEventSource { } return listeners; } - } diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index d79fad7ec2..8074c35f0d 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -1414,7 +1414,7 @@ public class Grid extends AbstractListing implements HasComponents, * @param cellDescriptionGenerator * the cell description generator to set, or {@code null} to * remove a previously set generator - * @param contentMode + * @param tooltipContentMode * the content mode for tooltips * @return this column * @@ -1422,9 +1422,9 @@ public class Grid extends AbstractListing implements HasComponents, */ public Column setDescriptionGenerator( DescriptionGenerator cellDescriptionGenerator, - ContentMode contentMode) { + ContentMode tooltipContentMode) { this.descriptionGenerator = cellDescriptionGenerator; - getState().contentMode = contentMode; + getState().tooltipContentMode = tooltipContentMode; getGrid().getDataCommunicator().reset(); return this; } diff --git a/server/src/main/java/com/vaadin/ui/Notification.java b/server/src/main/java/com/vaadin/ui/Notification.java index 6171c53562..82ee3b2f57 100644 --- a/server/src/main/java/com/vaadin/ui/Notification.java +++ b/server/src/main/java/com/vaadin/ui/Notification.java @@ -69,14 +69,6 @@ import com.vaadin.shared.ui.notification.NotificationState; */ public class Notification extends AbstractExtension { - /** - * The server RPC. - * - * @since 8.2 - */ - protected NotificationServerRpc rpc = () -> fireEvent( - new CloseEvent(Notification.this)); - public enum Type { HUMANIZED_MESSAGE("humanized"), WARNING_MESSAGE( "warning"), ERROR_MESSAGE("error"), TRAY_NOTIFICATION("tray"), @@ -128,6 +120,14 @@ public class Notification extends AbstractExtension { public static final int DELAY_FOREVER = -1; public static final int DELAY_NONE = 0; + /** + * The server RPC. + * + * @since 8.2 + */ + private NotificationServerRpc rpc = () -> fireEvent( + new CloseEvent(Notification.this)); + /** * Creates a "humanized" notification message. * diff --git a/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java b/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java index 5b2e4ead0d..08645d5b23 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java @@ -31,11 +31,14 @@ import java.io.Serializable; public interface DropIndexCalculator extends Serializable { /** - * Calculator for always dropping items to the end of the target grid, - * regardless of drop position. + * Returns a calculator for always dropping items to the end of the target + * grid, regardless of drop position. + * + * @return the created drop index calculator */ - @SuppressWarnings("rawtypes") - static DropIndexCalculator ALWAYS_DROP_TO_END = (event -> Integer.MAX_VALUE); + static DropIndexCalculator alwaysDropToEnd() { + return (GridDropEvent event) -> Integer.MAX_VALUE; + } /** * Called when Items are dropped onto a target grid. diff --git a/server/src/main/java/com/vaadin/ui/components/grid/Editor.java b/server/src/main/java/com/vaadin/ui/components/grid/Editor.java index 33d6fc0c4f..8238f2d15c 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/Editor.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/Editor.java @@ -104,12 +104,22 @@ public interface Editor extends Serializable { public void cancel(); /** - * Edits the selected row + * Opens the editor interface for the provided row. Scrolls the Grid to + * bring the row to view if it is not already visible. * - * @param rowNumber - * the row to edit + * Note that any cell content rendered by a WidgetRenderer will not be + * visible in the editor row. * + * @see #setEnabled(boolean) * @since 8.2 + * + * @param rowNumber + * the row number of the edited item + * @throws IllegalStateException + * if the editor is not enabled or already editing a different + * item in buffered mode + * @throws IllegalArgumentException + * if the {@code rowNumber} is not in the backing data provider */ public void editRow(int rowNumber); diff --git a/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java b/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java index 571594028e..ae89a41cab 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java @@ -267,22 +267,6 @@ public class EditorImpl extends AbstractGridExtension rpc.cancel(); } - /** - * Opens the editor interface for the provided row. Scrolls the Grid to - * bring the row to view if it is not already visible. - * - * Note that any cell content rendered by a WidgetRenderer will not be - * visible in the editor row. - * - * @param rowNumber - * the row number of the edited item - * @throws IllegalStateException - * if the editor is not enabled or already editing a different item - * in buffered mode - * @throws IllegalArgumentException - * if the {@code rowNumber} is not in the backing data provider - * @see #setEnabled(boolean) - */ @Override public void editRow(int rowNumber) throws IllegalStateException, IllegalArgumentException { diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java index 76de717978..703e63913e 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java @@ -43,7 +43,7 @@ public class GridDragEndEvent extends DragEndEvent> { * @param dropEffect * Drop effect from {@code DataTransfer.dropEffect} object. * @param draggedItems - * Set of items having been dragged. + * List of items having been dragged. */ public GridDragEndEvent(Grid source, DropEffect dropEffect, List draggedItems) { diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java index 0372235111..37507aa13a 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java @@ -43,7 +43,7 @@ public class GridDragStartEvent extends DragStartEvent> { * @param effectAllowed * Allowed effect from {@code DataTransfer.effectAllowed} object. * @param draggedItems - * Set of items being dragged. + * List of items being dragged. */ public GridDragStartEvent(Grid source, EffectAllowed effectAllowed, List draggedItems) { diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java index 632a787107..c2e1cb2411 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java @@ -41,7 +41,7 @@ public class GridDropTarget extends DropTargetExtension> { private Registration sortListenerRegistration; private DropMode cachedDropMode; - private boolean dropAllowedOnSortedGridRows = true; + private boolean dropAllowedOnRowsWhenSorted = true; /** * Extends a Grid and makes it's rows drop targets for HTML5 drag and drop. @@ -88,18 +88,18 @@ public class GridDropTarget extends DropTargetExtension> { * in this case. *

* NOTE: {@link DropMode#ON_GRID} is used automatically when the grid - * has been sorted and {@link #setDropAllowedOnSortedGridRows(boolean)} is + * has been sorted and {@link #setDropAllowedOnRowsWhenSorted(boolean)} is * {@code false} - since the drop location would not necessarily match the * correct row because of the sorting. During the sorting, any calls to this * method don't have any effect until the sorting has been removed, or - * {@link #setDropAllowedOnSortedGridRows(boolean)} is set back to + * {@link #setDropAllowedOnRowsWhenSorted(boolean)} is set back to * {@code true}. * * @param dropMode * Drop mode that describes the allowed drop locations within the * Grid's row. * @see GridDropEvent#getDropLocation() - * @see #setDropAllowedOnSortedGridRows(boolean) + * @see #setDropAllowedOnRowsWhenSorted(boolean) */ public void setDropMode(DropMode dropMode) { if (dropMode == null) { @@ -147,10 +147,10 @@ public class GridDropTarget extends DropTargetExtension> { * drops on sorted grid rows * @since 8.2 */ - public void setDropAllowedOnSortedGridRows( + public void setDropAllowedOnRowsWhenSorted( boolean dropAllowedOnSortedGridRows) { - if (this.dropAllowedOnSortedGridRows != dropAllowedOnSortedGridRows) { - this.dropAllowedOnSortedGridRows = dropAllowedOnSortedGridRows; + if (this.dropAllowedOnRowsWhenSorted != dropAllowedOnSortedGridRows) { + this.dropAllowedOnRowsWhenSorted = dropAllowedOnSortedGridRows; if (!dropAllowedOnSortedGridRows) { @@ -194,8 +194,8 @@ public class GridDropTarget extends DropTargetExtension> { * the grid * @since 8.2 */ - public boolean isDropAllowedOnSortedGridRows() { - return dropAllowedOnSortedGridRows; + public boolean isDropAllowedOnRowsWhenSorted() { + return dropAllowedOnRowsWhenSorted; } /** diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java b/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java index 960fe3ec39..09e2b22c9b 100644 --- a/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java +++ b/server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java @@ -190,7 +190,7 @@ public class GridRowDragger implements Serializable { gridDragSource = new GridDragSource<>(source); gridDropTarget = new GridDropTarget<>(target, dropMode); - gridDropTarget.setDropAllowedOnSortedGridRows(false); + gridDropTarget.setDropAllowedOnRowsWhenSorted(false); gridDragSource.addGridDragStartListener(event -> { draggedItems = event.getDraggedItems(); diff --git a/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java b/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java index d5de0ad808..cc5b31de0f 100644 --- a/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java +++ b/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java @@ -24,14 +24,14 @@ public class GridDropTargetTest { @Test public void dropAllowedOnSortedGridRows_defaultValue_isTrue() { Assert.assertTrue("Default drop allowed should be backwards compatible", - target.isDropAllowedOnSortedGridRows()); + target.isDropAllowedOnRowsWhenSorted()); } @Test public void dropAllowedOnSortedGridRows_notAllowed_changesDropModeWhenSorted() { Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(false); + target.setDropAllowedOnRowsWhenSorted(false); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); @@ -64,11 +64,11 @@ public class GridDropTargetTest { Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(false); + target.setDropAllowedOnRowsWhenSorted(false); Assert.assertEquals(DropMode.ON_GRID, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(true); + target.setDropAllowedOnRowsWhenSorted(true); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); } @@ -77,7 +77,7 @@ public class GridDropTargetTest { public void dropAllowedOnSortedGridRows_notAllowedBackToAllowed_changesBackToUserDefinedMode() { Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(false); + target.setDropAllowedOnRowsWhenSorted(false); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); @@ -85,7 +85,7 @@ public class GridDropTargetTest { Assert.assertEquals(DropMode.ON_GRID, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(true); + target.setDropAllowedOnRowsWhenSorted(true); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); @@ -98,19 +98,19 @@ public class GridDropTargetTest { public void dropAllowedOnSortedGridRows_swappingAllowedDropOnSortedOffAndOn() { Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(false); + target.setDropAllowedOnRowsWhenSorted(false); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(false); + target.setDropAllowedOnRowsWhenSorted(false); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(true); + target.setDropAllowedOnRowsWhenSorted(true); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(true); + target.setDropAllowedOnRowsWhenSorted(true); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); } @@ -119,7 +119,7 @@ public class GridDropTargetTest { public void dropAllowedOnSortedGridRows_changingDropModeWhileSorted_replacesPreviouslyCachedButDoesntOverride() { Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(false); + target.setDropAllowedOnRowsWhenSorted(false); Assert.assertEquals(DropMode.BETWEEN, target.getDropMode()); @@ -131,7 +131,7 @@ public class GridDropTargetTest { Assert.assertEquals(DropMode.ON_GRID, target.getDropMode()); Assert.assertFalse("Changing drop mode should not have any effect here", - target.isDropAllowedOnSortedGridRows()); + target.isDropAllowedOnRowsWhenSorted()); grid.clearSortOrder(); @@ -145,7 +145,7 @@ public class GridDropTargetTest { Assert.assertEquals(DropMode.ON_GRID, target.getDropMode()); - target.setDropAllowedOnSortedGridRows(true); + target.setDropAllowedOnRowsWhenSorted(true); Assert.assertEquals(DropMode.ON_TOP_OR_BETWEEN, target.getDropMode()); } diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java index 9888f42807..08b647cdde 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerOneGridTest.java @@ -15,8 +15,8 @@ import com.vaadin.data.provider.ListDataProvider; import com.vaadin.shared.ui.grid.DropLocation; import com.vaadin.ui.Grid; import com.vaadin.ui.components.grid.DropIndexCalculator; -import com.vaadin.ui.components.grid.GridRowDragger; import com.vaadin.ui.components.grid.GridDropEvent; +import com.vaadin.ui.components.grid.GridRowDragger; import com.vaadin.ui.components.grid.SourceDataProviderUpdater; public class GridRowDraggerOneGridTest { @@ -143,7 +143,7 @@ public class GridRowDraggerOneGridTest { public void alwaysDropToEndCalculator() { source.setItems("0", "1", "2"); - dragger.setDropIndexCalculator(DropIndexCalculator.ALWAYS_DROP_TO_END); + dragger.setDropIndexCalculator(DropIndexCalculator.alwaysDropToEnd()); drop("1", DropLocation.ABOVE, "0"); @@ -208,7 +208,7 @@ public class GridRowDraggerOneGridTest { public void dropOnSortedGrid_byDefault_dropsToTheEnd() { Assert.assertFalse( "Default drops on sorted grid rows should not be allowed", - dragger.getGridDropTarget().isDropAllowedOnSortedGridRows()); + dragger.getGridDropTarget().isDropAllowedOnRowsWhenSorted()); source.setItems("0", "1", "2", "3", "4"); diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java index 127365be36..6401867e3b 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridRowDraggerTwoGridsTest.java @@ -15,8 +15,8 @@ import com.vaadin.data.provider.ListDataProvider; import com.vaadin.shared.ui.grid.DropLocation; import com.vaadin.ui.Grid; import com.vaadin.ui.components.grid.DropIndexCalculator; -import com.vaadin.ui.components.grid.GridRowDragger; import com.vaadin.ui.components.grid.GridDropEvent; +import com.vaadin.ui.components.grid.GridRowDragger; import com.vaadin.ui.components.grid.SourceDataProviderUpdater; public class GridRowDraggerTwoGridsTest { @@ -186,7 +186,7 @@ public class GridRowDraggerTwoGridsTest { source.setItems("0"); target.setItems("1", "2"); - dragger.setDropIndexCalculator(DropIndexCalculator.ALWAYS_DROP_TO_END); + dragger.setDropIndexCalculator(DropIndexCalculator.alwaysDropToEnd()); drop("1", DropLocation.ABOVE, "0"); @@ -255,7 +255,7 @@ public class GridRowDraggerTwoGridsTest { public void dropOnSortedGrid_byDefault_dropsToTheEnd() { Assert.assertFalse( "Default drops on sorted grid rows should not be allowed", - dragger.getGridDropTarget().isDropAllowedOnSortedGridRows()); + dragger.getGridDropTarget().isDropAllowedOnRowsWhenSorted()); source.setItems("0", "1", "2"); target.setItems("4", "5"); -- cgit v1.2.3