Skip to content

Feature/tracking module - #3

Merged
sblaurock merged 60 commits into
masterfrom
feature/tracking-module
Oct 15, 2019
Merged

Feature/tracking module#3
sblaurock merged 60 commits into
masterfrom
feature/tracking-module

Conversation

@sblaurock

@sblaurock sblaurock commented Oct 1, 2019

Copy link
Copy Markdown
Contributor

5 tracking methods + tests implemented.

Storage library (store2) used for interactions with local and session storage. Tested to verify that no error is thrown in the case of QuotaExceeded error (thanks to overflow add-on).

TODO

  • Descriptions / types for tracking parameters (both in code via JSDoc and in README)
  • Decide which tracking parameters can be labeled as optional

FOLLOW-UP

  • Implement error event emitter to expose tracking request failures to client

All tests pass (147/147).

Screen Shot 2019-10-13 at 11 22 09 AM

Screen Shot 2019-10-13 at 11 22 20 AM

…r tracking module to match browser-tracking.
@sblaurock
sblaurock requested review from jjl014 and rubikzube October 13, 2019 17:46
@rubikzube

Copy link
Copy Markdown
Contributor

I'd like to pair together on structural changes to increase legibility before merging. It'll probably take a entire pairing session. Things like renaming and shifting files/functions around.

Comment thread README.md Outdated

#### Send autocomplete search event
```javascript
constructorio.tracker.sendAutocompleteSearch('dogs', {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'll need to rename to trackSearchSubmit for consistency with other UI code.

Comment thread README.md Outdated

#### Send autocomplete select event
```javascript
constructorio.tracker.sendAutocompleteSelect('dogs', {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'll want to rename all of the sendXxxEvents to trackXxxEvents in the name of consistency with other code.

Comment thread README.md Outdated

#### Send search results event
```javascript
constructorio.tracker.sendSearchResults('dogs', {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'll want to rename sendSearchResults to trackSearchResultsLoaded for consistency with other UI code in iOS/swift/autocomplete

Comment thread src/modules/autocomplete.js Outdated
@@ -1,8 +1,8 @@
/* eslint-disable object-curly-newline */
/* eslint-disable object-curly-newline, no-underscore-dangle */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we pull changes to autocomplete / search / recommendations into a separate PR?

@sblaurock
sblaurock requested a review from rubikzube October 15, 2019 18:14
@sblaurock
sblaurock merged commit a62b10c into master Oct 15, 2019
@sblaurock
sblaurock deleted the feature/tracking-module branch October 15, 2019 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants