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

Set analytics custom dimensions from `govuk:` meta tags #558

Merged
merged 3 commits into from Mar 13, 2015
Merged

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Mar 13, 2015

Related to alphagov/slimmer#119

  • Find govuk: namespaced meta tags and use the content of these tags to set dimensions
  • Be explicit about what's being read and set
  • Works with both old and new slimmer versions but can be simplified once apps are all on version 8.0.0 or greater
fofr added 2 commits Mar 10, 2015
* Find `govuk:` namespaced meta tags
* Use content of these tags to set dimensions
* Be explicit about what's being read and set
* Works with both old and new slimmer versions
Only version below 8.0.0 insert these old custom variables. When all
apps have been updated to use a modern slimmer this template can be
removed entirely.
@@ -78,6 +79,26 @@
tracker.setDimension(customVar[1], customVar[3], customVar[2], customVar[4]);
}

function setDimensionsFromMetaTags() {

This comment has been minimized.

@edds

edds Mar 13, 2015
Contributor

Is there any reason this method can't be added to the prototype? That would save the awkward apply(this) line above and would better follow the approach we suggest in the JavaScript styleguide.

This comment has been minimized.

@fofr

fofr Mar 13, 2015
Author Contributor

I saw no reason to add setDimensionsFromMetaTags to the public api as its only use is during initialisation.

This comment has been minimized.

@edds

edds Mar 13, 2015
Contributor

Adding it to the public api would let us add proper unit tests to just this method. Which if it is private we can't do. Also while there is no need for this to be added to the public api there is also no harm in it being public. I would lean on the side of making JavaScript methods public unless there is a good reason not to.

This comment has been minimized.

@fofr

fofr Mar 13, 2015
Author Contributor

Changed in 11f0014

* Avoid need for `.apply`
* Be more lenient in what can access this method
edds added a commit that referenced this pull request Mar 13, 2015
Set analytics custom dimensions from `govuk:` meta tags
@edds edds merged commit 2dbfc63 into master Mar 13, 2015
1 check passed
1 check passed
default "Build #419 succeeded on Jenkins"
Details
@edds edds deleted the slimmer-meta-tags branch Mar 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.