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

Jump to bottom

Themes: Record pageView() on Details page #3420

Closed
wants to merge 3 commits into from
Closed

Themes: Record pageView() on Details page #3420

wants to merge 3 commits into from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Feb 19, 2016

Add a pageView.record() when hitting a Details page (/themes/:slug).

Also, add /type/:tier to /design's pageView.record() path.

Please review for wording -- see my comments.

To test:

  • In the console: type localStorage.setItem( 'debug', 'calypso:analytics*' );
  • Land on a theme details page (try directly, and via a theme's 'Details' link). Verify that an event like Recording Page View ~ [URL: /themes/owari/:slug] [Title: Themes > Details Sheet] is tracked.

TODO:

@ockham ockham added [Feature Group] Appearance & Themes Features related to the appearance of sites. [Status] In Progress [Feature] Tracks Metrics & Monitoring Capturing analytics about user behavior on WordPress.com. labels Feb 19, 2016
@ockham ockham self-assigned this Feb 19, 2016
@ockham ockham force-pushed the update/themes-fix-analytics branch from 4dd093d to 55670e8 Compare February 19, 2016 21:57
@ockham ockham changed the title Themes: Fix analytics Themes: Record pageView() on Details page Feb 19, 2016
ockham
ockham reviewed Feb 19, 2016
View reviewed changes
client/my-sites/themes/controller.js
@@ -113,17 +113,30 @@ export function loggedOut( context, next ) {
next();
}

function getDetailsProps( context ) {
const { slug } = context.params;
const basePath = '/themes/:slug';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

:slug? :id? :theme?

Copy link
Member

Choose a reason for hiding this comment

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

Slug is fine.

@ockham ockham force-pushed the update/themes-fix-analytics branch from b7ed1c1 to 17fbcc0 Compare February 23, 2016 00:19
@ockham ockham mentioned this pull request Feb 23, 2016
Closed
4 tasks
@ockham ockham force-pushed the update/themes-fix-analytics branch 2 times, most recently from 4b0ddcc to 17fbcc0 Compare June 13, 2016 22:12
@ockham ockham force-pushed the update/themes-fix-analytics branch from 17fbcc0 to 55f343d Compare June 13, 2016 22:16
@ockham ockham mentioned this pull request Jun 13, 2016
Merged
@ockham
Copy link
Contributor Author

ockham commented Jun 13, 2016

Superseded by #6005.

@ockham ockham closed this Jun 13, 2016
@lancewillett lancewillett deleted the update/themes-fix-analytics branch September 2, 2016 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ockham ockham

Labels
[Feature Group] Appearance & Themes Features related to the appearance of sites. [Feature] Tracks Metrics & Monitoring Capturing analytics about user behavior on WordPress.com.
Projects
None yet
Milestone
Themes: Showcase M4-ThemeSheets
Development

Successfully merging this pull request may close these issues.
3 participants
@ockham @ehg @matticbot