Merge branch 'dropeffects' into dev

marceljuenemann · marceljuenemann · commit dc5d1c721452 · 2016-12-26T12:11:12.000Z
diff --git a/angular-drag-and-drop-lists.js b/angular-drag-and-drop-lists.js
@@ -381,9 +381,6 @@
         var ignoreDataTransfer = mimeType == MSIE_MIME_TYPE && dndState.isDragging;
         var dropEffect = getDropEffect(event, ignoreDataTransfer);
         if (dropEffect == 'none') return stopDragover();
-        if (!ignoreDataTransfer) {
-          event.dataTransfer.dropEffect = dropEffect;
-        }
 
         // Invoke the callback, which can transform the transferredObject and even abort the drop.
         var index = getPlaceholderIndex();
@@ -393,6 +390,12 @@
         }
         dndState.dropEffect = dropEffect;
 
+        // The drop is definitely going to happen now, store the dropEffect.
+        dndState.dropEffect = dropEffect;
+        if (!ignoreDataTransfer) {
+          event.dataTransfer.dropEffect = dropEffect;
+        }
+
         // Insert the object into the array, unless dnd-drop took care of that (returned true).
         if (data !== true) {
           scope.$apply(function() {
diff --git a/test/dndDraggableSpec.js b/test/dndDraggableSpec.js
@@ -70,6 +70,11 @@ describe('dndDraggable', function() {
       expect(Dragstart.on(element).effectAllowed).toBe('copyMove');
     });
 
+    it('sets effectAllowed to single effect in IE', function() {
+      element = compileAndLink('<div dnd-draggable dnd-effect-allowed="copyLink"></div>');
+      expect(Dragstart.on(element, {allowedMimeTypes: ['Text']}).effectAllowed).toBe('copy');
+    });
+
     it('adds CSS classes to element', inject(function($timeout) {
       Dragstart.on(element);
       expect(element.hasClass('dndDragging')).toBe(true);
@@ -124,24 +129,18 @@ describe('dndDraggable', function() {
       expect(element.hasClass('dndDraggingSource')).toBe(false);
     }));
 
-    var dropEffects = {move: 'moved', copy: 'copied', none: 'canceled'};
+    var dropEffects = {move: 'moved', copy: 'copied', link: 'linked', none: 'canceled'};
     angular.forEach(dropEffects, function(callback, dropEffect) {
       it('calls callbacks for dropEffect ' + dropEffect, function() {
-        var html = '<div dnd-draggable="{}" dnd-dragend="de = dropEffect" '
-                 + 'dnd-' + callback + '="ev = event"></div>';
+        var html = '<div dnd-draggable="{}" dnd-effect-allowed="' + dropEffect + '" '
+                 + 'dnd-dragend="returnedDropEffect = dropEffect" '
+                 + 'dnd-' + callback + '="returnedEvent = event"></div>';
         var element = compileAndLink(html);
+        var target = compileAndLink('<div dnd-list="[]"></div>');
+        Dragstart.on(element).dragover(target).drop(target).dragend(element);
 
-        var dragstart = Dragstart.on(element);
-        if (dropEffect != 'none') {
-          var target = compileAndLink('<div dnd-list="[]"></div>');
-          var options = {dropEffect: dropEffect};
-          dragstart.dragover(target, options).drop(target).dragend(element);
-        } else {
-          dragstart.dragend(element);
-        }
-
-        expect(element.scope().ev).toEqual(jasmine.any(DragEventMock));
-        expect(element.scope().de).toBe(dropEffect);
+        expect(element.scope().returnedEvent).toEqual(jasmine.any(DragEventMock));
+        expect(element.scope().returnedDropEffect).toBe(dropEffect);
       });
     });
   });
diff --git a/test/dndListSpec.js b/test/dndListSpec.js
@@ -341,24 +341,6 @@ describe('dndList', function() {
       var dragenter = Dragenter.externalOn(element, {'application/x-dnd': 'Lorem ipsum'});
       verifyDropCancelled(dragenter.dragover(element).drop(element), element, true, 3);
     });
-
-    describe('dropEffect calculation', function() {
-      testDropEffect('move', 'move');
-      testDropEffect('blub', 'blub');
-      testDropEffect('copy', 'none', 'copy');
-      testDropEffect('move', 'none', 'move');
-      testDropEffect('move', 'none', 'link');
-      testDropEffect('copy', 'none', 'link', true);
-
-      function testDropEffect(expected, dropEffect, effectAllowed, ctrlKey) {
-        it('stores ' + expected + ' for ' + [dropEffect, effectAllowed, ctrlKey], function() {
-          var src = compileAndLink('<div dnd-draggable="{}" dnd-dragend="eff = dropEffect"></div>');
-          var options = { dropEffect: dropEffect, effectAllowed: effectAllowed, ctrlKey: ctrlKey };
-          Dragstart.on(src).dragover(element, options).drop(element).dragend(src);
-          expect(src.scope().eff).toBe(expected);
-        });
-      }
-    });
   });
 
   describe('dragleave handler', function() {
@@ -399,6 +381,121 @@ describe('dndList', function() {
     }));
   });
 
