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

all/src/main/templates/release-notes.html

@@ -121,7 +121,6 @@ <h2 id="incompatible">Incompatible or Behavior-altering Changes in @version-mino
         <li>Error indicators are now <tt>&lt;span class="v-errorindicator"&gt;&lt;/span&gt;</tt> elements.</li>
         <li><tt>Embedded</tt> is not a <tt>LegacyComponent</tt> anymore.</li>
         <li><tt>Notification</tt> method <tt>show</tt> returns <tt>Notification</tt>, instead of <tt>void</tt>.</li>
-        <li><tt>SharedState</tt> field <tt>registeredEventListeners</tt> is a <tt>Map</tt> instead of <tt>Set</tt>.</li>
         <li>The client side <tt>SelectionModel</tt> interface has a new method <tt>isMultiSelectionAllowed</tt>.</li>
         <li><tt>AbstractDateField</tt> is not a <tt>LegacyComponent</tt> anymore.</li>
         <li><tt>AbstractDateField</tt>.<tt>formatDate</tt> is now abstract.</li>

client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java

@@ -42,7 +42,7 @@ public abstract static class CustomColumn
             extends Column<Object, JsonObject> {
 
         private final String connectorId;
-        private ContentMode contentMode;
+        private ContentMode tooltipContentMode;
 
         CustomColumn(String connectorId) {
             this.connectorId = connectorId;
@@ -64,20 +64,20 @@ protected void setDefaultHeaderContent(HeaderCell cell) {
          *
          * @since 8.2
          */
-        public ContentMode getContentMode() {
-            return contentMode;
+        public ContentMode getTooltipContentMode() {
+            return tooltipContentMode;
         }
 
         /**
          * Sets the content mode for tooltips in this column.
          *
-         * @param contentMode
+         * @param tooltipContentMode
          *            the content mode for tooltips
          *
          * @since 8.2
          */
-        public void setContentMode(ContentMode contentMode) {
-            this.contentMode = contentMode;
+        public void setTooltipContentMode(ContentMode tooltipContentMode) {
+            this.tooltipContentMode = tooltipContentMode;
         }
     }
 
@@ -189,9 +189,9 @@ void updateEditable() {
         column.setEditable(getState().editable);
     }
 
-    @OnStateChange("contentMode")
-    void updateContentMode() {
-        column.setContentMode(getState().contentMode);
+    @OnStateChange("tooltipContentMode")
+    void updateTooltipContentMode() {
+        column.setTooltipContentMode(getState().tooltipContentMode);
     }
 
     @Override

client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java

@@ -634,7 +634,7 @@ public TooltipInfo getTooltipInfo(Element element) {
                     if (cellDescriptions != null
                             && cellDescriptions.hasKey(id)) {
                         return new TooltipInfo(cellDescriptions.getString(id),
-                                ((CustomColumn) column).getContentMode());
+                                ((CustomColumn) column).getTooltipContentMode());
                     } else if (row.hasKey(GridState.JSONKEY_ROWDESCRIPTION)) {
                         return new TooltipInfo(
                                 row.getString(GridState.JSONKEY_ROWDESCRIPTION),

client/src/main/java/com/vaadin/client/ui/AbstractConnector.java

@@ -20,7 +20,7 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
+import java.util.Set;
 import java.util.logging.Logger;
 
 import com.google.gwt.core.client.JsArrayString;
@@ -490,8 +490,8 @@ public String getResourceUrl(String key) {
      */
     @Override
     public boolean hasEventListener(String eventIdentifier) {
-        Map<String, Integer> reg = getState().registeredEventListeners;
-        return reg != null && reg.containsKey(eventIdentifier);
+        Set<String> reg = getState().registeredEventListeners;
+        return reg != null && reg.contains(eventIdentifier);
     }
 
     /**

server/src/main/java/com/vaadin/data/provider/HierarchicalDataCommunicator.java

@@ -323,11 +323,11 @@ protected <F> void setFilter(F filter) {
     }
 
     /**
-     * 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<T, ?> getMapper() {
+    protected HierarchyMapper<T, ?> getHierarchyMapper() {
         return mapper;
     }
 }

server/src/main/java/com/vaadin/event/EventRouter.java

@@ -120,9 +120,10 @@ public Registration addListener(Class<?> eventType, Object target,
                 .addRegisteredEventListener(state, eventIdentifier);
 
         return () -> {
-            registration.remove();
-
             listenerList.remove(listenerMethod);
+            if (!hasListeners(eventType)) {
+                registration.remove();
+            }
         };
     }
 
@@ -313,5 +314,4 @@ public Collection<?> getListeners(Class<?> eventType) {
         }
         return listeners;
     }
-
 }

server/src/main/java/com/vaadin/ui/Grid.java

@@ -1414,17 +1414,17 @@ public Column<T, V> setDescriptionGenerator(
          * @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
          *
          * @since 8.2
          */
         public Column<T, V> setDescriptionGenerator(
                 DescriptionGenerator<T> cellDescriptionGenerator,
-                ContentMode contentMode) {
+                ContentMode tooltipContentMode) {
             this.descriptionGenerator = cellDescriptionGenerator;
-            getState().contentMode = contentMode;
+            getState().tooltipContentMode = tooltipContentMode;
             getGrid().getDataCommunicator().reset();
             return this;
         }

server/src/main/java/com/vaadin/ui/Notification.java

@@ -69,14 +69,6 @@
  */
 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 String getStyle() {
     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.
      *

server/src/main/java/com/vaadin/ui/components/grid/DropIndexCalculator.java

@@ -31,11 +31,14 @@
 public interface DropIndexCalculator<T> 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 <T> DropIndexCalculator<T> alwaysDropToEnd() {
+        return (GridDropEvent<T> event) -> Integer.MAX_VALUE;
+    }
 
     /**
      * Called when Items are dropped onto a target grid.

server/src/main/java/com/vaadin/ui/components/grid/Editor.java

@@ -104,12 +104,22 @@ public interface Editor<T> 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);
 

server/src/main/java/com/vaadin/ui/components/grid/EditorImpl.java

@@ -267,22 +267,6 @@ public void cancel() {
         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 {

server/src/main/java/com/vaadin/ui/components/grid/GridDragEndEvent.java

@@ -43,7 +43,7 @@ public class GridDragEndEvent<T> extends DragEndEvent<Grid<T>> {
      * @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<T> source, DropEffect dropEffect,
             List<T> draggedItems) {

server/src/main/java/com/vaadin/ui/components/grid/GridDragStartEvent.java

@@ -43,7 +43,7 @@ public class GridDragStartEvent<T> extends DragStartEvent<Grid<T>> {
      * @param effectAllowed
      *            Allowed effect from {@code DataTransfer.effectAllowed} object.
      * @param draggedItems
-     *            Set of items being dragged.
+     *            List of items being dragged.
      */
     public GridDragStartEvent(Grid<T> source, EffectAllowed effectAllowed,
             List<T> draggedItems) {

server/src/main/java/com/vaadin/ui/components/grid/GridDropTarget.java

@@ -41,7 +41,7 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
 
     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 Grid<T> getGrid() {
      * in this case.
      * <p>
      * <em>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}.</em>
      *
      * @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 DropMode getDropMode() {
      *            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 @@ private void updateDropModeForSortedGrid(boolean sorted) {
      *         the grid
      * @since 8.2
      */
-    public boolean isDropAllowedOnSortedGridRows() {
-        return dropAllowedOnSortedGridRows;
+    public boolean isDropAllowedOnRowsWhenSorted() {
+        return dropAllowedOnRowsWhenSorted;
     }
 
     /**

server/src/main/java/com/vaadin/ui/components/grid/GridRowDragger.java

@@ -190,7 +190,7 @@ public GridRowDragger(Grid<T> source, Grid<T> target, DropMode dropMode) {
         gridDragSource = new GridDragSource<>(source);
 
         gridDropTarget = new GridDropTarget<>(target, dropMode);
-        gridDropTarget.setDropAllowedOnSortedGridRows(false);
+        gridDropTarget.setDropAllowedOnRowsWhenSorted(false);
 
         gridDragSource.addGridDragStartListener(event -> {
             draggedItems = event.getDraggedItems();