Skip to content

Making Downloads Trackable (see #1141)#1148

Merged
fhemberger merged 2 commits into
masterfrom
pr/1141
Feb 22, 2017
Merged

Making Downloads Trackable (see #1141)#1148
fhemberger merged 2 commits into
masterfrom
pr/1141

Conversation

@fhemberger
Copy link
Copy Markdown
Contributor

@fhemberger fhemberger commented Feb 22, 2017

See #1141 for discussion.

ZibbyKeaton and others added 2 commits February 13, 2017 17:32
All - I updated the long form case studies and the 2016 survey report to make it so I can track on Google Analytics. Let me know if this looks good or what changes I need to make here.
Updated version of PR #1141, adds a global JS helper: Links with the
attribute `data-casestudy` will be tracked automatically, using the value of
the data attribute as title in Google Analytics.
s=o.getElementsByTagName(e)[0];j.async=1;j.src='//www.google-analytics.com/analytics.js';
s.parentNode.insertBefore(j,s)}(window,document,'ga','script');

if (!ga) return;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this redundant? The above IIFE should always add the global ga no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you use an ad-blocker, the GA script will not be loaded and thus the ga object doesn't exist.

* [Read the full announcement.](/en/blog/announcements/nodejs-foundation-survey/)
* [See the infographic.](/static/documents/2016-survey-infographic.png)
* [Read the full report.](/static/documents/2016-survey-report.pdf)
* <a href="/static/documents/2016-survey-report.pdf" data-casestudy="Node.js 2016 Report">Read the full report.</a>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a case of study?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was treated that way in the original PR. ¯_(ツ)_/¯

@fhemberger fhemberger merged commit 3ae39be into master Feb 22, 2017
@fhemberger fhemberger deleted the pr/1141 branch February 22, 2017 16:41
@phillipj
Copy link
Copy Markdown
Member

Just tested this on nodejs.org and doesn't seem to work, at least not in Chrome on my machine. There is no request sent to Google when clicking these links.

I assume that is primarily caused by this error:

screenshot1

I'll have a look and open a PR shortly.

@mikeal
Copy link
Copy Markdown
Contributor

mikeal commented Feb 22, 2017

This is great, thanks for cleaning this up 👍

@lpinca
Copy link
Copy Markdown
Member

lpinca commented Feb 22, 2017

Ops, that return seems to be outside a function so it should be probably be changed to if (ga) { ... }. Sorry missed that during review.

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.

5 participants