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

Added mixpanel tracking for some basic events #2462

Merged
merged 6 commits into from
Apr 27, 2017
Merged

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Apr 24, 2017

Addresses #2460 by making use of Service UI's configured window.mixpanel variable directly.

Tracked events
  • scope.layout.refresh.click
  • scope.layout.selector.click
  • scope.layout.selector.keypress
  • scope.metric.selector.pin.click
  • scope.metric.selector.pin.next.keypress
  • scope.metric.selector.pin.previous.keypress
  • scope.metric.selector.unpin.click
  • scope.metric.selector.unpin.keypress
  • scope.node.click
  • scope.node.relative.click
  • scope.search.query.change
  • scope.search.query.pin
  • scope.topology.option.click
  • scope.topology.selector.click

Most of them have the active topology and view mode (layout) props attached to them to have some context of where the events are coming from. I checked it on the mixpanel dashboard and it seems to work nicely!

Other changes
  • Replaced the hitEnter action with pinSearch which gets called directly from the search.js, since we only need to handle the ENTER key press from the search bar.
  • Disable r shortcut when the resource view is disabled.

@fbarl fbarl self-assigned this Apr 24, 2017
@fbarl fbarl force-pushed the add-mixpanel-tracking branch from 07606f1 to 7e5eca7 Compare April 24, 2017 16:51
@fbarl fbarl requested a review from foot April 24, 2017 17:42
@fbarl fbarl changed the title [WIP] Added mixpanel tracking for some basic events Added mixpanel tracking for some basic events Apr 24, 2017
Copy link
Contributor

@foot foot left a comment

Choose a reason for hiding this comment

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

LGTM w/ small changes!

// If the search query is parsable, pin it when ENTER key is hit.
if (ev.keyCode === ENTER_KEY_CODE && parseQuery(this.props.searchQuery)) {
trackMixpanelEvent('scope.search.query.pin', {
query: this.props.searchQuery,

This comment was marked as abuse.

This comment was marked as abuse.

handleFocus() {
this.props.focusSearch();
}

doSearch(value) {
if (value !== '') {
trackMixpanelEvent('scope.search.query.change', {
query: value,

This comment was marked as abuse.

This comment was marked as abuse.

@fbarl fbarl force-pushed the add-mixpanel-tracking branch from 7e5eca7 to 9644b97 Compare April 26, 2017 16:32
@fbarl fbarl force-pushed the add-mixpanel-tracking branch from 78b7728 to 8eaf48a Compare April 27, 2017 10:17
@fbarl
Copy link
Contributor Author

fbarl commented Apr 27, 2017

@foot I addressed your comment and also added some tracking for keypress events (related to the already implemented click ones). Also, there was a tiny bug there that I fixed - pressing r when resource view was not available would still pin the first metric if none was pinned :)

All these changes were rather small so I'll merge this PR after the tests pass without your second look at it.

@fbarl fbarl merged commit 65b9b48 into master Apr 27, 2017
@fbarl fbarl deleted the add-mixpanel-tracking branch June 16, 2017 09:54
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