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

Update data-module on layout header and footer #1913

Merged
merged 2 commits into from
Feb 9, 2021
Merged

Conversation

danacotoran
Copy link
Contributor

@danacotoran danacotoran commented Feb 9, 2021

What

Change value of data-module attribute to "gem-track-click" on the layout header and layout footer components.

Why

The reason behind this change is that click tracking is not working on these elements on GOVUK Accounts.
That is because Accounts does not use static and therefore also does not use the track-click script which enables click tracking on modules with the data-module="track-click" attribute.

Changing the value of this attribute to "gem-track-click" means that the header and footer will work with the component gem's track-click script instead.

https://trello.com/c/9A6UPqi8

@bevanloon bevanloon temporarily deployed to govuk-publis-copy-track-t0heqf February 9, 2021 11:18 Inactive
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Change value of data-module attribute to "gem-track-click" on the layout
header and layout footer components.
The reason behind this change is that click tracking is not working on
these elements on GOVUK Accounts. This is because Accounts does not use
static and therefore also does not use the track-click script which
enables click tracking on modules with the data-module="track-click"
attribute.

Changing the value of this attribute to "gem-track-click" means that the
header and footer will work with the gem's track-click script instead.
@danacotoran
Copy link
Contributor Author

Yeah you're right @alex-ju.
Andy suggested that we should copy the old track-click module from static as he thought that the gem's version would not work as it's a re-written version of the old one.
However, I tested this locally by changing the data-module attribute to "gem-track-click", and that does seem to do the trick.

@danacotoran danacotoran changed the title Add copy of track-click script from static Update data-module on layout header and footer Feb 9, 2021
@bevanloon bevanloon temporarily deployed to govuk-publis-copy-track-t0heqf February 9, 2021 18:20 Inactive
@danacotoran danacotoran requested a review from alex-ju February 9, 2021 18:23
@danacotoran danacotoran marked this pull request as ready for review February 9, 2021 18:23
@alex-ju
Copy link
Contributor

alex-ju commented Feb 9, 2021

Thanks for looking into this, @danacotoran! That's great news! This is how we actually ended up with the current version in lib (by copying it from static), so I'd really like to avoid doing this again.

Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Don't forget to add a changelog entry.

We'll have to do this for every component, unfortunately, before being able to remove the copy in static.

@bevanloon bevanloon temporarily deployed to govuk-publis-copy-track-t0heqf February 9, 2021 19:56 Inactive
@danacotoran danacotoran merged commit 223ce79 into master Feb 9, 2021
@danacotoran danacotoran deleted the copy-trackclick branch February 9, 2021 19:59
@danacotoran danacotoran mentioned this pull request Feb 10, 2021
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