From bd2616f9017801778b3624574d5e0aace5b24ac3 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Fri, 8 Jul 2016 11:22:54 +0200 Subject: [PATCH] Do not rely on options.draggable to compute draggableMoved (#4638) One can enable dragging of a marker after it has been initialized with draggable=false. --- debug/map/markers.html | 2 +- spec/suites/map/handler/Map.DragSpec.js | 136 +++++++++++++++++++++--- src/map/Map.js | 2 +- 3 files changed, 125 insertions(+), 15 deletions(-) diff --git a/debug/map/markers.html b/debug/map/markers.html index 9669c927..d72e78c4 100644 --- a/debug/map/markers.html +++ b/debug/map/markers.html @@ -32,12 +32,12 @@ var markerDraggable = new L.Marker([0, 10], { - draggable: true, title: 'Draggable' }); map.addLayer(markerDraggable); markerDraggable.bindPopup("Draggable"); + markerDraggable.dragging.enable(); var poly = new L.Polygon([[0, 10], [0, 15.5], [0, 50], [20, 20.5]]); map.addLayer(poly); diff --git a/spec/suites/map/handler/Map.DragSpec.js b/spec/suites/map/handler/Map.DragSpec.js index eab9cded..a865b867 100644 --- a/spec/suites/map/handler/Map.DragSpec.js +++ b/spec/suites/map/handler/Map.DragSpec.js @@ -38,15 +38,21 @@ describe("Map.Drag", function () { describe("mouse events", function () { - it("change the center of the map", function (done) { - var container = document.createElement('div'); + var container; + + beforeEach(function () { + container = document.createElement('div'); container.style.width = container.style.height = '600px'; container.style.top = container.style.left = 0; container.style.position = 'absolute'; - // container.style.background = '#808080'; - document.body.appendChild(container); + }); + afterEach(function () { + document.body.removeChild(container); + }); + + it("change the center of the map", function (done) { var map = new L.Map(container, { dragging: true, inertia: false @@ -58,7 +64,6 @@ describe("Map.Drag", function () { onStop: function () { var center = map.getCenter(); var zoom = map.getZoom(); - document.body.removeChild(container); expect(center.lat).to.be.within(21.9430, 21.9431); expect(center.lng).to.be(-180); expect(zoom).to.be(1); @@ -75,13 +80,6 @@ describe("Map.Drag", function () { }); it("does not change the center of the map when mouse is moved less than the drag threshold", function (done) { - var container = document.createElement('div'); - container.style.width = container.style.height = '600px'; - container.style.top = container.style.left = 0; - container.style.position = 'absolute'; - - document.body.appendChild(container); - var map = new L.Map(container, { dragging: true, inertia: false @@ -98,7 +96,6 @@ describe("Map.Drag", function () { onStop: function () { var center = map.getCenter(); var zoom = map.getZoom(); - document.body.removeChild(container); expect(center).to.be(originalCenter); // Expect center point to be the same as before the click expect(spy.callCount).to.eql(0); // No drag event should have been fired. expect(zoom).to.be(1); @@ -114,6 +111,117 @@ describe("Map.Drag", function () { .down().moveBy(1, 0, 20).moveBy(1, 0, 200).up(); }); + it("does not trigger preclick nor click", function (done) { + var map = new L.Map(container, { + dragging: true, + inertia: false + }); + map.setView([0, 0], 1); + var clickSpy = sinon.spy(); + var preclickSpy = sinon.spy(); + var dragSpy = sinon.spy(); + map.on('click', clickSpy); + map.on('preclick', preclickSpy); + map.on('drag', dragSpy); + + var hand = new Hand({ + timing: 'fastframe', + onStop: function () { + // A real user scenario would trigger a click on mouseup. + // We want to be sure we are cancelling it after a drag. + happen.click(container); + expect(dragSpy.called).to.be(true); + expect(clickSpy.called).to.be(false); + expect(preclickSpy.called).to.be(false); + done(); + } + }); + var mouse = hand.growFinger('mouse'); + + // We move 5 pixels first to overcome the 3-pixel threshold of + // L.Draggable. + mouse.wait(100).moveTo(200, 200, 0) + .down().moveBy(5, 0, 20).moveBy(256, 32, 200).up(); + }); + + it("does not trigger preclick nor click when dragging on top of a static marker", function (done) { + var map = new L.Map(container, { + dragging: true, + inertia: false + }); + map.setView([0, 0], 1); + var marker = L.marker(map.getCenter()).addTo(map); + var clickSpy = sinon.spy(); + var preclickSpy = sinon.spy(); + var markerDragSpy = sinon.spy(); + var mapDragSpy = sinon.spy(); + map.on('click', clickSpy); + map.on('preclick', preclickSpy); + map.on('drag', mapDragSpy); + marker.on('click', clickSpy); + marker.on('preclick', preclickSpy); + marker.on('drag', markerDragSpy); + + var hand = new Hand({ + timing: 'fastframe', + onStop: function () { + // A real user scenario would trigger a click on mouseup. + // We want to be sure we are cancelling it after a drag. + happen.click(container); + expect(mapDragSpy.called).to.be(true); + expect(markerDragSpy.called).to.be(false); + expect(clickSpy.called).to.be(false); + expect(preclickSpy.called).to.be(false); + done(); + } + }); + var mouse = hand.growFinger('mouse'); + + // We move 5 pixels first to overcome the 3-pixel threshold of + // L.Draggable. + mouse.moveTo(300, 280, 0) + .down().moveBy(5, 0, 20).moveBy(20, 20, 100).up(); + }); + + it("does not trigger preclick nor click when dragging a marker", function (done) { + var map = new L.Map(container, { + dragging: true, + inertia: false + }); + map.setView([0, 0], 1); + var marker = L.marker(map.getCenter(), {draggable: true}).addTo(map); + var clickSpy = sinon.spy(); + var preclickSpy = sinon.spy(); + var markerDragSpy = sinon.spy(); + var mapDragSpy = sinon.spy(); + map.on('click', clickSpy); + map.on('preclick', preclickSpy); + map.on('drag', mapDragSpy); + marker.on('click', clickSpy); + marker.on('preclick', preclickSpy); + marker.on('drag', markerDragSpy); + + var hand = new Hand({ + timing: 'fastframe', + onStop: function () { + // A real user scenario would trigger a click on mouseup. + // We want to be sure we are cancelling it after a drag. + happen.click(marker._icon); + expect(markerDragSpy.called).to.be(true); + expect(mapDragSpy.called).to.be(false); + expect(clickSpy.called).to.be(false); + expect(preclickSpy.called).to.be(false); + done(); + } + }); + var mouse = hand.growFinger('mouse'); + + // We move 5 pixels first to overcome the 3-pixel threshold of + // L.Draggable. + mouse.moveTo(300, 280, 0) + .down().moveBy(5, 0, 20).moveBy(50, 50, 100).up(); + }); + it("does not change the center of the map when drag is disabled on click", function (done) { var container = document.createElement('div'); container.style.width = container.style.height = '600px'; @@ -155,6 +263,7 @@ describe("Map.Drag", function () { mouse.wait(100).moveTo(200, 200, 0) .down().moveBy(5, 0, 20).moveBy(256, 32, 200).up(); }); + }); describe("touch events", function () { @@ -291,6 +400,7 @@ describe("Map.Drag", function () { .down().moveBy(210, 0, 1000).up(200); }); + }); }); diff --git a/src/map/Map.js b/src/map/Map.js index 5d7e6c71..d8630764 100644 --- a/src/map/Map.js +++ b/src/map/Map.js @@ -1043,7 +1043,7 @@ L.Map = L.Evented.extend({ }, _draggableMoved: function (obj) { - obj = obj.options.draggable ? obj : this; + obj = obj.dragging && obj.dragging.enabled() ? obj : this; return (obj.dragging && obj.dragging.moved()) || (this.boxZoom && this.boxZoom.moved()); },