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

Add track click code from static #1751

Merged
merged 3 commits into from
Oct 26, 2020
Merged

Add track click code from static #1751

merged 3 commits into from
Oct 26, 2020

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented Oct 24, 2020

What

Add the track click code from static into the gem. This has involved some changes:

  • the code is now a module which means that it has to be explicitly called using data-module on the element to be tracked, whereas previously this was done by the presence of other data attributes alone (data-track-category and data-track-action) EDIT: this was based on a misunderstanding of the code, it always needed to be initialised as a module
  • the module has been renamed because if we did a proper migration (and removed it from static) the above change would be a massive breaking change across all of GOV.UK, which we don't have time to deal with right now. Instead, this code will exist as a small semi-duplication, which is less than ideal, but allows us to use it

A side effect of this change was that until the module had been renamed it broke the tests because the details component explicitly uses the track click code from static for tracking, but because it can't access it the test created a dummy object which deleted the real one from the gem. For reference there are some notes below about how to fix this, which I did before realising renaming the track click code was a better solution.

Background

The track click code is a simple wrapper to allow easy custom analytics tracking on elements. It should be part of the analytics code but for historical reasons isn't (yet). To make use of it, add data-module="gem-track-click" to any element, along with some additional data attributes, and clicking on that element will fire a GA event. You can also put the module on a parent element, then put the tracking attributes on any number of child elements to track them. Note that the elements don't have to be links.

See examples in the tests for further clarity.

Why

We need to be able to do track click stuff in Accounts, which only has access to code from the gem, not static.

Visual Changes

None.

Details component

For reference, here's how to update the details component to use the track click code in the gem, not static.

in details.js...

lines 15 and 16:

var trackDetails = new window.GOVUK.Modules.TrackClick()
trackDetails.start(element)

change to:

var trackDetails = new window.GOVUK.Modules.GemTrackClick()
trackDetails.start(element)

details-spec.js

remove lines 11 to 16:

  var callback = jasmine.createSpy()
  GOVUK.Modules.TrackClick = function () {
    this.start = function () {
      callback()
    }
  }

update this test as follows:

  it('uses built in tracking module when provided with a track-label', function () {
    spyOn(GOVUK.Modules, 'GemTrackClick').and.callThrough()
    loadDetailsComponent()

    $('.govuk-details__summary').click()

    expect(GOVUK.Modules.GemTrackClick).toHaveBeenCalled()
    expect(GOVUK.analytics.trackEvent).toHaveBeenCalledWith('track-category', 'track-action', { transport: 'beacon', label: 'track-label' })
  })

Trello card: https://trello.com/c/2gMj38xR/330-build-analytics

- completely unmodified, will fail tests
- this will be migrated out of static in a separate PR
- remove jQuery, write as a module, lint
- some changes...
- because it's now a module, to initialise it requires the correct `data-module` attribute on the parent element, whereas before the code only needed the data track attributes to match the JS and execute
- changing this represents too much of a breaking change so for now the plan is to leave the original track click code in static and have this be a small duplication, with an eventual migration planned
- therefore this module has been renamed to not clash with the existing code in static, and reduce confusion
@bevanloon bevanloon temporarily deployed to govuk-publis-add-track--jwfu6j October 24, 2020 11:01 Inactive
@bevanloon bevanloon temporarily deployed to govuk-publis-add-track--jwfu6j October 24, 2020 11:01 Inactive
@bevanloon bevanloon temporarily deployed to govuk-publis-add-track--jwfu6j October 24, 2020 11:02 Inactive
@andysellick andysellick mentioned this pull request Oct 24, 2020
@andysellick andysellick requested a review from alex-ju October 24, 2020 11:04
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.

3 participants