]> source.dussan.org Git - vaadin-framework.git/commitdiff
Fix partly missing drag image regression on Safari
authorPekka Hyvönen <pekka@vaadin.com>
Wed, 10 May 2017 11:42:31 +0000 (14:42 +0300)
committerIlia Motornyi <elmot@vaadin.com>
Wed, 10 May 2017 11:42:31 +0000 (14:42 +0300)
Doesn't fix #9261, drag image missing on Safari when dragging grid row because
that has position: absolute and offset.

client/src/main/java/com/vaadin/client/connectors/grid/GridDragSourceConnector.java
client/src/main/java/com/vaadin/client/extensions/DragSourceExtensionConnector.java
documentation/advanced/advanced-dragndrop.asciidoc
server/src/main/java/com/vaadin/event/dnd/ButtonDragSource.java
themes/src/main/themes/VAADIN/themes/valo/shared/_overlay.scss
uitest/src/main/java/com/vaadin/tests/components/grid/GridDragAndDrop.java
uitest/src/main/java/com/vaadin/tests/dnd/DragImage.java [new file with mode: 0644]

index 3f673bac0157e855a3d380d96c39aaeda34c4ae6..134e76e7e5d5850be5097a4e47e06ff8d709a378 100644 (file)
@@ -27,6 +27,7 @@ import com.google.gwt.dom.client.Style;
 import com.google.gwt.dom.client.TableRowElement;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Window;
+import com.google.gwt.user.client.ui.Image;
 import com.vaadin.client.BrowserInfo;
 import com.vaadin.client.ServerConnector;
 import com.vaadin.client.WidgetUtil;
@@ -37,6 +38,7 @@ import com.vaadin.client.widgets.Escalator;
 import com.vaadin.client.widgets.Grid;
 import com.vaadin.shared.Range;
 import com.vaadin.shared.ui.Connect;
+import com.vaadin.shared.ui.dnd.DragSourceState;
 import com.vaadin.shared.ui.dnd.DropEffect;
 import com.vaadin.shared.ui.grid.GridDragSourceRpc;
 import com.vaadin.shared.ui.grid.GridDragSourceState;
@@ -98,33 +100,53 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
             return;
         }
 