+  describe('dropEffect', function() {
+    // This matrix shows the expected drop effect, given two effectAllowed values.
+    var ALL = [  'all',  'move', 'copy', 'link', 'copyLink', 'copyMove', 'linkMove'];
+    var EXPECTED_MATRIX = {
+      move:     ['move', 'move', 'none', 'none', 'none',     'move',     'move'],
+      copy:     ['copy', 'none', 'copy', 'none', 'copy',     'copy',     'none'],
+      link:     ['link', 'none', 'none', 'link', 'link',     'none',     'link'],
+      copyLink: ['copy', 'none', 'copy', 'link', 'copy',     'copy',     'link'],
+      copyMove: ['move', 'move', 'copy', 'none', 'copy',     'move',     'move'],
+      linkMove: ['move', 'move', 'none', 'link', 'link',     'move',     'move'],
+      all:      ['move', 'move', 'copy', 'link', 'copy',     'move',     'move'],
+      '':       ['move', 'move', 'copy', 'link', 'copy',     'move',     'move'],
+    };
+    angular.forEach(ALL, function(sourceEffectAllowed, index) {
+      angular.forEach(EXPECTED_MATRIX, function(expected, targetEffectAllowed) {
+        expected = expected[index];
+        it('is ' + expected + ' for effect-allowed ' + sourceEffectAllowed
+            + ' and ' + targetEffectAllowed, function() {
+          var src = compileAndLink('<div dnd-draggable="{}" dnd-dragend="result = dropEffect" '
+                                 + 'dnd-effect-allowed="' + sourceEffectAllowed + '"></div>');
+          var target = createListWithItemsAndCallbacks(false, targetEffectAllowed);
+          expect(Dragstart.on(src).effectAllowed).toBe(sourceEffectAllowed);
+          if (expected != 'none') {
+            // Verify dragover.
+            expect(Dragstart.on(src).dragover(target).dropEffect).toBe(expected);
+            expect(target.scope().dragover.dropEffect).toBe(expected);
+            // Verify drop.
+            expect(Dragstart.on(src).dragover(target).drop(target).dropEffect).toBe(expected);
+            expect(target.scope().drop.dropEffect).toBe(expected);
+            // Verify dragend.
+            Dragstart.on(src).dragover(target).drop(target).dragend(src);
+            expect(src.scope().result).toBe(expected);
+          } else {
+            verifyDropCancelled(Dragstart.on(src).dragover(target), target, false, 3);
+            verifyDropCancelled(Dragstart.on(src).dragover(target).drop(target), target, true, 3);
+            Dragstart.on(src).dragend(src);
+            expect(src.scope().result).toBe('none');
+          }
+        });
+      });
+    });
+
+    // In Safari dataTransfer.effectAllowed is always 'all', ignoring the value set in dragstart.
+    it('is determined from internal state in Safari', function() {
+      var src = compileAndLink('<div dnd-draggable="{}" dnd-effect-allowed="link"></div>');
+      var target = createListWithItemsAndCallbacks(false, 'copyLink');
+      var options = {effectAllowed: 'all'};
+      Dragstart.on(src).dragover(target, options).drop(target, options);
+      expect(target.scope().dragover.dropEffect).toBe('link');
+      expect(target.scope().drop.dropEffect).toBe('link');
+    });
+
+    // On MacOS, modifiers automatically limit the effectAllowed passed to dragover and drop.
+    it('is limited by modifier keys on MacOS', function() {
+      var src = compileAndLink('<div dnd-draggable="{}" dnd-effect-allowed="all"></div>');
+      var target = createListWithItemsAndCallbacks();
+      Dragstart.on(src).dragover(target, {effectAllowed: 'copyLink'}).drop(target);
+      expect(target.scope().dragover.dropEffect).toBe('copy');
+      expect(target.scope().drop.dropEffect).toBe('copy');
+    });
+
+    it('is copy if Ctrl key is pressed', function() {
+      var src = compileAndLink('<div dnd-draggable="{}" dnd-effect-allowed="all"></div>');
+      var target = createListWithItemsAndCallbacks();
+      Dragstart.on(src).dragover(target, {ctrlKey: true}).drop(target);
+      expect(target.scope().dragover.dropEffect).toBe('copy');
+      expect(target.scope().drop.dropEffect).toBe('copy');
+    });
+
+    it('is link if Alt key is pressed', function() {
+      var src = compileAndLink('<div dnd-draggable="{}" dnd-effect-allowed="all"></div>');
+      var target = createListWithItemsAndCallbacks();
+      Dragstart.on(src).dragover(target, {altKey: true}).drop(target);
+      expect(target.scope().dragover.dropEffect).toBe('link');
+      expect(target.scope().drop.dropEffect).toBe('link');
+    });
+
+    it('ignores Ctrl key if copy is not possible', function() {
+      var src = compileAndLink('<div dnd-draggable="{}" dnd-effect-allowed="linkMove"></div>');
+      var target = createListWithItemsAndCallbacks();
+      Dragstart.on(src).dragover(target, {ctrlKey: true}).drop(target);
+      expect(target.scope().dragover.dropEffect).toBe('move');
+      expect(target.scope().drop.dropEffect).toBe('move');
+    });
+
+    it('respects effectAllowed from external drops', function() {
+      var target = createListWithItemsAndCallbacks();
+      Dragenter.validExternalOn(target, {effectAllowed: 'copyLink'}).dragover(target).drop(target);
+      expect(target.scope().dragover.dropEffect).toBe('copy');
+      expect(target.scope().drop.dropEffect).toBe('copy');
+    });
+
+    it('respects effectAllowed from external drops even in IE', function() {
+      var target = createListWithItemsAndCallbacks();
+      Dragenter.externalOn(target, {'Text': '{}'}, {effectAllowed: 'copyLink'})
+               .dragover(target).drop(target);
+      expect(target.scope().dragover.dropEffect).toBe('copy');
+      expect(target.scope().drop.dropEffect).toBe('copy');
+    });
+
+    it('ignores effectAllowed from internal drops in IE', function() {
+      var src = compileAndLink('<div dnd-draggable="{}" dnd-effect-allowed="copyLink"></div>');
+      var target = createListWithItemsAndCallbacks();
+      Dragstart.on(src, {allowedMimeTypes: ['Text']}).dragover(target, {altKey: true});
+      expect(target.scope().dragover.dropEffect).toBe('link');
+    });
+
+    it('does not set dropEffect in IE', function() {
+      var src = compileAndLink('<div dnd-draggable="{}" dnd-effect-allowed="copyLink"></div>');
+      var target = createListWithItemsAndCallbacks();
+      var dragover = Dragstart.on(src, {allowedMimeTypes: ['Text']}).dragover(target);
+      expect(dragover.dropEffect).toBeUndefined();
+    });
+  });
+
   function verifyDropAccepted(result) {
     expect(result.defaultPrevented).toBe(true);
     if (result.type == 'dragenter') {
@@ -414,6 +511,7 @@ describe('dndList', function() {
     expect(result.returnValue).toBe(true);
     expect(result.propagationStopped).toBe(false);
     expect(result.defaultPrevented).toBe(opt_defaultPrevented || false);
+    expect(result.dropEffect).toBeUndefined();
     expect(element.hasClass("dndDragover")).toBe(false);
     expect(element.children().length).toBe(opt_children || 0);
   }
@@ -428,10 +526,12 @@ describe('dndList', function() {
     verify(drop, element);
   }
 
-  function createListWithItemsAndCallbacks(horizontal) {
-    var params = '{event: event, index: index, item: item, external: external, type: type}';
+  function createListWithItemsAndCallbacks(horizontal, effectAllowed) {
+    var params = '{event: event, dropEffect: dropEffect, index: index, '
+               + 'item: item, external: external, type: type}';
     var element = compileAndLink('<ul dnd-list="list" dnd-external-sources="true" ' +
                   'dnd-horizontal-list="' + (horizontal || 'false') + '" ' +
+                  (effectAllowed ? 'dnd-effect-allowed="' + effectAllowed + '" ' : '') +
                   'dnd-dragover="dragover = ' + params + '" ' +
                   'dnd-drop="dropHandler(' + params + ')" ' +
                   'dnd-inserted="inserted = ' + params + '">' +
diff --git a/test/mocks.js b/test/mocks.js
@@ -73,6 +73,7 @@ class DragEventMock {
   get clientX() { return this.$options.clientX || 0; }
   get clientY() { return this.$options.clientY || 0; }
   get ctrlKey() { return this.$options.ctrlKey || false; }
+  get altKey() { return this.$options.altKey || false; }
   get dataTransfer() { return this.$dataTransfer; }
   get originalEvent() { return this; }
   get target() { return this.$options.target || undefined; }
@@ -96,6 +97,7 @@ class DragEventResult {
   get propagationStopped() { return !!this.$results.invokedStopPropagation; }
   get defaultPrevented() { return !!this.$results.invokedPreventDefault; }
   get returnValue() { return this.$results.returnValue; }
+  get dropEffect() { return this.$results.dataTransfer.dropEffect; }
   get type() { return this.$type; }
 }
 
