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.

Sign up for GitHub

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

Add third parameter to once event handler #8875

Merged
merged 10 commits into from
Jan 7, 2020
Merged

Add third parameter to once event handler #8875

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6d7277a
adds third parameter to once event handler
malwoodsantoro Oct 15, 2019
64bd5cb
refactor
malwoodsantoro Nov 2, 2019
c73f64f
fix lint error
malwoodsantoro Nov 5, 2019
b0d3555
fix flow error
malwoodsantoro Nov 5, 2019
6becd9a
change super.on to super.once
malwoodsantoro Nov 12, 2019
e5492f7
remove one time delegated listeners on off
malwoodsantoro Dec 10, 2019
c373a75
remove comments
malwoodsantoro Dec 10, 2019
4d5172f
add docs for once event
malwoodsantoro Dec 17, 2019
3e0cc73
utilized the _oneTimeListeners array in Evented
malwoodsantoro Dec 17, 2019
ffe7d5d
Docs fixes per review
Dec 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 100 additions & 49 deletions src/ui/map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,54 @@ class Map extends Camera {
this.dragRotate.isActive();
}

_createDelegatedListener(type: MapEvent, layerId: any, listener: any) {
if (type === 'mouseenter' || type === 'mouseover') {
let mousein = false;
const mousemove = (e) => {
const features = this.getLayer(layerId) ? this.queryRenderedFeatures(e.point, {layers: [layerId]}) : [];
if (!features.length) {
mousein = false;
} else if (!mousein) {
mousein = true;
listener.call(this, new MapMouseEvent(type, this, e.originalEvent, {features}));
}
};
const mouseout = () => {
mousein = false;
};
return {layer: layerId, listener, delegates: {mousemove, mouseout}};
kkaefer marked this conversation as resolved.
Show resolved Hide resolved
} else if (type === 'mouseleave' || type === 'mouseout') {
let mousein = false;
const mousemove = (e) => {
const features = this.getLayer(layerId) ? this.queryRenderedFeatures(e.point, {layers: [layerId]}) : [];
if (features.length) {
mousein = true;
} else if (mousein) {
mousein = false;
listener.call(this, new MapMouseEvent(type, this, e.originalEvent));
}
};
const mouseout = (e) => {
if (mousein) {
mousein = false;
listener.call(this, new MapMouseEvent(type, this, e.originalEvent));
}
};
return {layer: layerId, listener, delegates: {mousemove, mouseout}};
} else {
const delegate = (e) => {
const features = this.getLayer(layerId) ? this.queryRenderedFeatures(e.point, {layers: [layerId]}) : [];
if (features.length) {
// Here we need to mutate the original event, so that preventDefault works as expected.
e.features = features;
listener.call(this, e);
delete e.features;
}
};
return {layer: layerId, listener, delegates: {[type]: delegate}};
}
}

/**
* Adds a listener for events of a specified type.
*
Expand Down Expand Up @@ -776,53 +824,7 @@ class Map extends Camera {
return super.on(type, layerId);
}

const delegatedListener = (() => {
if (type === 'mouseenter' || type === 'mouseover') {
let mousein = false;
const mousemove = (e) => {
const features = this.getLayer(layerId) ? this.queryRenderedFeatures(e.point, {layers: [layerId]}) : [];
if (!features.length) {
mousein = false;
} else if (!mousein) {
mousein = true;
listener.call(this, new MapMouseEvent(type, this, e.originalEvent, {features}));
}
};
const mouseout = () => {
mousein = false;
};
return {layer: layerId, listener, delegates: {mousemove, mouseout}};
} else if (type === 'mouseleave' || type === 'mouseout') {
let mousein = false;
const mousemove = (e) => {
const features = this.getLayer(layerId) ? this.queryRenderedFeatures(e.point, {layers: [layerId]}) : [];
if (features.length) {
mousein = true;
} else if (mousein) {
mousein = false;
listener.call(this, new MapMouseEvent(type, this, e.originalEvent));
}
};
const mouseout = (e) => {
if (mousein) {
mousein = false;
listener.call(this, new MapMouseEvent(type, this, e.originalEvent));
}
};
return {layer: layerId, listener, delegates: {mousemove, mouseout}};
} else {
const delegate = (e) => {
const features = this.getLayer(layerId) ? this.queryRenderedFeatures(e.point, {layers: [layerId]}) : [];
if (features.length) {
// Here we need to mutate the original event, so that preventDefault works as expected.
e.features = features;
listener.call(this, e);
delete e.features;
}
};
return {layer: layerId, listener, delegates: {[type]: delegate}};
}
})();
const delegatedListener = this._createDelegatedListener(type, layerId, listener);

this._delegatedListeners = this._delegatedListeners || {};
this._delegatedListeners[type] = this._delegatedListeners[type] || [];
Expand All @@ -835,6 +837,51 @@ class Map extends Camera {
return this;
}

/**
* Adds a listener for events of a specified type.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should match the comment in evented

Adds a listener that will be called only once to a specified event type.

*
* @method
* @name once
* @memberof Map
* @instance
* @param {string} type The event type to add a listen for.
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: listener not listen

* @param {Function} listener The function to be called when the event is fired.
* The listener function is called with the data object passed to `fire`,
* extended with `target` and `type` properties.
* @returns {Map} `this`
*/