+        super.onDragStart(event);
+    }
+
+    @Override
+    protected void setDragImage(NativeEvent dragStartEvent) {
+        // do not call super since need to handle specifically
+        // 1. use resource if set (never needs safari hack)
+        // 2. add number badge if necessary (with safari hack if needed)
+        // 3. just use normal (with safari hack if needed)
+
         // Add badge showing the number of dragged columns
-        if (draggedItemKeys.size() > 1) {
-            Element draggedRowElement = (Element) event.getTarget();
-
-            Element badge = DOM.createSpan();
-            badge.setClassName(gridConnector.getWidget().getStylePrimaryName()
-                    + "-row" + STYLE_SUFFIX_DRAG_BADGE);
-            badge.setInnerHTML(draggedItemKeys.size() + "");
-
-            badge.getStyle().setMarginLeft(
-                    getRelativeX(draggedRowElement, (NativeEvent) event) + 10,
-                    Style.Unit.PX);
-            badge.getStyle().setMarginTop(
-                    getRelativeY(draggedRowElement, (NativeEvent) event)
-                            - draggedRowElement.getOffsetHeight() + 10,
-                    Style.Unit.PX);
-
-            draggedRowElement.appendChild(badge);
-
-            // Remove badge on the next animation frame. Drag image will still
-            // contain the badge.
-            AnimationScheduler.get().requestAnimationFrame(timestamp -> {
-                badge.removeFromParent();
-            }, (Element) event.getTarget());
+        String imageUrl = getResourceUrl(DragSourceState.RESOURCE_DRAG_IMAGE);
+        if (imageUrl != null && !imageUrl.isEmpty()) {
+            Image dragImage = new Image(
+                    getConnection().translateVaadinUri(imageUrl));
+            dragStartEvent.getDataTransfer()
+                    .setDragImage(dragImage.getElement(), 0, 0);
+        } else {
+            Element draggedRowElement = (Element) dragStartEvent
+                    .getEventTarget().cast();
+            if (draggedItemKeys.size() > 1) {
+
+                Element badge = DOM.createSpan();
+                badge.setClassName(
+                        gridConnector.getWidget().getStylePrimaryName() + "-row"
+                                + STYLE_SUFFIX_DRAG_BADGE);
+                badge.setInnerHTML(draggedItemKeys.size() + "");
+
+                badge.getStyle().setMarginLeft(
+                        getRelativeX(draggedRowElement, dragStartEvent) + 10,
+                        Style.Unit.PX);
+                badge.getStyle().setMarginTop(
+                        getRelativeY(draggedRowElement, dragStartEvent)
+                                - draggedRowElement.getOffsetHeight() + 10,
+                        Style.Unit.PX);
+
+                draggedRowElement.appendChild(badge);
+
+                // Remove badge on the next animation frame. Drag image will
+                // still contain the badge.
+                AnimationScheduler.get().requestAnimationFrame(timestamp -> {
+                    badge.removeFromParent();
+                }, (Element) dragStartEvent.getEventTarget().cast());
+                setDraggable(draggedRowElement);
+            }
+            fixDragImageForSafari(draggedRowElement);
         }
-
-        super.onDragStart(event);
     }
 
     private int getRelativeY(Element element, NativeEvent event) {
@@ -196,9 +218,9 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * allowed and a selected row is dragged.
      *
      * @param draggedRow
-     *         Data of dragged row.
+     *            Data of dragged row.
      * @return {@code true} if multiple rows are dragged, {@code false}
-     * otherwise.
+     *         otherwise.
      */
     private boolean dragMultipleRows(JsonObject draggedRow) {
         SelectionModel<JsonObject> selectionModel = getGrid()
@@ -221,7 +243,7 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * Get all selected rows from a subset of rows defined by {@code range}.
      *
      * @param range
-     *         Range of indexes.
+     *            Range of indexes.
      * @return List of data of all selected rows in the given range.
      */
     private List<JsonObject> getSelectedRowsInRange(Range range) {
@@ -241,7 +263,7 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * Converts a list of {@link JsonObject}s to a {@link JsonArray}.
      *
      * @param objects
-     *         List of json objects.
+     *            List of json objects.
      * @return Json array containing all json objects.
      */
     private JsonArray toJsonArray(List<JsonObject> objects) {
@@ -257,7 +279,7 @@ public class GridDragSourceConnector extends DragSourceExtensionConnector {
      * otherwise.
      *
      * @param row
-     *         Row data.
+     *            Row data.
      * @return Drag data if present or row data otherwise.
      */
     private JsonObject getDragData(JsonObject row) {
index 932128a8b3a0d773ad2ac6fca61f31ef72534382..e718fb90d735c295ec3167cbcca4cd07be126d39 100644 (file)
  */
 package com.vaadin.client.extensions;
 
+import com.google.gwt.animation.client.AnimationScheduler;
 import com.google.gwt.dom.client.DataTransfer;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.NativeEvent;
+import com.google.gwt.dom.client.Style;
+import com.google.gwt.dom.client.Style.Position;
 import com.google.gwt.user.client.ui.Image;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.BrowserInfo;
@@ -163,7 +166,7 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
         }
 
         // Set drag image
-        setDragImage(event);
+        setDragImage(nativeEvent);
 
         // Set text data parameter
         String dataTransferText = createDataTransferText(event);
@@ -184,6 +187,48 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
         nativeEvent.stopPropagation();
     }
 
+    /**
+     * Fixes missing drag image for Safari by making the dragged element
+     * position to relative if needed. Safari won't show drag image unless the
+     * dragged element position is relative or absolute / fixed, but not with
+     * display block for the latter.
+     * <p>
+     * This method is a NOOP for non-safari browser.
+     * <p>
+     * This fix is not needed if a custom drag image is used on Safari.
+     *
+     * @param draggedElement
+     *            the element that forms the drag image
+     */
+    protected void fixDragImageForSafari(Element draggedElement) {
+        if (!BrowserInfo.get().isSafari()) {
+            return;
+        }
+        final Style style = draggedElement.getStyle();
+        final String position = style.getPosition();
+
+        // relative works always
+        if ("relative".equalsIgnoreCase(position)) {
+            return;
+        }
+
+        // absolute & fixed don't work when there is offset used
+        if ("absolute".equalsIgnoreCase(position)
+                || "fixed".equalsIgnoreCase(position)) {
+            // FIXME #9261 need to figure out how to get absolute and fixed to
+            // position work when there is offset involved, like in Grid.
+            // The following hack with setting position to relative did not
+            // work, nor did clearing top/right/bottom/left.
+        }
+
+        // for all other positions, set the position to relative and revert it
+        // in an animation frame
+        draggedElement.getStyle().setPosition(Position.RELATIVE);
+        AnimationScheduler.get().requestAnimationFrame(timestamp -> {
+            draggedElement.getStyle().setProperty("position", position);
+        }, draggedElement);
+    }
+
     /**
      * Creates data of type {@code "text"} for the {@code DataTransfer} object
      * of the given event.
@@ -215,13 +260,16 @@ public class DragSourceExtensionConnector extends AbstractExtensionConnector {
      * @param dragStartEvent
      *            The drag start event.
      */
-    protected void setDragImage(Event dragStartEvent) {
+    protected void setDragImage(NativeEvent dragStartEvent) {
         String imageUrl = getResourceUrl(DragSourceState.RESOURCE_DRAG_IMAGE);
         if (imageUrl != null && !imageUrl.isEmpty()) {
             Image dragImage = new Image(
                     getConnection().translateVaadinUri(imageUrl));
-            ((NativeEvent) dragStartEvent).getDataTransfer()
+            dragStartEvent.getDataTransfer()
                     .setDragImage(dragImage.getElement(), 0, 0);
+        } else {
+            fixDragImageForSafari(
+                    (Element) dragStartEvent.getCurrentEventTarget().cast());
         }
     }
 
index bcdfb2343db7f0d232f64fc0e8bad33eef0f3eff..57764b55ddc913520180f74cf74d796e659a54a7 100644 (file)
@@ -73,20 +73,24 @@ dragSource.addDragEndListener(event ->
 };
 ----
 
-[NOTE]
-====
-The browsers allow the user to select and drag and drop text, which may cause some issues when trying to drag a component that has text. You can fix this by setting the following style rule to the drag source component to prevent dragging of the text instead of the whole component.
-[source, css]
-----
-user-select: none;
-----
-====
-
 === CSS Style Rules
 
 The drag source element, additional to it's primary style name, have a style name with the `-dragsource` suffix. For example, a Label component would have the style name `v-label-dragsource` when the drag source extension is applied to it.
 Additionally, the elements also have the `v-draggable` style name that is independent of the component's primary style.
 
+The browsers allow the user to select and drag and drop text, which could cause issues with components that have text. The Framework tries to prevent this by automatically adding the following style to all `v-draggable` elements. It is included by the sass mixin `valo-drag-element`.
+
+[source, css]
+----
+.v-draggable {
+    -moz-user-select: none !important;
+    -ms-user-select: none !important;
+    -webkit-user-select: none !important;
+    user-select: none !important;
+}
+----
+
+
 [[advanced.dragndrop.drophandler]]
 == Drop Target
 
index 4c681f5331f232e5cf9fdc668adf9df14491be5e..3d0d93dfa25e06c5d9329b2ebe185fbb27a02676 100644 (file)
@@ -15,6 +15,7 @@
  */
 package com.vaadin.event.dnd;
 
+import com.vaadin.annotations.Widgetset;
 import com.vaadin.shared.ui.dnd.ButtonDragSourceState;
 import com.vaadin.ui.Button;
 
@@ -24,6 +25,7 @@ import com.vaadin.ui.Button;
  * @author Vaadin Ltd.
  * @since 8.1
  */
+@Widgetset("com.vaadin.DefaultWidgetSet")
 public class ButtonDragSource extends DragSourceExtension<Button> {
 
     public ButtonDragSource(Button target) {
index 5b44e94cac328003f739cd6ef38431d736272f21..eabf4d3227cf3deb431aec9d92b3fbfba9299f9f 100644 (file)
@@ -277,6 +277,9 @@ $v-selection-item-selection-color: $v-selection-color !default;
 
 @mixin valo-draggable {
   .v-draggable {
+    -moz-user-select: none !important;
+    -ms-user-select: none !important;
+    -webkit-user-select: none !important;
     user-select: none !important;
   }
 }
index 149dec92e30b701624ec1ccbbd2bafb7ced17924..529d8cf56219cf32123e5aede137fcb30e19397b 100644 (file)
@@ -122,7 +122,8 @@ public class GridDragAndDrop extends AbstractTestUIWithLog {
 
         // Add drag end listener
         dragSource.addGridDragEndListener(event -> {
-            if (event.getDropEffect() == DropEffect.MOVE) {
+            if (event.getDropEffect() == DropEffect.MOVE
+                    && draggedItems != null) {
                 // If drop is successful, remove dragged item from source Grid
                 ((ListDataProvider<Person>) grid.getDataProvider()).getItems()
                         .removeAll(draggedItems);
diff --git a/uitest/src/main/java/com/vaadin/tests/dnd/DragImage.java b/uitest/src/main/java/com/vaadin/tests/dnd/DragImage.java
new file mode 100644 (file)
index 0000000..b4ef7ea
--- /dev/null
@@ -0,0 +1,67 @@
+package com.vaadin.tests.dnd;
+
+import java.util.stream.Stream;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.event.dnd.DragSourceExtension;
+import com.vaadin.server.Page;
+import com.vaadin.server.Page.Styles;
+import com.vaadin.server.ThemeResource;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.tests.components.uitest.TestSampler;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.VerticalLayout;
+
+@Theme("valo")
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class DragImage extends AbstractTestUIWithLog {
+
+    private final String[] positions = { "default", "relative", "absolute",
+            "static", "fixed", "sticky", "inherit", "initial", "unset" };
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        HorizontalLayout layout1 = new HorizontalLayout();
+        layout1.setCaption("No custom drag image");
+        Styles styles = Page.getCurrent().getStyles();
+
+        Stream.of(positions).forEach(position -> {
+            Label label = new Label(position);
+            label.addStyleName(position);
+            new DragSourceExtension<>(label);
+
+            layout1.addComponents(label, new Label("spacer"));
+
+            styles.add(new StringBuilder(".").append(position)
+                    .append(" { position: ").append(position).append(";}")
+                    .toString());
+        });
+
+        HorizontalLayout layout2 = new HorizontalLayout();
+        layout2.setCaption("Custom drag image");
+        Stream.of(positions).forEach(position -> {
+            Label label = new Label(position);
+            label.addStyleName(position);
+            new DragSourceExtension<>(label)
+                    .setDragImage(new ThemeResource(TestSampler.ICON_URL));
+            layout2.addComponents(label, new Label("spacer"));
+
+            styles.add(new StringBuilder(".").append(position)
+                    .append(" { position: ").append(position).append(";}")
+                    .toString());
+        });
+
+        // #9261 grid row like element that safari can't show the drag image for
+        String css = ".absolute-pos { position: absolute; top:0; }";
+        Label gridRowLikeLabel = new Label(css);
+        gridRowLikeLabel.addStyleName("absolute-pos");
+        new DragSourceExtension(gridRowLikeLabel);
+        styles.add(css);
+
+        addComponent(new VerticalLayout(layout1, layout2, gridRowLikeLabel));
+    }
+
+}