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

Fix single page notification button data attributes for tracking #2471

Merged
merged 3 commits into from Nov 25, 2021

Conversation

danacotoran
Copy link
Contributor

@danacotoran danacotoran commented Nov 24, 2021

  1. The data attributes for tracking were not named correctly.
    The data-label attribute should be data-track-label, data-action should be data-track-action, data-category should be data-track-category.
  2. Apply data-module=gem-track-click attribute to a wrapper div instead of the form itself. The reason for this is that
    the form gets dynamically swapped out with the form returned from the personalisation endpoint. If data-module=gem-track-click was applied to the form, then in order for click tracking to fire correctly after the JS update has occurred, the track click module would have to be re-started on the updated form, which is not ideal.
    This is a workaround which can be credited to @andysellick

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 10:17 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 10:23 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 10:24 Inactive
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 10:26 Inactive
@danacotoran danacotoran marked this pull request as ready for review November 24, 2021 10:26
@danacotoran danacotoran marked this pull request as draft November 24, 2021 10:59
@danacotoran
Copy link
Contributor Author

Re-drafted this while I investigate another potential tracking issue which could go in the same PR

@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 13:44 Inactive
@danacotoran danacotoran changed the title Fix single page notification button data attributes for tracking Tracking fixes for the single page notification button Nov 24, 2021
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 13:53 Inactive
@danacotoran danacotoran marked this pull request as ready for review November 24, 2021 13:53
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Nothing wrong here but I'm wondering about the approach, although I could be wrong.

@danacotoran danacotoran changed the title Tracking fixes for the single page notification button Fix single page notification button data attributes for tracking Nov 24, 2021
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 17:15 Inactive
@danacotoran danacotoran marked this pull request as draft November 24, 2021 17:31
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 24, 2021 18:27 Inactive
@danacotoran danacotoran marked this pull request as ready for review November 24, 2021 18:33
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

All looks good. I think originally there was a test for the JavaScript behaviour, but that seems to have disappeared? Would be worth putting it back in.

The data attributes for tracking are not named correctly. data-label
should be data-track-label, data-action should be data-track-action,
data-category should be data-track-category.
Apply the data-module="gem-track-click" for the single page notification
button to a component wrapper div.
The reason behind applying this to a wrapper and not to the form is that
the form gets swapped out with the form returned from the
personalisation endpoint, so the track click module would have to be
re-started on the new, updated form.
Applying the track-click attribute to a wrapper instead means we don't
have to re-start the gem-track-click module for the updated form.
@govuk-ci govuk-ci temporarily deployed to components-gem-pr-2471 November 25, 2021 10:25 Inactive
@danacotoran danacotoran merged commit c6f49c3 into master Nov 25, 2021
@danacotoran danacotoran deleted the fix-tracking-attributes branch November 25, 2021 10:30
@danacotoran danacotoran mentioned this pull request Nov 25, 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.

None yet

3 participants