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

Upgrade govuk_frontend_toolkit #1899

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@chrisroos
Contributor

chrisroos commented Aug 20, 2015

This replaces #1896.

I've upgraded the version of the govuk_frontend_toolkit from 3.1.0 to 4.2.1 so that we can use the GOVUK.analytics.trackEvent function instead of using the ga function directly.

I've been through the govuk_frontend_toolkit changelog and each of the commits following the one that bumped the version to 3.1.0 (see dbba96e).

The most significant change from 3.1.0 appears to be the move of GOVUK.analytics from the static app to govuk_frontend_toolkit. It was added to govuk_frontend_toolkit on 10th Mar 2015 (see PR 162 for more), and removed from static on 12th Mar 2015 (see PR 557 for more).

We're using GOVUK.analytics to track:

  • An event when a user clicks "Change" or "Start again"
  • An event when a user reaches an Outcome page
  • Pageviews as users navigate through the Smart Answer (we call .trackPageView() because the Smart Answer questions are loaded into the page using JavaScript).

For each of these cases I used my local Smart Answers and the Google Analytics Debugger to observe the data being sent to GA before and after this branch. I'm confident that the data remains the same and that this upgrade shouldn't affect our Google Analytics tracking.

chrisroos added some commits Aug 19, 2015

Update to 4.2.1
I've upgraded to the latest version available and specified that we're
compatible with the 4.2.x versions.

The important thing is that we've got the changes introduced in 4.1.0[1] (see
PR #203[2] for more information) that allow us to send the `page` option to
`GOVUK.analytics.trackEvent`[3].

[1]: https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG.md#410

[2]: alphagov/govuk_frontend_toolkit#203

[3]: alphagov/govuk_frontend_toolkit@a65f3d7
Use GOVUK.analytics.trackEvent instead of ga function
Version 4.1.0 of the govuk_frontend_toolkit Gem introduced the ability to
specify the page when calling the `trackEvent` function.

I've upgraded govuk_frontend_toolkit to 4.1.0 which means that we can use
`GOVUK.analytics.trackEvent()` instead of `ga()`.

The use of `ga()` was introduced in ae2a255.
See that commit for more information about why recording the `page` is useful.

I used the Google Analytics Debugger Chrome extension[1] to compare the events
being sent before and after this change:

== Before

Running command: ga("send", {hitType: "event", eventCategory: "Smart Answer",
eventAction: "Completed", eventLabel: "additional-commodity-code",
nonInteraction: 1, page: "/additional-commodity-code/y/75/5/85"})

== After

Running command: ga("send", {hitType: "event", eventCategory: "Smart Answer",
eventAction: "Completed", eventLabel: "additional-commodity-code",
nonInteraction: 1, page: "/additional-commodity-code/y/75/5/85"})

[1]: https://chrome.google.com/webstore/detail/google-analytics-debugger/jnkmfdileelhofjcijamephohjechhna?hl=en
@tadast

This comment has been minimized.

Contributor

tadast commented on 1d89a87 Aug 20, 2015

Minor: commit message "Update govuk_frontend_toolkit to 4.2.1" would be clearer :)

This comment has been minimized.

Contributor

chrisroos replied Aug 24, 2015

Yup! I'll make the change and resubmit the pull request.

};
if (typeof window.ga === "function") {
window.ga('send', event);
var page = document.location.pathname;

This comment has been minimized.

@tadast

tadast Aug 20, 2015

Contributor

Not sure if it's worth worrying about, but this makes page and options variables available globally. They're rather generic names and if I'm correct it could in theory cause problems with other JS that may rely on if(options === undefined) or similar code. Putting it in a closure would avoid this problem e.g.:

(function(){
  var flowName = '<%= @name %>';
  var page = document.location.pathname;
  var options = {
    label: flowName,
    nonInteraction: true,
    page: page
  };
  GOVUK.analytics.trackEvent('Smart Answer', 'Completed', options);
})();

What do you think?

This comment has been minimized.

@chrisroos

chrisroos Aug 24, 2015

Contributor

I think you're right. I'll make the change and update this pull request.

@chrisroos

This comment has been minimized.

Contributor

chrisroos commented Aug 24, 2015

In working on this branch again this morning I've learnt that the Analytics JavaScript we're using actually comes from the version of govuk_frontend_toolkit in Static, and not the one in our app. Which means that I don't need to upgrade govuk_frontend_toolkit in our app.

I'm going to close this, update the branch and open a new pull request.

@chrisroos chrisroos closed this Aug 24, 2015

chrisroos added a commit that referenced this pull request Aug 25, 2015

Use GOVUK.analytics.trackEvent instead of ga function
This replaces PR #1899.

Version 4.1.0 of the govuk_frontend_toolkit Gem introduced the ability to
specify the page when calling the `trackEvent` function.

The `GOVUK.analytics.trackEvent` function comes from the govuk_frontend_toolkit
Gem in Static. Static has been using version 4.1.0 of that Gem since 22nd
July[1] which means that we're safe to use the updated `trackEvent` function
too.

The use of `ga()` was introduced in ae2a255.
See that commit for more information about why recording the `page` is useful.

I've wrapped the call to `GOVUK.analytics.trackEvent()` in an anonymous
function to avoid polluting the global namespace. I'm executing this function
using jQuery's document ready[2] although I'm not entirely sure this adds much
over simply executing the anonymous function directly.

It's worth mentioning that the JavaScript in this _result partial is moved to
just before the closing `body` tag in the page that's rendered. It's moved by
the `Slimmer::Processors::TagMover` in the slimmer Gem.

I used the Google Analytics Debugger Chrome extension[3] to compare the events
being sent before and after this change, and confirmed that they were unchanged:

== Before

Running command: ga("send", {hitType: "event", eventCategory: "Smart Answer",
eventAction: "Completed", eventLabel: "additional-commodity-code",
nonInteraction: 1, page: "/additional-commodity-code/y/75/5/85"})

== After

Running command: ga("send", {hitType: "event", eventCategory: "Smart Answer",
eventAction: "Completed", eventLabel: "additional-commodity-code",
nonInteraction: 1, page: "/additional-commodity-code/y/75/5/85"})

[1]: alphagov/static#623
[2]: https://learn.jquery.com/using-jquery-core/document-ready/
[3]: https://chrome.google.com/webstore/detail/google-analytics-debugger/jnkmfdileelhofjcijamephohjechhna?hl=en

@chrisroos chrisroos deleted the upgrade-govuk-frontend-toolkit branch Aug 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment