Show drop hints when dropping in empty Grid (#9353)

Also makes sure that the drop location and target row return something sensible in drop event.
Clarifies docs on drop location.

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

@@ -18,16 +18,17 @@ package com.vaadin.client.connectors.grid;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Optional;
 
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.NativeEvent;
 import com.google.gwt.dom.client.TableRowElement;
+import com.google.gwt.dom.client.TableSectionElement;
 import com.google.gwt.user.client.Window;
 import com.vaadin.client.ServerConnector;
 import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.extensions.DropTargetExtensionConnector;
 import com.vaadin.client.widget.escalator.RowContainer;
+import com.vaadin.client.widget.escalator.RowContainer.BodyRowContainer;
 import com.vaadin.client.widgets.Escalator;
 import com.vaadin.shared.ui.Connect;
 import com.vaadin.shared.ui.grid.DropLocation;
@@ -75,6 +76,19 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
      */
     private String styleDragBottom;
 
+    /**
+     * Class name to apply when dragged over an empty grid.
+     */
+    private String styleDragEmpty;
+
+    /**
+     * The latest row that was dragged on top of, or the grid body if drop is
+     * not applicable for any rows. Need to store this so that can remove drop
+     * hint styling when the target has changed since all browsers don't seem to
+     * always fire the drag-enter drag-exit events in a consistent order.
+     */
+    private Element latestTargetElement;
+
     @Override
     protected void extend(ServerConnector target) {
         gridConnector = (GridConnector) target;
@@ -90,12 +104,15 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
         String rowKey = null;
         DropLocation dropLocation = null;
 
-        Optional<TableRowElement> targetRow = getTargetRow(
+        Element targetElement = getTargetElement(
                 (Element) dropEvent.getEventTarget().cast());
-        if (targetRow.isPresent()) {
-            rowKey = getRowData(targetRow.get())
+        // the target element is either the body or one of the rows
+        if (TableRowElement.is(targetElement)) {
+            rowKey = getRowData(targetElement.cast())
                     .getString(GridState.JSONKEY_ROWKEY);
-            dropLocation = getDropLocation(targetRow.get(), dropEvent);
+            dropLocation = getDropLocation(targetElement, dropEvent);
+        } else {
+            dropLocation = DropLocation.EMPTY;
         }
 
         getRpcProxy(GridDropTargetRpc.class).drop(types, data, dropEffect,
@@ -112,23 +129,28 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
      * Returns the location of the event within the row.
      */
     private DropLocation getDropLocation(Element target, NativeEvent event) {
-        if (getState().dropMode == DropMode.BETWEEN) {
-            if (getRelativeY(target, event) < (target.getOffsetHeight() / 2)) {
-                return DropLocation.ABOVE;
-            } else {
-                return DropLocation.BELOW;
-            }
-        } else if (getState().dropMode == DropMode.ON_TOP_OR_BETWEEN) {
-            if (getRelativeY(target, event) < getState().dropThreshold) {
-                return DropLocation.ABOVE;
-            } else if (target.getOffsetHeight()
-                    - getRelativeY(target, event) < getState().dropThreshold) {
-                return DropLocation.BELOW;
+        if (TableRowElement.is(target)) {
+            if (getState().dropMode == DropMode.BETWEEN) {
+                if (getRelativeY(target,
+                        event) < (target.getOffsetHeight() / 2)) {
+                    return DropLocation.ABOVE;
+                } else {
+                    return DropLocation.BELOW;
+                }
+            } else if (getState().dropMode == DropMode.ON_TOP_OR_BETWEEN) {
+                if (getRelativeY(target, event) < getState().dropThreshold) {
+                    return DropLocation.ABOVE;
+                } else if (target.getOffsetHeight() - getRelativeY(target,
+                        event) < getState().dropThreshold) {
+                    return DropLocation.BELOW;
+                } else {
+                    return DropLocation.ON_TOP;
+                }
             } else {
                 return DropLocation.ON_TOP;
             }
         }
-        return DropLocation.ON_TOP;
+        return DropLocation.EMPTY;
     }
 
     private int getRelativeY(Element element, NativeEvent event) {
@@ -144,27 +166,37 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
         styleDragCenter = styleRow + STYLE_SUFFIX_DRAG_CENTER;
         styleDragTop = styleRow + STYLE_SUFFIX_DRAG_TOP;
         styleDragBottom = styleRow + STYLE_SUFFIX_DRAG_BOTTOM;
+        styleDragEmpty = gridConnector.getWidget().getStylePrimaryName()
+                + "-body" + STYLE_SUFFIX_DRAG_TOP;
 
         super.onDragEnter(event);
     }
 
     @Override
     protected void addDragOverStyle(NativeEvent event) {
-        getTargetRow(((Element) event.getEventTarget().cast()))
-                .ifPresent(target -> {
-
-                    // Get required class name
-                    String className = getTargetClassName(target, event);
-
-                    // Add or replace class name if changed
-                    if (!target.hasClassName(className)) {
-                        if (currentStyleName != null) {
-                            target.removeClassName(currentStyleName);
-                        }
-                        target.addClassName(className);
-                        currentStyleName = className;
-                    }
-                });
+        Element targetElement = getTargetElement(
+                ((Element) event.getEventTarget().cast()));
+        // Get required class name
+        String className = getTargetClassName(targetElement, event);
+
+        // it seems that sometimes the events are not fired in a consistent
+        // order, and this could cause that the correct styles are not removed
+        // from the previous target element in removeDragOverStyle(event)
+        if (latestTargetElement != null
+                && targetElement != latestTargetElement) {
+            removeStyles(latestTargetElement);
+        }
+
+        latestTargetElement = targetElement;
+
+        // Add or replace class name if changed
+        if (!targetElement.hasClassName(className)) {
+            if (currentStyleName != null) {
+                targetElement.removeClassName(currentStyleName);
+            }
+            targetElement.addClassName(className);
+            currentStyleName = className;
+        }
     }
 
     private String getTargetClassName(Element target, NativeEvent event) {
@@ -177,6 +209,9 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
         case BELOW:
             className = styleDragBottom;
             break;
+        case EMPTY:
+            className = styleDragEmpty;
+            break;
         case ON_TOP:
         default:
             className = styleDragCenter;
@@ -188,23 +223,38 @@ public class GridDropTargetConnector extends DropTargetExtensionConnector {
 
     @Override
     protected void removeDragOverStyle(NativeEvent event) {
-
         // Remove all possible style names
-        getTargetRow((Element) event.getEventTarget().cast()).ifPresent(e -> {
-            e.removeClassName(styleDragCenter);
-            e.removeClassName(styleDragTop);
-            e.removeClassName(styleDragBottom);
-        });
+        Element targetElement = getTargetElement(
+                (Element) event.getEventTarget().cast());
+        removeStyles(targetElement);
+    }
+
+    private void removeStyles(Element element) {
+        element.removeClassName(styleDragCenter);
+        element.removeClassName(styleDragTop);
+        element.removeClassName(styleDragBottom);
+        element.removeClassName(styleDragEmpty);
     }
 
-    private Optional<TableRowElement> getTargetRow(Element source) {
-        while (!Objects.equals(source, getGridBody().getElement())) {
+    private Element getTargetElement(Element source) {
+        final BodyRowContainer gridBody = getGridBody();
+        final TableSectionElement bodyElement = gridBody.getElement();
+        while (!Objects.equals(source, bodyElement)) {
             if (TableRowElement.is(source)) {
-                return Optional.of(source.cast());
+                return source;
             }
             source = source.getParentElement();
         }
-        return Optional.empty();
+        // the drag is on top of the body
+        final int rowCount = gridBody.getRowCount();
+        // if no rows in grid, or if the drop mode is on top, then there is no
+        // target row for the drop
+        if (rowCount == 0 || getState().dropMode == DropMode.ON_TOP) {
+            return bodyElement;
+        } else { // if dragged under the last row to empty space, drop target
+                 // needs to be below the last row
+            return gridBody.getRowElement(rowCount - 1);
+        }
     }
 
     @Override

documentation/advanced/advanced-dragndrop.asciidoc

@@ -306,10 +306,10 @@ GridDropTarget<Person> dropTarget = new GridDropTarget<>(grid, DropMode.BETWEEN)
 dropTarget.setDropEffect(DropEffect.MOVE);
 ----
 
-The _drop mode_ specifies the behaviour of the row when an element is dragged over or dropped onto it. Use `DropMode.ON_TOP` when you want to drop elements on top of a row and `DropMode.BETWEEN` when you want to drop elements between rows.
+The _drop mode_ specifies the behaviour of the row when an element is dragged over or dropped onto it. Use `DropMode.ON_TOP` when you want to drop elements on top of a row and `DropMode.BETWEEN` when you want to drop elements between rows. `DropMode_ON_TOP_OR_BETWEEN` allows to drop on between or top rows.
 
-The [classname]#GridDropEvent# is fired when data is dropped onto one of the Grid's rows. The following example shows how you can insert items into the Grid at the drop position. If the drag source is another Grid, you can access the generated drag data with the event's `getDataTransferText()` method.
-If the drag source Grid uses a custom generator for a different type than `"text"`, you can access it's generated data using the `getDataTransferData(type)` method.
+The [classname]#GridDropEvent# is fired when data is dropped onto one of the Grid's rows. The following example shows how you can insert items into the Grid at the drop position. If the drag source is another Grid, you can access the generated drag data with the event's [methodname]#getDataTransferText()# method.
+If the drag source Grid uses a custom generator for a different type than `"text"`, you can access it's generated data using the [methodname]#getDataTransferData(type)# method. You can also check all the received data transfer data by fetching the type-to-data map with the [methodname]#getDataTransferData()# method.
 
 [source,java]
 ----
@@ -340,12 +340,16 @@ dropTarget.addGridDropListener(event -> {
 });
 ----
 
+The _drop location_ property in the [classname]#GridDropEvent# specifies the dropped location in relative to grid row the drop happened on and depends on the used [classname]#DropMode#. When the drop happened on top of a row, the possible options for the location are `ON_TOP`, `ABOVE` and `BELOW`.
+
+If the grid is empty, or if there was empty space after the last row in grid and the [classname]#DropMode.ON_TOP# was used, then the drop location `EMPTY` will be used. If the drop modes [classname]#DropMode.BETWEEN# or [classname]#DropMode.ON_TOP_OR_BETWEEN# are used, then the location can be `EMPTY` only when the grid was empty; otherwise the drop happened ´BELOW´ the last row. When the drop location is `EMPTY`, the [methodname]#getDropTargetRow# method will also return an empty optional.
+
 ==== CSS Style Rules
 
 A drop target Grid's body has the style name `v-grid-body-droptarget` to indicate that it is a potential target for data to be dropped.
 
-When dragging data over a drop target Grid's row, depending on the drop mode and the mouse position relative to the row, a style name is applied to the row to indicate the drop location.
-`v-grid-row-drag-center` indicates ON_TOP, `v-grid-row-drag-top` indicates ABOVE and `v-grid-row-drag-bottom` indicates BELOW locations.
+When dragging data over a drop target Grid's row, depending on the drop mode and the mouse position relative to the row, a style name is applied to the row or to the grid body to indicate the drop location.
+When dragging on top of a row, `v-grid-row-drag-center` indicates ON_TOP, `v-grid-row-drag-top` indicates ABOVE and `v-grid-row-drag-bottom` indicates BELOW locations. When dragging on top of an empty grid, or when the drop location is ON_TOP and dragged below the last row in grid (and there is empty space visible), the `v-grid-body-body-drag-top` style is applied to the table body element.
 
 (((range="endofrange", startref="term.advanced.dragndrop")))
 

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

@@ -20,6 +20,7 @@ import java.util.Optional;
 
 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.AbstractComponent;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.dnd.DragSourceExtension;
@@ -43,19 +44,21 @@ public class GridDropEvent<T> extends DropEvent<Grid<T>> {
      * Creates a Grid row drop event.
      *
      * @param target
-     *         Grid that received the drop.
+     *            Grid that received the drop.
      * @param data
-     *         Map containing all types and corresponding data from the {@code
+     *            Map containing all types and corresponding data from the
+     *            {@code
      *         DataTransfer} object.
      * @param dropEffect
-     *         the desired drop effect
+     *            the desired drop effect
      * @param dragSourceExtension
-     *         Drag source extension of the component that initiated the drop
-     *         event.
+     *            Drag source extension of the component that initiated the drop
+     *            event.
      * @param dropTargetRow
-     *         Target row that received the drop.
+     *            Target row that received the drop, or {@code null} if dropped
+     *            on empty grid
      * @param dropLocation
-     *         Location of the drop within the target row.
+     *            Location of the drop within the target row.
      */
     public GridDropEvent(Grid<T> target, Map<String, String> data,
             DropEffect dropEffect,
@@ -68,10 +71,13 @@ public class GridDropEvent<T> extends DropEvent<Grid<T>> {
     }
 
     /**
-     * Get the row item source of this event.
+     * 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.
      *
-     * @return The optional row item if the event was originated from a row,
-     * otherwise 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
      */
     public Optional<T> getDropTargetRow() {
         return Optional.ofNullable(dropTargetRow);
@@ -79,8 +85,13 @@ 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, the location
+     * will be {@link DropLocation#EMPTY}.</em>
      *
      * @return Location of the drop within the row.
+     * @see GridDropTarget#setDropMode(DropMode)
      */
     public DropLocation getDropLocation() {
         return dropLocation;

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

@@ -55,6 +55,17 @@ 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.
+     * <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.
      *
      * @param dropMode
      *            Drop mode that describes the allowed drop locations within the

shared/src/main/java/com/vaadin/shared/ui/grid/DropLocation.java

@@ -36,5 +36,11 @@ public enum DropLocation {
     /**
      * Drop below or after the row.
      */
-    BELOW
+    BELOW,
+
+    /**
+     * Dropping into an empty grid, or to the empty area below the grid rows
+     * when {@link DropMode#ON_TOP} is used.
+     */
+    EMPTY;
 }

themes/src/main/themes/VAADIN/themes/valo/components/_grid.scss

@@ -861,7 +861,7 @@ $v-grid-drag-indicator-color: $v-focus-color;
     top: 0;
     left: 0;
     bottom: 0;
-    right: 0;
+    right: 2px;
     border: 2px solid $v-grid-drag-indicator-color;
     pointer-events: none;
   }
@@ -879,8 +879,22 @@ $v-grid-drag-indicator-color: $v-focus-color;
 
   // Expand Grid's body to cover the whole Grid
   .#{$primary-stylename}-body-droptarget {
-    width: 100%;
-    height: 100%;
+    top: 0;
+    right: 0;
+    bottom: 0;
+    left: 0;
+  }
+  // Show drop hint when the grid is empty or,
+  // if the DropMode.ON_TOP is used and dragging below last row
+  .#{$primary-stylename}-body-drag-top:after {
+    content: "";
+    position: absolute;
+    top: 0;
+    right: 2px;
+    bottom: 0;
+    left: 0;
+    pointer-events: none;
+    border: 2px solid $v-grid-drag-indicator-color;
   }
 }
 

uitest/src/main/java/com/vaadin/tests/components/grid/GridDragAndDrop.java

@@ -61,6 +61,7 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
         // Layout the two grids
         Layout grids = new HorizontalLayout();
         grids.addComponents(left, right);
+        grids.setWidth("100%");
 
         // Selection modes
         List<Grid.SelectionMode> selectionModes = Arrays
@@ -87,6 +88,7 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
 
     private Grid<Person> createGridAndFillWithData(int numberOfItems) {
         Grid<Person> grid = new Grid<>();
+        grid.setWidth("100%");
 
         grid.setItems(generateItems(numberOfItems));
         grid.addColumn(
@@ -162,7 +164,7 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
                     if (event.getDropTargetRow().isPresent()) {
                         index = items.indexOf(event.getDropTargetRow().get())
                                 + (event.getDropLocation() == DropLocation.BELOW
-                                ? 1 : 0);
+                                        ? 1 : 0);
                     }
 
                     // Add dragged items to the target Grid
@@ -173,10 +175,12 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
                             + ", dragDataJson="
                             + event.getDataTransferData("application/json")
                             + ", target="
-                            + (event.getDropTargetRow().isPresent() ?
-                            event.getDropTargetRow().get().getFirstName() + " "
+                            + (event.getDropTargetRow().isPresent() ? event
+                                    .getDropTargetRow().get().getFirstName()
+                                    + " "
                                     + event.getDropTargetRow().get()
-                                    .getLastName() : "[BODY]")
+                                            .getLastName()
+                                    : "[BODY]")
                             + ", location=" + event.getDropLocation());
                 }
             });