Navigation Menu

Skip to content

Commit

Permalink
Fix canvas not filtering click event after drag (#4493)
Browse files Browse the repository at this point in the history
* Fix preclick not fired in canvas
* Do not add canvas layers to click event targets if just dragged (fix #4458)
  • Loading branch information
yohanboniface authored and mourner committed Jun 1, 2016
1 parent bb7d23d commit 95d5b59
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 12 deletions.
45 changes: 45 additions & 0 deletions spec/suites/layer/vector/CanvasSpec.js
Expand Up @@ -73,6 +73,51 @@ describe('Canvas', function () {
map.off("click", spy);
});

it("should fire preclick before click", function () {
var clickSpy = sinon.spy();
var preclickSpy = sinon.spy();
layer.on('click', clickSpy);
layer.on('preclick', preclickSpy);
layer.once('preclick', function (e) {
expect(clickSpy.called).to.be(false);
});
happen.at('click', 50, 50); // Click on the layer.
expect(clickSpy.callCount).to.eql(1);
expect(preclickSpy.callCount).to.eql(1);
happen.at('click', 150, 150); // Click outside layer.
expect(clickSpy.callCount).to.eql(1);
expect(preclickSpy.callCount).to.eql(1);
layer.off();
});

it("should not fire click when dragging the map on top of it", function (done) {
var downSpy = sinon.spy();
var clickSpy = sinon.spy();
var preclickSpy = sinon.spy();
layer.on('click', clickSpy);
layer.on('preclick', preclickSpy);
layer.on('mousedown', downSpy);
var hand = new Hand({
timing: 'fastframe',
onStop: function () {
// Prosthetic does not fire a click when we down+up, but it real world
// browsers would, so let's simulate it.
happen.at('click', 70, 60);
expect(downSpy.called).to.be(true);
expect(clickSpy.called).to.be(false);
expect(preclickSpy.called).to.be(false);
layer.off();
done();
}
});
var mouse = hand.growFinger('mouse');

// We move 5 pixels first to overcome the 3-pixel threshold of
// L.Draggable.
mouse.moveTo(50, 50, 0)
.down().moveBy(20, 10, 200).up();
});

});

describe("#events(interactive=false)", function () {
Expand Down
2 changes: 1 addition & 1 deletion src/layer/vector/Canvas.js
Expand Up @@ -253,7 +253,7 @@ L.Canvas = L.Renderer.extend({

for (var id in this._layers) {
layer = this._layers[id];
if (layer.options.interactive && layer._containsPoint(point)) {
if (layer.options.interactive && layer._containsPoint(point) && !this._map._draggableMoved(layer)) {
L.DomEvent._fakeStop(e);
layers.push(layer);
}
Expand Down
22 changes: 11 additions & 11 deletions src/map/Map.js
Expand Up @@ -990,17 +990,6 @@ L.Map = L.Evented.extend({

var type = e.type === 'keypress' && e.keyCode === 13 ? 'click' : e.type;

if (e.type === 'click') {
// Fire a synthetic 'preclick' event which propagates up (mainly for closing popups).
// @event preclick: MouseEvent
// Fired before mouse click on the map (sometimes useful when you
// want something to happen on click before any existing click
// handlers start running).
var synth = L.Util.extend({}, e);
synth.type = 'preclick';
this._handleDOMEvent(synth);
}

if (type === 'mousedown') {
// prevents outline when clicking on keyboard-focusable element
L.DomUtil.preventOutline(e.target || e.srcElement);
Expand All @@ -1011,6 +1000,17 @@ L.Map = L.Evented.extend({

_fireDOMEvent: function (e, type, targets) {

if (e.type === 'click') {
// Fire a synthetic 'preclick' event which propagates up (mainly for closing popups).
// @event preclick: MouseEvent
// Fired before mouse click on the map (sometimes useful when you
// want something to happen on click before any existing click
// handlers start running).
var synth = L.Util.extend({}, e);
synth.type = 'preclick';
this._fireDOMEvent(synth, synth.type, targets);
}

if (e._stopped) { return; }

// Find the layer the event is propagating from and its parents.
Expand Down

0 comments on commit 95d5b59

Please sign in to comment.