Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handles firing closeOnClick before another listener #10926

Merged
merged 9 commits into from Aug 12, 2021
12 changes: 12 additions & 0 deletions src/ui/events.js
Expand Up @@ -43,6 +43,7 @@ export class MapMouseEvent extends Event {
*/
type: 'mousedown'
| 'mouseup'
| 'preclick'
| 'click'
| 'dblclick'
| 'mousemove'
Expand Down Expand Up @@ -575,6 +576,17 @@ export type MapEvent =
*/
| 'mousemove'

/**
* Triggered when a click event occurs and is fired before the click event.
* Primarily implemented to ensure closeOnClick for pop-ups is fired before any other listeners.
*
* @event preclick
* @memberof Map
* @instance
* @type {MapMouseEvent}
*/
| 'preclick'

/**
* Fired when a pointing device (usually a mouse) is pressed and released at the same point on the map.
*
Expand Down
5 changes: 5 additions & 0 deletions src/ui/handler/map_event.js
Expand Up @@ -38,8 +38,13 @@ export class MapEventHandler {
this._map.fire(new MapMouseEvent(e.type, this._map, e));
}

preclick(e: MouseEvent) {
this._map.fire(new MapMouseEvent(e.type, this._map, e));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.type will be equal to click here (propagating from the DOM event of the same name) so looks like it'll fire click twice — I think we want to explicitly specify preclick here instead.

}

click(e: MouseEvent, point: Point) {
if (this._mousedownPos && this._mousedownPos.dist(point) >= this._clickTolerance) return;
this.preclick(e);
this._map.fire(new MapMouseEvent(e.type, this._map, e));
}

Expand Down
2 changes: 1 addition & 1 deletion src/ui/popup.js
Expand Up @@ -140,7 +140,7 @@ export default class Popup extends Evented {

this._map = map;
if (this.options.closeOnClick) {
this._map.on('click', this._onClose);
this._map.on('preclick', this._onClose);
}

if (this.options.closeOnMove) {
Expand Down
5 changes: 0 additions & 5 deletions src/util/evented.js
Expand Up @@ -125,11 +125,6 @@ export class Evented {
// make sure adding or removing listeners inside other listeners won't cause an infinite loop
const listeners = this._listeners && this._listeners[type] ? this._listeners[type].slice() : [];

// Check if onClose is an event listener and move to front of list to fire first
if (listeners.length > 1 && listeners[1].name === "bound _onClose") {
listeners.unshift(listeners.splice(1, 1)[0]);
}

for (const listener of listeners) {
listener.call(this, event);
}
Expand Down
16 changes: 8 additions & 8 deletions test/unit/ui/map_events.test.js
Expand Up @@ -12,7 +12,7 @@ test('Map#on adds a non-delegated event listener', (t) => {
map.on('click', spy);
simulate.click(map.getCanvas());

t.ok(spy.calledOnce);
t.ok(spy.calledTwice);
t.end();
});

Expand Down Expand Up @@ -47,7 +47,7 @@ test('Map#on adds a listener for an event on a given layer', (t) => {
map.on('click', 'layer', spy);
simulate.click(map.getCanvas());

t.ok(spy.calledOnce);
t.ok(spy.calledTwice);
t.end();
});

Expand Down Expand Up @@ -129,8 +129,8 @@ test('Map#on distinguishes distinct layers', (t) => {
map.on('click', 'B', spyB);
simulate.click(map.getCanvas());

t.ok(spyA.calledOnce);
t.ok(spyB.calledOnce);
t.ok(spyA.calledTwice);
t.ok(spyB.calledTwice);
t.end();
});

Expand All @@ -147,8 +147,8 @@ test('Map#on distinguishes distinct listeners', (t) => {
map.on('click', 'layer', spyB);
simulate.click(map.getCanvas());

t.ok(spyA.calledOnce);
t.ok(spyB.calledOnce);
t.ok(spyA.calledTwice);
t.ok(spyB.calledTwice);
t.end();
});

Expand Down Expand Up @@ -206,7 +206,7 @@ test('Map#off distinguishes distinct layers', (t) => {
map.off('click', 'B', spy);
simulate.click(map.getCanvas());

t.ok(spy.calledOnce);
t.ok(spy.calledTwice);
t.end();
});

Expand All @@ -224,7 +224,7 @@ test('Map#off distinguishes distinct listeners', (t) => {
map.off('click', 'layer', spyB);
simulate.click(map.getCanvas());

t.ok(spyA.calledOnce);
t.ok(spyA.calledTwice);
t.ok(spyB.notCalled);
t.end();
});
Expand Down