/**
* Adds a listener for events of a specified type occurring on features in a specified style layer.
Copy link
Contributor

Choose a reason for hiding this comment

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

And this one should basically match the comment above but with the addition of occurring on features in a specified style layer

*
* @param {string} type The event type to listen for; one of `'mousedown'`, `'mouseup'`, `'click'`, `'dblclick'`,
* `'mousemove'`, `'mouseenter'`, `'mouseleave'`, `'mouseover'`, `'mouseout'`, `'contextmenu'`, `'touchstart'`,
* `'touchend'`, or `'touchcancel'`. `mouseenter` and `mouseover` events are triggered when the cursor enters
* a visible portion of the specified layer from outside that layer or outside the map canvas. `mouseleave`
* and `mouseout` events are triggered when the cursor leaves a visible portion of the specified layer, or leaves
* the map canvas.
* @param {string} layerId The ID of a style layer. Only events whose location is within a visible
* feature in this layer will trigger the listener. The event will have a `features` property containing
* an array of the matching features.
* @param {Function} listener The function to be called when the event is fired.
* @returns {Map} `this`
*/

once(type: MapEvent, layerId: any, listener: any) {

if (listener === undefined) {
return super.once(type, layerId);
}

const delegatedListener = this._createDelegatedListener(type, layerId, listener);

for (const event in delegatedListener.delegates) {
this.once((event: any), delegatedListener.delegates[event]);
}

return this;
}

/**
* Removes an event listener previously added with `Map#on`.
*
Expand All @@ -860,8 +907,8 @@ class Map extends Camera {
return super.off(type, layerId);
}

if (this._delegatedListeners && this._delegatedListeners[type]) {
const listeners = this._delegatedListeners[type];
const removeDelegatedListener = (delegatedListeners) => {
const listeners = delegatedListeners[type];
for (let i = 0; i < listeners.length; i++) {
const delegatedListener = listeners[i];
if (delegatedListener.layer === layerId && delegatedListener.listener === listener) {
Expand All @@ -872,6 +919,10 @@ class Map extends Camera {
return this;
}
}
};

if (this._delegatedListeners && this._delegatedListeners[type]) {
removeDelegatedListener(this._delegatedListeners);
}

return this;
Expand Down
2 changes: 1 addition & 1 deletion src/util/evented.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class Evented {
* @param {Function} listener The function to be called when the event is fired the first time.
* @returns {Object} `this`
*/
once(type: string, listener: Listener) {
once(type: *, listener: Listener) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should remain string. Any reason this had to change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kkaefer flow fails without * and on currently uses * as well (assuming for similar reasons).

Copy link
Member

Choose a reason for hiding this comment

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

Ah, flow seems to have trouble matching a string to a string literal :/

this._oneTimeListeners = this._oneTimeListeners || {};
_addEventListener(type, listener, this._oneTimeListeners);

Expand Down