Fix video tracking on AB test

[andysellick]

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

What

Fixes video tracking on https://www.gov.uk/self-assessment-ready-reckoner

Note that this appears on the B variant of the A/B test, use the GOVUK browser extension to switch between them or use a mod header extension.

Frontend components should be implemented in templates or helpers, using the code examples found in https://components.publishing.service.gov.uk/component-guide. Injecting pre-rendered component markup into a page like this causes the following problems:

• component is not kept up to date with any changes upstream
• component JavaScript may not work properly (in this case the data-image-card-module-started attribute was included in the markup, which is only added after the JS has finished initialising on the component, so JS was not running on the component)
• styles for the component are not loaded, this is handled by the component template (see https://github.com/alphagov/govuk_publishing_components/blob/main/docs/set-up-individual-component-css-loading.md for more information)

For video tracking and JavaScript in general to work properly on videos they should be wrapped in a govspeak component and rendered in the page template. Pre-rendered markup for components should not be passed through from the locale files.

Why

Video tracking for GA4 was not working on this video.

Visual changes

None.

Trello card: https://trello.com/c/rHlNjPxe/775-video-tracking-not-working-on-a-b-test-pages

[hannako]

I was just wondering if we should put the href and link title in the locale file as well?

[andysellick]

Good shout, will look into.

[davidtrussler]

Hi @andysellick - this seems to be a very sensible change. My only concern is to wonder why we didn't do it like that originally so I'd be happier if one of the other requested reviewers could approve it as well in case there's a reason for that which I have missed. 🤔
I would certainly not have expected to implement this in this way more permanently if it was the outcome of the A/B Test.

[andysellick]

Hi @andysellick - this seems to be a very sensible change. My only concern is to wonder why we didn't do it like that originally so I'd be happier if one of the other requested reviewers could approve it as well in case there's a reason for that which I have missed. 🤔 I would certainly not have expected to implement this in this way more permanently if it was the outcome of the A/B Test.

Thanks @davidtrussler. I don't know why this was done like this originally, I think on the surface it seems like a reasonable way to do it if you're not familiar with the way our frontend components work. I think it's been a while since we (the frontend team) did any kind of talk about how frontend stuff works to the wider dev community and we're probably overdue.

I'm glad you mentioned this because I've also been asked with fixing a similar problem with this change but that one's a bit more complicated to unravel. Can you take a look?

[davidtrussler]

I'm glad you mentioned this because I've also been asked with fixing a similar problem with this change but that one's a bit more complicated to unravel. Can you take a look?

@andysellick Ah right, so I think on that one there is no other way to do this since the alternate (A/B) content is provided by the translation files by design. I guess that in turn explains why this one was done in a similar way since it followed in the wake of the one you're referring to here (also under some duress over the Xmas period!) We queried this approach with @richardTowers at the time (since embedded videos was a more complex scenario than a simple text change) and it was agreed there was no other way than to approach it like this.

[andysellick]

@andysellick Ah right, so I think on that one there is no other way to do this since the alternate (A/B) content is provided by the translation files by design. I guess that in turn explains why this one was done in a similar way since it followed in the wake of the one you're referring to here (also under some duress over the Xmas period!) We queried this approach with @richardTowers at the time (since embedded videos was a more complex scenario than a simple text change) and it was agreed there was no other way than to approach it like this.

Thanks, that's good to know. If it's just a temporary AB test then I'll take a look at seeing if changing the markup in the locale files could make the video tracking work 👍

[richardTowers]

Ah yeah, it was me that asked for it to be done like this originally - sorry about that.

The reasoning was exactly as you suggest - it was a temporary solution, and the ugliness was acceptable in the short term. The longer term plan is to find an elegant way to have this kind of test created in a self-service way by content designers.

I didn't predict the issues with tracking or stylesheet loading though.

To be honest, I'm not sure why I didn't consider using the govspeak component... that seems like a better approach all around. Perhaps I'd suggest putting the whole govspeak block into the locale file though? Rather than splitting it into description / video bits?