Skip to content

Commit

Permalink
don't fire click event if default is prevented on mousedown for a dra…
Browse files Browse the repository at this point in the history
…g event (#6697)
  • Loading branch information
mollymerp authored May 21, 2018
1 parent 695ed28 commit 6cbfe8c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/ui/bind_handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export default function bindHandlers(map: Map, options: {interactive: boolean})
const el = map.getCanvasContainer();
let contextMenuEvent = null;
let mouseDown = false;
let startPos = null;

for (const name in handlers) {
(map: any)[name] = new handlers[name](map, options);
Expand Down Expand Up @@ -56,6 +57,7 @@ export default function bindHandlers(map: Map, options: {interactive: boolean})

function onMouseDown(e: MouseEvent) {
mouseDown = true;
startPos = DOM.mousePos(el, e);

const mapEvent = new MapMouseEvent('mousedown', map, e);
map.fire(mapEvent);
Expand Down Expand Up @@ -149,7 +151,10 @@ export default function bindHandlers(map: Map, options: {interactive: boolean})
}

function onClick(e: MouseEvent) {
map.fire(new MapMouseEvent('click', map, e));
const pos = DOM.mousePos(el, e);
if (pos.equals(startPos)) {

This comment has been minimized.

Copy link
@Ready4theCrush

Ready4theCrush Jun 13, 2018

This logic essentially prevents the 'click' event from every firing (at least for me) I have a simple map with markers. Even if I leave the mouse in place over a marker and click the button without touching the track pad, the map still logs a slight movement and the 'click' event doesn't fire, no matter what. I've gone back to 0.44.2 to get the click events to fire, but wanted to let y'all know :-)

This comment has been minimized.

Copy link
@Ready4theCrush

Ready4theCrush Jun 13, 2018

Here's a great test from the examples pages:
https://www.mapbox.com/mapbox-gl-js/example/popup-on-click/
For me, none of these do anything when I click on them.

This comment has been minimized.

Copy link
@jfirebaugh

jfirebaugh Jun 13, 2018

Contributor

This was fixed in #6757.

This comment has been minimized.

Copy link
@Ready4theCrush

Ready4theCrush Jun 13, 2018

Awesome, thanks!

map.fire(new MapMouseEvent('click', map, e));
}
}

function onDblClick(e: MouseEvent) {
Expand Down
9 changes: 9 additions & 0 deletions test/node_modules/mapbox-gl-js-test/simulate_interaction.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions test/unit/ui/map_events.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -544,3 +544,19 @@ test(`Map#on mousedown can have default behavior prevented and still fire subseq
map.remove();
t.end();
});

test(`Map#on mousedown doesn't fire subsequent click event if mousepos changes`, (t) => {
const map = createMap();

map.on('mousedown', e => e.preventDefault());

const click = t.spy();
map.on('click', click);
const canvas = map.getCanvas();

simulate.drag(canvas, {}, {clientX: 100, clientY: 100});
t.ok(click.notCalled);

map.remove();
t.end();
});

0 comments on commit 6cbfe8c

Please sign in to comment.