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

Start save location on cookie consent #2759

Merged
merged 1 commit into from
May 25, 2021

Conversation

andysellick
Copy link
Contributor

@andysellick andysellick commented May 10, 2021

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

DON'T MERGE - relies upon a new version of the gem with alphagov/govuk_publishing_components#2041 first
Done and rebased

What / why

Updates the save location bank holidays experiment code so that if users consent to cookies on that page, the functionality immediately appears. Before, it wouldn't appear unless users refreshed the page, because it depended on cookie consent to work.

Visual changes

None, but you can test that it works properly by loading up the /bank-holidays page, clearing cookies and reloading. The cookie banner should be present, the save location option not. Consenting to cookies should cause the save location option to immediately appear.

Trello card: https://trello.com/c/QweAgiPc/693-bank-holidays-experiment-things-to-clean-up

@bevanloon bevanloon temporarily deployed to govuk-fronte-save-locat-uqlnyk May 10, 2021 08:42 Inactive
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Code looks great. I haven't tested it though – let me know if you'd like me to do a more thorough check or you're confident with it working well.

@andysellick andysellick force-pushed the save-location-cookie-consent branch from 3604f29 to 070845f Compare May 11, 2021 08:40
@bevanloon bevanloon temporarily deployed to govuk-fronte-save-locat-uqlnyk May 11, 2021 08:40 Inactive
@danacotoran
Copy link
Contributor

This is probably more related to the supporting gem PR alphagov/govuk_publishing_components#2041 but since that PR's been closed I will leave it here.
I was cross-browser testing this and noticed a console error seemingly related to the trigger-event script on IE11. It can be recreated by consenting to cookies on the bank holidays page: the locations panel does not appear immediately upon accepting cookies.
Screenshot 2021-05-11 at 15 21 50

@danacotoran
Copy link
Contributor

Have opened alphagov/govuk_publishing_components#2079 to try and fix the error I outlined in my earlier comment.

- save location on bank holidays page checks for cookie consent first
- if it finds it, execution continues normally
- if it doesn't, it sets up an event listener to continue normally when cookies are consented
- this means that if a user consents to cookies on the bank holidays page, the save location functionality now appears immediately, instead of requiring a page refresh
@andysellick andysellick force-pushed the save-location-cookie-consent branch from 070845f to 686974c Compare May 20, 2021 14:39
Copy link
Contributor

@danacotoran danacotoran left a comment

Choose a reason for hiding this comment

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

Works as intended 👍

@andysellick andysellick changed the title [DO NOT MERGE] Start save location on cookie consent Start save location on cookie consent May 25, 2021
@andysellick andysellick merged commit 5946714 into master May 25, 2021
@andysellick andysellick deleted the save-location-cookie-consent branch May 25, 2021 08:59
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.

4 participants