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(theme): dynamically render the cookie consent scripts only on *.commercetools.com domain #348

Merged
merged 2 commits into from
Apr 8, 2020

Conversation

nkuehn
Copy link
Collaborator

@nkuehn nkuehn commented Apr 7, 2020

addresses #346

This is the technique that is used on the current docs.commercetools.com site - it's a client side approach that activates a CSS rule that removes the complete thing from the visible DOM, letting it do it's buggy work however it wants as long as it doesn't show up.

The check is on the .commercetools.com domain level because that's the cookie scope the script uses.
The root cause it that the cookie consent script fails on other domains and does not close.

@vercel
Copy link

vercel bot commented Apr 7, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/commercetools/commercetools-docs-kit/blda3wmqt
✅ Preview: https://commercetools-docs-kit-git-nk-brute-force-hide-cookie-annoyance.commercetools.now.sh

@nkuehn nkuehn requested a review from emmenko April 7, 2020 19:00
@nkuehn nkuehn changed the title Try to fix the cookie overlay in preview modes Fix the cookie overlay in preview modes Apr 7, 2020
Copy link
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

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

See comment, I would avoid using a class and instead just remove the container

packages/gatsby-theme-docs/gatsby-browser.js Outdated Show resolved Hide resolved
@emmenko emmenko force-pushed the nk-brute-force-hide-cookie-annoyance branch from 894e019 to 20d23b2 Compare April 8, 2020 11:21
@vercel vercel bot temporarily deployed to Preview April 8, 2020 11:21 Inactive
@emmenko emmenko force-pushed the nk-brute-force-hide-cookie-annoyance branch from 20d23b2 to 1e95c89 Compare April 8, 2020 11:22
@vercel vercel bot temporarily deployed to Preview April 8, 2020 11:22 Inactive
Comment on lines +47 to +56
if (window && window.location.host.includes('.commercetools.com')) {
injectScript(
'https://cdn.cookielaw.org/consent/b104027d-4d10-4b75-9675-9ffef11562a8/OtAutoBlock.js'
);
injectScript(
'https://cdn.cookielaw.org/scripttemplates/otSDKStub.js',
{ 'data-domain-script': 'b104027d-4d10-4b75-9675-9ffef11562a8' },
'function OptanonWrapper() {};'
);
}
Copy link
Member

Choose a reason for hiding this comment

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

This seems to do the trick.

2020-04-08 13 19 21

I'd like to try this approach first.

@emmenko emmenko added the 🐛 Type: Bug Something isn't working label Apr 8, 2020
@emmenko emmenko changed the title Fix the cookie overlay in preview modes fix(theme): dynamically render the cookie consent scripts only on *.commercetools.com domain Apr 8, 2020
@emmenko
Copy link
Member

emmenko commented Apr 8, 2020

@nkuehn please take a look, I would like to try this approach first.

@nkuehn
Copy link
Collaborator Author

nkuehn commented Apr 8, 2020

OK, just merge and approve yourself - we'll have to wait all through production anyways to test.

@emmenko
Copy link
Member

emmenko commented Apr 8, 2020

I'll test the canary release on appkit.

@emmenko emmenko force-pushed the nk-brute-force-hide-cookie-annoyance branch from 1e95c89 to ed2b9e9 Compare April 8, 2020 11:32
@vercel vercel bot temporarily deployed to Preview April 8, 2020 11:32 Inactive
@emmenko emmenko merged commit 40dd773 into master Apr 8, 2020
@emmenko emmenko deleted the nk-brute-force-hide-cookie-annoyance branch April 8, 2020 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants