Skip to content

Add link tracking #951

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

Merged

Conversation

muffinresearch
Copy link
Contributor

@kumar303
Copy link
Contributor

Looks like you'll need to rebase this on master to resolve the conflict and also to check that the new lint rules aren't nagging you about anything.

@muffinresearch muffinresearch force-pushed the instrument-addon-clicks branch from 7b89dc7 to 084224c Compare August 22, 2016 09:29
@muffinresearch muffinresearch assigned mstriemer and unassigned kumar303 Aug 22, 2016
@@ -102,3 +106,10 @@ export function loadAddonIfNeeded(
return fetchAddon({ slug, api: state.api })
.then(({ entities }) => dispatch(loadEntities(entities)));
}

export function getAction(type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this should be in core/tracking. It might make sense to specify that this is a tracking action as well by naming it trackingAction or getTrackingAction since "action" is a term used in redux as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, if this were in core/tracking it could probably be part of tracking.sendEvent:

export function sendEvent({ category, action, label, type, value } = {}) {
  if (type !== undefined) {
    action = getAction(type);  // This won't pass eslint but you know.
  }
  // ...
}

@mstriemer
Copy link
Contributor

I can see a case for keeping getAction out of tracking.sendEvent since it is likely specific to extensions and themes but changing it's module might still be a good idea if that's the case.

r+wc

@muffinresearch muffinresearch force-pushed the instrument-addon-clicks branch from 084224c to 0068795 Compare August 22, 2016 20:17
@muffinresearch muffinresearch merged commit f7f83b2 into mozilla:master Aug 23, 2016
@muffinresearch muffinresearch deleted the instrument-addon-clicks branch August 23, 2016 11:23
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.

Instrument clicks on Add-on names in Disco Pane
3 participants