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

Allow tracking of the details component #1187

Merged
merged 2 commits into from Nov 6, 2019

Conversation

@vanitabarrett
Copy link
Contributor

vanitabarrett commented Nov 4, 2019

Trello:
https://trello.com/c/Dtle7GcF/197-modify-details-component-to-enable-tracking-analytics
to unblock alphagov/finder-frontend#1702

What

Allow tracking of the details component, e.g: on the Brexit checker.

Why

The details component, by default, has data-module="govuk-details which makes it difficult for us to add our usual data-module="track-click". We also want some more custom tracking of the open and close status of the component. This adds tracking JS to the component.

Passing all the necessary data attributes, including track-label simply passes the details element to our usual TrackClick method in static. This then works in exactly the same way as the rest of our tracking.

Passing in all the necessary data attributes except `track-label means that the track-label by default becomes the open/close status of the component. In this case, we manually add an event listener and fire a track event on click.

Screen Shot 2019-11-04 at 15 18 16

https://govuk-publishing-compo-pr-1187.herokuapp.com/

@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 831b0cb to 17aee34 Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from d4a2817 to c8571f8 Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 976766f to b7eea6d Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from b7eea6d to 4578fe3 Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 4578fe3 to 2a1fe94 Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 2a1fe94 to a9f1adf Nov 4, 2019
@bevanloon bevanloon had a problem deploying to govuk-publishing-compo-pr-1187 Nov 4, 2019 Failure
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from a9f1adf to a1b3832 Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from a1b3832 to c8571f8 Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from c8571f8 to 90cdb9a Nov 4, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 4, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 84ad325 to 914a368 Nov 5, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 5, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch 5 times, most recently from 7e59096 to 6ef2507 Nov 5, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 5, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 6ef2507 to 7b97ea0 Nov 5, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 6, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 7714ecf to a4b62ad Nov 6, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 6, 2019 Inactive
@vanitabarrett vanitabarrett marked this pull request as ready for review Nov 6, 2019
@vanitabarrett vanitabarrett requested review from andysellick, alex-ju and huwd Nov 6, 2019
@vanitabarrett

This comment has been minimized.

Copy link
Contributor Author

vanitabarrett commented Nov 6, 2019

Thank you helping fix the tests @alex-ju ! 🙇‍♀

@alex-ju
alex-ju approved these changes Nov 6, 2019
Copy link
Member

alex-ju left a comment

I've left a comment regarding a conditional statement.

I have mixed feelings about where the tracking scrips should live (within the component script - as we do for checkboxes, or separated - as we do for select). I would incline more towards the latter — maybe something to be discussed within the community, especially before moving other analytics-related scripts into this repo.

@vanitabarrett

This comment has been minimized.

Copy link
Contributor Author

vanitabarrett commented Nov 6, 2019

@alex-ju Agreed - I wasn't sure whether to make this component-specific or not. I opted for making it specific to the details component just because it seemed like quite a custom bit of tracking, but happy to move it when we come to a consensus on where these things should live.

@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from a4b62ad to f8ca215 Nov 6, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 6, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from f8ca215 to 9053153 Nov 6, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 6, 2019 Inactive
Copy link
Contributor

andysellick left a comment

Couple of tiny things, but otherwise looks good.

@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 6, 2019 Inactive
@vanitabarrett vanitabarrett force-pushed the add-tracking-details-component branch from 6fe7d30 to 9053153 Nov 6, 2019
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1187 Nov 6, 2019 Inactive
@vanitabarrett

This comment has been minimized.

Copy link
Contributor Author

vanitabarrett commented Nov 6, 2019

Waiting for confirmation on the tracking attributes before this can be merged

@vanitabarrett

This comment has been minimized.

Copy link
Contributor Author

vanitabarrett commented Nov 6, 2019

Update on the tracking from Paul. In summary, the logic should be ok as is.

We're already inconsistent in how we use eventLabel vs eventAction so it doesn't really matter. Choose which ever is easiest for you! What's important is that we record which question the event takes place on. We normally depend on the URL for this but this doesn't work within the checker. I think there are labels for questions like 'travelling' and 'employment' which are used on the checkbox event.

@vanitabarrett vanitabarrett merged commit e00dc77 into master Nov 6, 2019
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@vanitabarrett vanitabarrett deleted the add-tracking-details-component branch Nov 6, 2019
@vanitabarrett vanitabarrett referenced this pull request Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.