-
Notifications
You must be signed in to change notification settings - Fork 5
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
Civic uk: Accept/reject behaviour #10685
Changes from all commits
2e6e80b
f2cfa46
51f70ed
aeb899d
68a1c4f
5faeefb
e74ba02
0e82296
9f3f2dc
58db27c
630bbe8
b226d95
d2b5e2e
a225363
ea714e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,13 +11,14 @@ type Props = { | |
data: { | ||
toggles?: Toggles; | ||
}; | ||
hasAnalyticsConsent: boolean; | ||
}; | ||
|
||
// We send toggles as an event parameter to GA4 so we can determine the condition in which a particular event took place. | ||
// GA4 now limits event parameter values to 100 characters: https://support.google.com/analytics/answer/9267744?hl=en | ||
// So instead of sending the whole toggles JSON blob, we only look at the "test" typed toggles and send a concatenated string made of the toggles' name | ||
// , preceeded with a! if its value is false. | ||
function createToggleString(toggles: Toggles | undefined): string | null { | ||
function createABToggleString(toggles: Toggles | undefined): string | null { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From what I gathered, this only returns AB test strings, so I renamed to clarify. |
||
const testToggles = toggles | ||
? Object.keys(toggles).reduce((acc, key) => { | ||
if (toggles?.[key].type === 'test') { | ||
|
@@ -42,18 +43,40 @@ function createToggleString(toggles: Toggles | undefined): string | null { | |
: null; | ||
} | ||
|
||
export const Ga4DataLayer: FunctionComponent<Props> = ({ data }) => { | ||
const toggleString = createToggleString(data.toggles); | ||
export const Ga4DataLayer: FunctionComponent<Props> = ({ | ||
data, | ||
hasAnalyticsConsent, | ||
}) => { | ||
const abTestsToggleString = createABToggleString(data.toggles); | ||
|
||
return toggleString ? ( | ||
return data.toggles?.cookiesWork?.value || abTestsToggleString ? ( | ||
<script | ||
dangerouslySetInnerHTML={{ | ||
__html: ` | ||
window.dataLayer = window.dataLayer || []; | ||
window.dataLayer.push({ | ||
toggles: '${toggleString}' | ||
}); | ||
`, | ||
window.dataLayer = window.dataLayer || []; | ||
|
||
${ | ||
data.toggles?.cookiesWork?.value | ||
? `function gtag(){window.dataLayer.push(arguments);} | ||
|
||
gtag('consent', 'default', { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that we need some more fields here to adhere to "consent mode v2": https://developers.google.com/tag-platform/security/guides/consent?consentmode=advanced#upgrade-consent-v2, specifically I expect we'll just hard-code the values to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yess I wasn't sure we used those because adding them caused me TS errors, looks like the gtag helpers don't know about them?
![]() That being said, as I'm looking at this, it's only a problem in the TS code when we If ever we want those to be changeable, we'll deal with TS errors then. Not a problem for now. I'll stop thinking out loud and apply changes 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
'analytics_storage': ${ | ||
hasAnalyticsConsent ? '"granted"' : '"denied"' | ||
}, | ||
'ad_storage': 'denied', | ||
'ad_user_data': 'denied', | ||
'ad_personalization': 'denied' | ||
});` | ||
: `` | ||
} | ||
|
||
${ | ||
abTestsToggleString && | ||
`window.dataLayer.push({ | ||
toggles: '${abTestsToggleString}' | ||
});` | ||
} | ||
`, | ||
}} | ||
/> | ||
) : null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,11 +70,11 @@ const CivicUK = () => ( | |
'__utmv', | ||
], | ||
onAccept: function () { | ||
const event = new CustomEvent('analyticsConsentChange', {}); | ||
const event = new CustomEvent('analyticsConsentChanged', { detail: { consent: 'granted' }}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👌 |
||
window.dispatchEvent(event); | ||
}, | ||
onRevoke: function () { | ||
const event = new CustomEvent('analyticsConsentChange', {}); | ||
const event = new CustomEvent('analyticsConsentChanged', { detail: { consent: 'denied' } }); | ||
window.dispatchEvent(event); | ||
}, | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import Head from 'next/head'; | ||
import { useEffect, useState } from 'react'; | ||
import CivicUK from './ConsentAndScripts.CivicUK'; | ||
import { getAnalyticsConsentState } from '@weco/common/services/app/civic-uk'; | ||
|
||
const ConsentAndScripts = ({ segmentSnippet }: { segmentSnippet: string }) => { | ||
const [consentState, setConsentState] = useState(false); | ||
|
||
const onAnalyticsConsentChanged = ( | ||
event: CustomEvent<{ consent: 'granted' | 'denied' }> | ||
) => { | ||
// Toggle rendering of scripts | ||
setConsentState(event.detail.consent === 'granted'); | ||
kenoir marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Update datalayer config with consent value | ||
gtag('consent', 'update', { | ||
analytics_storage: event.detail.consent, | ||
}); | ||
}; | ||
|
||
useEffect(() => { | ||
setConsentState(getAnalyticsConsentState()); | ||
|
||
window.addEventListener( | ||
'analyticsConsentChanged', | ||
onAnalyticsConsentChanged | ||
); | ||
|
||
return () => { | ||
window.removeEventListener( | ||
'analyticsConsentChanged', | ||
onAnalyticsConsentChanged | ||
); | ||
}; | ||
}, []); | ||
|
||
return ( | ||
<> | ||
{consentState && ( | ||
<Head> | ||
<script dangerouslySetInnerHTML={{ __html: segmentSnippet }} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally also put the GTM script in this conditional, but I was experiencing what I think is this Next bug where on loading it/removing it/re-adding it, random meta tags would duplicate themselves, even if they had a I think it's ok though as we set the consent to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I believe the tag still "phones home" via what the docs describe as a "cookie-less ping". There is the possibly contentious case for adding the GTM script before consent that we still record some analytics data about these users in a legally compliant manner (so long as we set the consent state in the data layer correctly). See https://support.google.com/tagmanager/answer/13802165?sjid=2210990408479536825-EU for more details about what gets communicated to Google in the situation consent is denied. See "Behavioral modeling for consent mode" for one of the "features" available to us. Notably modelled data for un-consented visitors is not sent via any BigQuery integration. We should be sure we're comfortable with this, and check with @LaurenFBaily. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had a look at this and the documentation, I'm happy with that. |
||
</Head> | ||
)} | ||
|
||
<CivicUK /> | ||
</> | ||
); | ||
}; | ||
|
||
export default ConsentAndScripts; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -162,14 +162,22 @@ const PageLayoutComponent: FunctionComponent<Props> = ({ | |
<> | ||
<Head> | ||
<title>{fullTitle}</title> | ||
<meta name="description" content={description} /> | ||
<meta key="metadescription" name="description" content={description} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be useful to add a comment linking to the next issue describing why we're adding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think not here, as the Next issue is saying that this should work but doesn't. It's a recommendation from their official documentation for If we wanted to keep track of that issue I could add it to where we load the GTM script and explain that's why we're not removing it? |
||
<link rel="canonical" href={absoluteUrl} /> | ||
{/* meta elements need to be contained as direct children of the Head element, so don't componentise the following */} | ||
<meta property="og:site_name" content="Wellcome Collection" /> | ||
<meta property="og:type" content={openGraphType} /> | ||
<meta property="og:title" content={title} /> | ||
<meta property="og:description" content={description} /> | ||
<meta property="og:url" content={absoluteUrl} /> | ||
<meta | ||
key="og:sitename" | ||
property="og:site_name" | ||
content="Wellcome Collection" | ||
/> | ||
<meta key="og:type" property="og:type" content={openGraphType} /> | ||
<meta key="og:title" property="og:title" content={title} /> | ||
<meta | ||
key="og:description" | ||
property="og:description" | ||
content={description} | ||
/> | ||
<meta key="og:url" property="og:url" content={absoluteUrl} /> | ||
<meta | ||
key="og:image" | ||
property="og:image" | ||
|
@@ -207,9 +215,21 @@ const PageLayoutComponent: FunctionComponent<Props> = ({ | |
content={description} | ||
/> | ||
<meta key="twitter:image" name="twitter:image" content={imageUrl} /> | ||
<meta name="twitter:image:alt" content={imageAltText} /> | ||
<meta httpEquiv="X-UA-Compatible" content="IE=edge,chrome=1" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
<meta | ||
key="twitter:image:alt" | ||
name="twitter:image:alt" | ||
content={imageAltText} | ||
/> | ||
<meta | ||
key="httpEquiv" | ||
httpEquiv="X-UA-Compatible" | ||
content="IE=edge,chrome=1" | ||
/> | ||
<meta | ||
key="viewport" | ||
name="viewport" | ||
content="width=device-width, initial-scale=1" | ||
/> | ||
<link | ||
rel="apple-touch-icon" | ||
sizes="180x180" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future we'll need the default no consent state to be false, worth adding a comment to be double sure we do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we remove the toggle and therefore the first condition, and only keep the second condition, with a change to the comments, that's what it'll do 👍
We'll for sure test that a lot, it's why we'll requisition e2e for a bit, with all the toggle stuff out of the way 🤞