-
Notifications
You must be signed in to change notification settings - Fork 179
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
Event logging update #212
Merged
Merged
Event logging update #212
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0d6a0bd
add keystroke event
7391edc
add fields to selection event
2019e92
add events tests
4e364be
Merge branch 'version4' of github.com:mapbox/mapbox-gl-geocoder into …
60bf39d
change event method name
710fbc2
update changelog with proposed changes
301eb30
do not debounce keyevents
f2ccd56
batch events together to minimize network calls
17cd846
add and cleanup tests
8ba0600
expose event opt-out constructor option
b8104fd
Add documentation for event opt-out option
cd5682b
pull version4 upstream changes and resolve conflicts
f6ad56f
Change according to @ingalls review
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,15 +5,22 @@ var nanoid = require('nanoid') | |
* Construct a new mapbox event client to send interaction events to the mapbox event service | ||
* @param {Object} options options with which to create the service | ||
* @param {String} options.accessToken the mapbox access token to make requests | ||
* @param {Number} [options.flushInterval=1000] the number of ms after which to flush the event queue | ||
* @param {Number} [options.maxQueueSize=100] the number of events to queue before flushing | ||
* @private | ||
*/ | ||
function MapboxEventManager(options) { | ||
this.origin = options.origin || 'https://api.mapbox.com'; | ||
this.endpoint = '/events/v2'; | ||
this.endpoint = 'events/v2'; | ||
this.access_token = options.accessToken; | ||
this.version = '0.0.1' | ||
this.version = '0.2.0' | ||
this.sessionID = this.generateSessionID(); | ||
this.userAgent = this.getUserAgent(); | ||
|
||
this.options = options; | ||
this.send = this.send.bind(this); | ||
|
||
|
||
// parse global options to be sent with each request | ||
this.countries = (options.countries) ? options.countries.split(",") : null; | ||
this.types = (options.types) ? options.types.split(",") : null; | ||
|
@@ -22,47 +29,71 @@ function MapboxEventManager(options) { | |
this.limit = (options.limit) ? +options.limit : null; | ||
this.locale = navigator.language || null; | ||
this.enableEventLogging = this.shouldEnableLogging(options); | ||
this.eventQueue = new Array(); | ||
this.flushInterval = options.flushInterval || 1000; | ||
this.maxQueueSize = options.maxQueueSize || 100; | ||
this.timer = (this.flushInterval) ? setTimeout(this.flush.bind(this), this.flushInterval) : null; | ||
// keep some state to deduplicate requests if necessary | ||
this.lastSentInput = ""; | ||
this.lastSentIndex = 0; | ||
} | ||
|
||
MapboxEventManager.prototype = { | ||
|
||
/** | ||
* Send a search.select event to the mapbox events service | ||
* This event marks the array index of the item selected by the user out of the array of possible options | ||
* @private | ||
* @param {Object} selected the geojson feature selected by the user | ||
* @param {Object} geocoder a mapbox-gl-geocoder instance | ||
* @param {Function} callback a callback function to invoke once the event has been sent (optional) | ||
* @returns {Promise} | ||
*/ | ||
select: function (selected, geocoder, callback) { | ||
* Send a search.select event to the mapbox events service | ||
* This event marks the array index of the item selected by the user out of the array of possible options | ||
* @private | ||
* @param {Object} selected the geojson feature selected by the user | ||
* @param {Object} geocoder a mapbox-gl-geocoder instance | ||
* @returns {Promise} | ||
*/ | ||
select: function(selected, geocoder){ | ||
var resultIndex = this.getSelectedIndex(selected, geocoder); | ||
var payload = this.getEventPayload('search.select', geocoder); | ||
payload.resultIndex = resultIndex; | ||
payload.resultPlaceName = selected.place_name; | ||
payload.resultId = selected.id; | ||
if ((resultIndex === this.lastSentIndex && payload.queryString === this.lastSentInput) || resultIndex == -1) { | ||
// don't log duplicate events if the user re-selected the same feature on the same search | ||
if (callback) return callback(); | ||
else return; | ||
return; | ||
} | ||
this.lastSentIndex = resultIndex; | ||
this.lastSentInput = payload.queryString; | ||
return this.send(payload, callback) | ||
if (!payload.queryString) return; // will be rejected | ||
return this.push(payload) | ||
}, | ||
|
||
/** | ||
* Send a search-start event to the mapbox events service | ||
* This turnstile event marks when a user starts a new search | ||
* Send a search-start event to the mapbox events service | ||
* This turnstile event marks when a user starts a new search | ||
* @private | ||
* @param {Object} geocoder a mapbox-gl-geocoder instance | ||
* @returns {Promise} | ||
*/ | ||
start: function(geocoder){ | ||
var payload = this.getEventPayload('search.start', geocoder); | ||
if (!payload.queryString) return; // will be rejected | ||
return this.push(payload); | ||
}, | ||
|
||
/** | ||
* Send a search-keyevent event to the mapbox events service | ||
* This event records each keypress in sequence | ||
* @private | ||
* @param {Object} geocoder a mapbox-gl-geocoder instance | ||
* @param {Function} callback | ||
* @returns {Promise} | ||
* @param {Object} keyEvent the keydown event to log | ||
* @param {Obeject} geocoder a mapbox-gl-geocoder instance | ||
* | ||
*/ | ||
start: function (geocoder, callback) { | ||
var payload = this.getEventPayload('search.start', geocoder); | ||
return this.send(payload, callback); | ||
keyevent: function(keyEvent, geocoder){ | ||
//pass invalid event | ||
if (!keyEvent.key) return; | ||
// don't send events for keys that don't change the input | ||
// TAB, ESC, LEFT, RIGHT, ENTER, UP, DOWN | ||
if (keyEvent.metaKey || [9, 27, 37, 39, 13, 38, 40].indexOf(keyEvent.keyCode) !== -1) return; | ||
var payload = this.getEventPayload('search.keystroke', geocoder); | ||
payload.lastAction = keyEvent.key; | ||
if (!payload.queryString) return; // will be rejected | ||
return this.push(payload); | ||
}, | ||
|
||
/** | ||
|
@@ -72,27 +103,29 @@ MapboxEventManager.prototype = { | |
* | ||
* @private | ||
* @param {Object} payload the http POST body of the event | ||
* @param {Function} [callback] a callback function to invoke when the send has completed | ||
* @returns {Promise} | ||
*/ | ||
send: function (payload, callback) { | ||
if (!callback) callback = function () { | ||
return | ||
}; | ||
if (!this.enableEventLogging) { | ||
return callback(); | ||
if (callback) return callback(); | ||
return; | ||
} | ||
var options = this.getRequestOptions(payload); | ||
this.request(options, function (err) { | ||
this.request(options, function(err){ | ||
if (err) return this.handleError(err, callback); | ||
if (callback) return callback(); | ||
}) | ||
if (callback) { | ||
return callback(); | ||
} | ||
}.bind(this)) | ||
}, | ||
/** | ||
* Get http request options | ||
* @private | ||
* @param {*} payload | ||
*/ | ||
getRequestOptions: function (payload) { | ||
getRequestOptions: function(payload){ | ||
if (!Array.isArray(payload)) payload = [payload]; | ||
var options = { | ||
// events must be sent with POST | ||
method: "POST", | ||
|
@@ -101,7 +134,7 @@ MapboxEventManager.prototype = { | |
headers: { | ||
'Content-Type': 'application/json' | ||
}, | ||
body: JSON.stringify([payload]) //events are arrays | ||
body:JSON.stringify(payload) //events are arrays | ||
} | ||
return options | ||
}, | ||
|
@@ -134,7 +167,7 @@ MapboxEventManager.prototype = { | |
proximity: proximity, | ||
limit: geocoder.options.limit, | ||
// routing: search.routing, //todo --> add to plugin | ||
queryString: geocoder.inputString, | ||
queryString: (geocoder._inputEl) ? geocoder._inputEl.value : geocoder.inputString, | ||
mapZoom: zoom, | ||
keyboardLocale: this.locale | ||
} | ||
|
@@ -159,7 +192,8 @@ MapboxEventManager.prototype = { | |
} | ||
} | ||
}; | ||
xhttp.open(opts.method, opts.host + opts.path, true); | ||
|
||
xhttp.open(opts.method, opts.host + '/' + opts.path, true); | ||
for (var header in opts.headers){ | ||
var headerValue = opts.headers[header]; | ||
xhttp.setRequestHeader(header, headerValue) | ||
|
@@ -194,13 +228,14 @@ MapboxEventManager.prototype = { | |
}, | ||
|
||
/** | ||
* Get the 0-based numeric index of the item that the user selected out of the list of options | ||
* @private | ||
* @param {Object} selected the geojson feature selected by the user | ||
* @param {Object} geocoder a Mapbox-GL-Geocoder instance | ||
* @returns {Number} the index of the selected result | ||
*/ | ||
getSelectedIndex: function (selected, geocoder) { | ||
* Get the 0-based numeric index of the item that the user selected out of the list of options | ||
* @private | ||
* @param {Object} selected the geojson feature selected by the user | ||
* @param {Object} geocoder a Mapbox-GL-Geocoder instance | ||
* @returns {Number} the index of the selected result | ||
*/ | ||
getSelectedIndex: function(selected, geocoder){ | ||
if (!geocoder._typeahead) return; | ||
var results = geocoder._typeahead.data; | ||
var selectedID = selected.id; | ||
var resultIDs = results.map(function (feature) { | ||
|
@@ -211,17 +246,53 @@ MapboxEventManager.prototype = { | |
}, | ||
|
||
/** | ||
* Check whether events should be logged | ||
* Clients using a localGeocoder or an origin other than mapbox should not have events logged | ||
* @private | ||
*/ | ||
shouldEnableLogging: function (options) { | ||
if (this.origin.indexOf('api.mapbox.com') == -1) return false; | ||
* Check whether events should be logged | ||
* Clients using a localGeocoder or an origin other than mapbox should not have events logged | ||
* @private | ||
*/ | ||
shouldEnableLogging: function(options){ | ||
if (options.enableEventLogging === false) return false; | ||
if (options.origin && options.origin.indexOf('api.mapbox.com') == -1) return false; | ||
// hard to make sense of events when a local instance is suplementing results from origin | ||
if (options.localGeocoder) return false; | ||
// hard to make sense of events when a custom filter is in use | ||
if (options.filter) return false; | ||
return true; | ||
}, | ||
|
||
/** | ||
* Flush out the event queue by sending events to the events service | ||
* @private | ||
*/ | ||
flush: function(){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The JS Doc says this function should have a |
||
if (this.eventQueue.length > 0){ | ||
this.send(this.eventQueue); | ||
this.eventQueue = new Array(); | ||
} | ||
// //reset the timer | ||
if (this.timer) clearTimeout(this.timer); | ||
if (this.flushInterval) this.timer = setTimeout(this.flush.bind(this), this.flushInterval) | ||
}, | ||
|
||
/** | ||
* Push event into the pending queue | ||
* @param {Object} evt the event to send to the events service | ||
* @param {Boolean} forceFlush indicates that the event queue should be flushed after adding this event regardless of size of the queue | ||
* @private | ||
*/ | ||
push: function(evt, forceFlush){ | ||
this.eventQueue.push(evt); | ||
if (this.eventQueue.length >= this.maxQueueSize || forceFlush){ | ||
this.flush(); | ||
} | ||
}, | ||
|
||
/** | ||
* Flush any remaining events from the queue before it is removed | ||
* @private | ||
*/ | ||
remove: function(){ | ||
this.flush(); | ||
} | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment with the actual keys that these codes correspond to