Add documentation on GridDragger (#10328)

This pathc also switches GridDragger to by default not allow drops on rows when the
grid has been sorted.

6 files changed, 85 insertions, 5 deletions

diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java
index 4b5ab2a84b..d79fad7ec2 100644
--- a/server/src/main/java/com/vaadin/ui/Grid.java
+++ b/server/src/main/java/com/vaadin/ui/Grid.java
@@ -1182,6 +1182,10 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
 
         /**
          * Sets whether the user can sort this column or not.
+         * <p>
+         * By default, a grid using a in-memory data provider has its columns
+         * sortable by default. For a backend data provider, the columns are not
+         * sortable by default.
          *
          * @param sortable
          *            {@code true} if the column can be sorted by the user;
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDragSource.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDragSource.java
index c5a3b0dd94..e3482a04f2 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridDragSource.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDragSource.java
@@ -43,6 +43,7 @@ import elemental.json.JsonObject;
  *            The Grid bean type.
  * @author Vaadin Ltd.
  * @since 8.1
+ * @see GridDragger
  */
 public class GridDragSource<T> extends DragSourceExtension<Grid<T>> {
 
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/GridDragger.java b/server/src/main/java/com/vaadin/ui/components/grid/GridDragger.java
index a7186a0801..a181b792ad 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/GridDragger.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/GridDragger.java
@@ -26,12 +26,21 @@ import com.vaadin.shared.ui.dnd.DropEffect;
 import com.vaadin.shared.ui.grid.DropLocation;
 import com.vaadin.shared.ui.grid.DropMode;
 import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.Column;
 
 /**
- * Allows dragging rows for reordering within a Grid and between separate Grids.
+ * Allows dragging rows for reordering within a Grid and between two separate
+ * Grids when the item type is the same.
  * <p>
  * When dragging a selected row, all the visible selected rows are dragged. Note
- * that ONLY currently visible rows are taken into account.
+ * that ONLY currently visible rows are taken into account. The drop mode for
+ * the target grid is by default {@link DropMode#BETWEEN}.
+ * <p>
+ * To customize the settings for either the source or the target grid, use
+ * {@link #getGridDragSource()} and {@link #getGridDropTarget()}.The drop target
+ * grid has been set to not allow drops for a target row when the grid has been
+ * sorted, since the visual drop target location would not match where the item
+ * would actually be dropped into.
  * <p>
  * <em>NOTE: this helper works only with {@link ListDataProvider} on both grids.
  * If you have another data provider, you should customize data provider
@@ -68,6 +77,20 @@ public class GridDragger<T> implements Serializable {
      * Enables DnD reordering for the rows in the given grid.
      * <p>
      * {@link DropMode#BETWEEN} is used.
+     * <p>
+     * <em>NOTE:</em> this only works when the grid has a
+     * {@link ListDataProvider}. Use the custom handlers
+     * {@link #setSourceDataProviderUpdater(SourceDataProviderUpdater)} and
+     * {@link #setTargetDataProviderUpdater(TargetDataProviderUpdater)} for
+     * other data providers.
+     * <p>
+     * <em>NOTE:</em> When allowing the user to DnD reorder a grid's rows, you
+     * should not allow the user to sort the grid since when the grid is sorted,
+     * as the reordering doens't make any sense since the drop target cannot be
+     * shown for the correct place due to the sorting. Sorting columns is
+     * enabled by default for in-memory data provider grids. Sorting can be
+     * disabled for columns with {@link Grid#getColumns()} and
+     * {@link Column#setSortable(boolean)}.
      *
      * @param grid
      *            Grid to be extended.
@@ -80,11 +103,19 @@ public class GridDragger<T> implements Serializable {
      * Enables DnD reordering the rows in the given grid with the given drop
      * mode.
      * <p>
-     * <em>NOTE: this only works when the grid has a
-     * {@link ListDataProvider}.</em> Use the custom handlers
+     * <em>NOTE:</em> this only works when the grid has a
+     * {@link ListDataProvider}. Use the custom handlers
      * {@link #setSourceDataProviderUpdater(SourceDataProviderUpdater)} and
      * {@link #setTargetDataProviderUpdater(TargetDataProviderUpdater)} for
      * other data providers.
+     * <p>
+     * <em>NOTE:</em> When allowing the user to DnD reorder a grid's rows, you
+     * should not allow the user to sort the grid since when the grid is sorted,
+     * as the reordering doens't make any sense since the drop target cannot be
+     * shown for the correct place due to the sorting. Sorting columns is
+     * enabled by default for in-memory data provider grids. Sorting can be
+     * disabled for columns with {@link Grid#getColumns()} and
+     * {@link Column#setSortable(boolean)}.
      *
      * @param grid
      *            the grid to enable row DnD reordering on
@@ -159,6 +190,7 @@ public class GridDragger<T> implements Serializable {
         gridDragSource = new GridDragSource<>(source);
 
         gridDropTarget = new GridDropTarget<>(target, dropMode);
+        gridDropTarget.setDropAllowedOnSortedGridRows(false);
 
         gridDragSource.addGridDragStartListener(event -> {
             draggedItems = event.getDraggedItems();
@@ -277,7 +309,7 @@ public class GridDragger<T> implements Serializable {
      * Returns the currently dragged items captured from the source grid no drag
      * start event, or {@code null} if no drag active.
      *
-     * @return the currenytly dragged items or {@code null}
+     * @return the currently dragged items or {@code null}
      */
     protected List<T> getDraggedItems() {
         return draggedItems;
@@ -374,6 +406,10 @@ public class GridDragger<T> implements Serializable {
         // instead of using setItems or creating a new data provider,
         // refresh the existing one to keep filters etc. in place
         listDataProvider.refreshAll();
+
+        // if dropped to the end of the grid, the grid should scroll there so
+        // that the dropped row is visible, but that is just recommended in
+        // documentation and left for the users to take into use
     }
 
     private int calculateDropIndex(GridDropEvent<T> event) {
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 e4b175fe81..5073287c52 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
@@ -35,6 +35,7 @@ import com.vaadin.ui.dnd.DropTargetExtension;
  *            Type of the Grid bean.
  * @author Vaadin Ltd
  * @since 8.1
+ * @see GridDragger
  */
 public class GridDropTarget<T> extends DropTargetExtension<Grid<T>> {
 
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerOneGridTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerOneGridTest.java
index 6d24020e45..24e72566f4 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerOneGridTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerOneGridTest.java
@@ -45,6 +45,7 @@ public class GridDraggerOneGridTest {
     @Before
     public void setupListCase() {
         source = new Grid<>();
+        source.addColumn(s -> s).setId("1");
         dragger = new TestGridDragger(source);
     }
 
@@ -203,4 +204,23 @@ public class GridDraggerOneGridTest {
         verifyDataProvider("1", "0", "2", "4", "3");
     }
 
+    @Test
+    public void dropOnSortedGrid_byDefault_dropsToTheEnd() {
+        Assert.assertFalse(
+                "Default drops on sorted grid rows should not be allowed",
+                dragger.getGridDropTarget().isDropAllowedOnSortedGridRows());
+
+        source.setItems("0", "1", "2", "3", "4");
+
+        drop("3", DropLocation.BELOW, "1");
+
+        verifyDataProvider("0", "2", "3", "1", "4");
+
+        source.sort("1");
+
+        drop(null, DropLocation.EMPTY, "0");
+
+        verifyDataProvider("2", "3", "1", "4", "0");
+    }
+
 }
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerTwoGridsTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerTwoGridsTest.java
index 0711b711df..c73641097d 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerTwoGridsTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDraggerTwoGridsTest.java
@@ -47,6 +47,7 @@ public class GridDraggerTwoGridsTest {
     public void setupListCase() {
         source = new Grid<>();
         target = new Grid<>();
+        target.addColumn(s -> s).setId("1");
         dragger = new TestGridDragger(source, target);
 
         target.setItems(); // setup to use list data provider
@@ -249,4 +250,21 @@ public class GridDraggerTwoGridsTest {
         Assert.assertEquals("given drop index to target updater is wrong", 2,
                 updaterTrigger.get());
     }
+
+    @Test
+    public void dropOnSortedGrid_byDefault_dropsToTheEnd() {
+        Assert.assertFalse(
+                "Default drops on sorted grid rows should not be allowed",
+                dragger.getGridDropTarget().isDropAllowedOnSortedGridRows());
+
+        source.setItems("0", "1", "2");
+        target.setItems("4", "5");
+
+        target.sort("1");
+
+        drop(null, DropLocation.EMPTY, "0");
+
+        verifySourceDataProvider("1", "2");
+        verifyTargetDataProvider("4", "5", "0");
+    }
 }