Add new drop mode ON_GRID for GridDropTarget (#10296)

* Add new drop mode ON_GRID for GridDropTarget

Also adds a way to not accept drops on rows when the user has sorted the grid.
This way the bad UX can be avoided for showing the drop indicator for the wrong place when the grid has been sorted.
This has not been made default behavior to GridDropTarget since it would change behavior compared to 8.1.
Instead if is triggerable via API in GridDropTarget.

* Refactor sorted grid drop logic to server side

* Block setDropMode calls

Blocking setDropMode set values if the grid has been sorted and drop on
sorted rows is not allowed. The value is used once the grid is not sorted
anymore or the drops are allowed on sorted rows.

3 files changed, 281 insertions, 14 deletions

diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDropEvent.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDropEvent.java
index 11d56713f8..a137255383 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridDropEvent.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDropEvent.java
@@ -57,7 +57,7 @@ public class GridDropEvent<T> extends DropEvent<Grid<T>> {
      *            event.
      * @param dropTargetRow
      *            Target row that received the drop, or {@code null} if dropped
-     *            on empty grid
+     *            on empty grid or {@link DropMode#ON_GRID} is used
      * @param dropLocation
      *            Location of the drop within the target row.
      * @param mouseEventDetails
@@ -78,11 +78,11 @@ public class GridDropEvent<T> extends DropEvent<Grid<T>> {
     /**
      * Get the row the drop happened on.
      * <p>
-     * If the drop was not on top of a row (see {@link #getDropLocation()}),
-     * then returns an empty optional.
+     * If the drop was not on top of a row (see {@link #getDropLocation()}) or
+     * {@link DropMode#ON_GRID} is used, then returns an empty optional.
      *
-     * @return The row the drop happened on, or an empty optional if dropped on
-     *         the in grid but not on top of any row, like to an empty grid
+     * @return The row the drop happened on, or an empty optional if drop was
+     *         not on a row
      */
     public Optional<T> getDropTargetRow() {
         return Optional.ofNullable(dropTargetRow);
@@ -91,12 +91,19 @@ public class GridDropEvent<T> extends DropEvent<Grid<T>> {
     /**
      * Get the location of the drop within the row.
      * <p>
-     * <em>NOTE: when dropped on an empty grid, or when {@link DropMode#ON_TOP}
-     * is used and the drop happened on empty space after last row or on top of
-     * the header / footer, the location will be
-     * {@link DropLocation#EMPTY}.</em>
+     * <em>NOTE: the location will be {@link DropLocation#EMPTY} if:
+     * <ul>
+     * <li>dropped on an empty grid</li>
+     * <li>dropping on rows was not possible because of
+     * {@link DropMode#ON_GRID } was used</li>
+     * <li>{@link DropMode#ON_TOP} is used and the drop happened on empty space
+     * after last row or on top of the header / footer</li>
+     * </ul>
+     * </em>
      *
-     * @return Location of the drop within the row.
+     * @return location of the drop in relative to the
+     *         {@link #getDropTargetRow()} or {@link DropLocation#EMPTY} if no
+     *         target row present
      * @see GridDropTarget#setDropMode(DropMode)
      */
     public DropLocation getDropLocation() {
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 edb1534a05..3f7ee6128e 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
@@ -38,6 +38,10 @@ import com.vaadin.ui.dnd.DropTargetExtension;
  */
 public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
 
+    private Registration sortListenerRegistration;
+    private DropMode cachedDropMode;
+    private boolean dropAllowedOnSortedGridRows = true;
+
     /**
      * Extends a Grid and makes it's rows drop targets for HTML5 drag and drop.
      *
@@ -57,27 +61,49 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
     /**
      * Sets the drop mode of this drop target.
      * <p>
-     * Note that when using {@link DropMode#ON_TOP}, and the grid is either
-     * empty or has empty space after the last row, the drop can still happen on
-     * the empty space, and the {@link GridDropEvent#getDropTargetRow()} will
-     * return an empty optional.
+     * When using {@link DropMode#ON_TOP}, and the grid is either empty or has
+     * empty space after the last row, the drop can still happen on the empty
+     * space, and the {@link GridDropEvent#getDropTargetRow()} will return an
+     * empty optional.
      * <p>
      * When using {@link DropMode#BETWEEN} or
      * {@link DropMode#ON_TOP_OR_BETWEEN}, and there is at least one row in the
      * grid, any drop after the last row in the grid will get the last row as
      * the {@link GridDropEvent#getDropTargetRow()}. If there are no rows in the
      * grid, then it will return an empty optional.
+     * <p>
+     * If using {@link DropMode#ON_GRID}, then the drop will not happen on any
+     * row, but instead just "on the grid". The target row will not be present
+     * in this case.
+     * <p>
+     * <em>NOTE: {@link DropMode#ON_GRID} is used automatically when the grid
+     * has been sorted and {@link #setDropAllowedOnSortedGridRows(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
+     * {@code true}.</em>
      *
      * @param dropMode
      *            Drop mode that describes the allowed drop locations within the
      *            Grid's row.
      * @see GridDropEvent#getDropLocation()
+     * @see #setDropAllowedOnSortedGridRows(boolean)
      */
     public void setDropMode(DropMode dropMode) {
         if (dropMode == null) {
             throw new IllegalArgumentException("Drop mode cannot be null");
         }
 
+        if (cachedDropMode != null) {
+            cachedDropMode = dropMode;
+        } else {
+            internalSetDropMode(dropMode);
+        }
+
+    }
+
+    private void internalSetDropMode(DropMode dropMode) {
         getState().dropMode = dropMode;
     }
 
@@ -92,6 +118,76 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
     }
 
     /**
+     * Sets whether the grid accepts drop on rows as target when the grid has
+     * been sorted by the user.
+     * <p>
+     * Default value is {@code true} for backwards compatibility with 8.1. When
+     * {@code true} is used or the grid is not sorted, the mode used in
+     * {@link #setDropMode(DropMode)} is always used.
+     * <p>
+     * {@code false} value means that when the grid has been sorted, the drop
+     * mode is always {@link DropMode#ON_GRID}, regardless of what was set with
+     * {@link #setDropMode(DropMode)}. Once the grid is not sorted anymore, the
+     * sort mode is reverted back to what was set with
+     * {@link #setDropMode(DropMode)}.
+     *
+     * @param dropAllowedOnSortedGridRows
+     *            {@code true} for allowing, {@code false} for not allowing
+     *            drops on sorted grid rows
+     * @since
+     */
+    public void setDropAllowedOnSortedGridRows(
+            boolean dropAllowedOnSortedGridRows) {
+        if (this.dropAllowedOnSortedGridRows != dropAllowedOnSortedGridRows) {
+            this.dropAllowedOnSortedGridRows = dropAllowedOnSortedGridRows;
+
+            if (!dropAllowedOnSortedGridRows) {
+
+                sortListenerRegistration = getParent()
+                        .addSortListener(event -> {
+                            updateDropModeForSortedGrid(
+                                    !event.getSortOrder().isEmpty());
+                        });
+
+                updateDropModeForSortedGrid(
+                        !getParent().getSortOrder().isEmpty());
+
+            } else {
+                // if the grid has been sorted, but now dropping on sorted grid
+                // is allowed, switch back to the previously allowed drop mode
+                if (cachedDropMode != null) {
+                    internalSetDropMode(cachedDropMode);
+                }
+                sortListenerRegistration.remove();
+                sortListenerRegistration = null;
+                cachedDropMode = null;
+            }
+        }
+    }
+
+    private void updateDropModeForSortedGrid(boolean sorted) {
+        if (sorted && cachedDropMode == null) {
+            cachedDropMode = getDropMode();
+            internalSetDropMode(DropMode.ON_GRID);
+        } else if (!sorted && cachedDropMode != null) {
+            internalSetDropMode(cachedDropMode);
+            cachedDropMode = null;
+        }
+    }
+
+    /**
+     * Gets whether drops are allowed on rows as target, when the user has
+     * sorted the grid.
+     *
+     * @return whether drop are allowed for the grid's rows when user has sorted
+     *         the grid
+     * @since
+     */
+    public boolean isDropAllowedOnSortedGridRows() {
+        return dropAllowedOnSortedGridRows;
+    }
+
+    /**
      * Attaches drop listener for the current drop target.
      * {@link GridDropListener#drop(GridDropEvent)} is called when drop event
      * happens on the client side.
@@ -174,4 +270,16 @@ public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
     protected GridDropTargetState getState(boolean markAsDirty) {
         return (GridDropTargetState) super.getState(markAsDirty);
     }
+
+    @Override
+    public void remove() {
+        super.remove();
+
+        // this handler can be removed from the grid and cannot be added to
+        // another grid, thus enough to just remove the listener
+        if (sortListenerRegistration != null) {
+            sortListenerRegistration.remove();
+            sortListenerRegistration = null;
+        }
+    }
 }
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
new file mode 100644
index 0000000000..d5de0ad808
--- /dev/null
+++ b/server/src/test/java/com/vaadin/tests/components/grid/GridDropTargetTest.java
@@ -0,0 +1,152 @@
+package com.vaadin.tests.components.grid;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.vaadin.shared.ui.grid.DropMode;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.components.grid.GridDropTarget;
+
+public class GridDropTargetTest {
+
+    private Grid<String> grid;
+    private GridDropTarget<String> target;
+
+    @Before
+    public void setup() {
+        grid = new Grid<>();
+        grid.addColumn(s -> s).setId("1");
+        grid.addColumn(s -> s).setId("2");
+        target = new GridDropTarget<>(grid, DropMode.BETWEEN);
+    }
+
+    @Test
+    public void dropAllowedOnSortedGridRows_defaultValue_isTrue() {
+        Assert.assertTrue("Default drop allowed should be backwards compatible",
+                target.isDropAllowedOnSortedGridRows());
+    }
+
+    @Test
+    public void dropAllowedOnSortedGridRows_notAllowed_changesDropModeWhenSorted() {
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(false);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        grid.sort("1");
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+
+        grid.sort("2");
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+
+        grid.clearSortOrder();
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        grid.clearSortOrder();
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        grid.sort("2");
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+    }
+
+    @Test
+    public void dropAllowedOnSortedGridRows_sortedGridIsDisallowed_modeChangesToOnGrid() {
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        grid.sort("1");
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(false);
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(true);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+    }
+
+    @Test
+    public void dropAllowedOnSortedGridRows_notAllowedBackToAllowed_changesBackToUserDefinedMode() {
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(false);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        grid.sort("1");
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(true);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        grid.clearSortOrder();
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+    }
+
+    @Test
+    public void dropAllowedOnSortedGridRows_swappingAllowedDropOnSortedOffAndOn() {
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(false);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(false);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(true);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(true);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+    }
+
+    @Test
+    public void dropAllowedOnSortedGridRows_changingDropModeWhileSorted_replacesPreviouslyCachedButDoesntOverride() {
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(false);
+
+        Assert.assertEquals(DropMode.BETWEEN, target.getDropMode());
+
+        grid.sort("1");
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+
+        target.setDropMode(DropMode.ON_TOP);
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+        Assert.assertFalse("Changing drop mode should not have any effect here",
+                target.isDropAllowedOnSortedGridRows());
+
+        grid.clearSortOrder();
+
+        Assert.assertEquals(DropMode.ON_TOP, target.getDropMode());
+
+        grid.sort("1");
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+
+        target.setDropMode(DropMode.ON_TOP_OR_BETWEEN);
+
+        Assert.assertEquals(DropMode.ON_GRID, target.getDropMode());
+
+        target.setDropAllowedOnSortedGridRows(true);
+
+        Assert.assertEquals(DropMode.ON_TOP_OR_BETWEEN, target.getDropMode());
+    }
+}