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

Add cookie banner component #795

Merged
merged 3 commits into from
Mar 26, 2019
Merged

Add cookie banner component #795

merged 3 commits into from
Mar 26, 2019

Conversation

alex-ju
Copy link
Contributor

@alex-ju alex-ju commented Mar 21, 2019

Need

We need to update the cookie banner which is currently part of govuk_template. There was an attempt to do this update in govuk_template, but since that's a deprecated repository we shouldn't do any new work or releases on it.

Instead, this PR moves the remaining govuk-template.js scripts into app/assets/javascripts/govuk_publishing_components/lib/ and extracts the cookie banner into its own component.

Complementary work

A follow up PR will come after this, updating static to remove govuk-template.js and the associated markup for the cookie banner and replace it with this component.

Update: PR in static alphagov/static#1662

Trello card

@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 21, 2019 15:16 Inactive
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 21, 2019 15:19 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 766d284 to 1d23ff9 Compare March 21, 2019 16:20
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 21, 2019 16:20 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 1d23ff9 to 7fb1d99 Compare March 21, 2019 17:24
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 21, 2019 17:25 Inactive
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.

Just a few minor comments, also does this need more tests? I thinking of the JS in the first commit and a spec test for the component to check that clicking the link makes the banner disappear?

@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 7fb1d99 to 7a7205e Compare March 22, 2019 13:33
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 22, 2019 13:33 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 7a7205e to 0b8acfa Compare March 22, 2019 13:48
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 22, 2019 13:48 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 0b8acfa to 96c3338 Compare March 22, 2019 13:56
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 22, 2019 13:57 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 96c3338 to 1969474 Compare March 26, 2019 09:44
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 09:45 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 1969474 to 264d0fd Compare March 26, 2019 10:14
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 10:14 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 264d0fd to 9ab104c Compare March 26, 2019 10:41
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 10:41 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 9ab104c to 920420d Compare March 26, 2019 10:44
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 10:44 Inactive
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 10:57 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 5bfda86 to 9f5e768 Compare March 26, 2019 11:11
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 11:11 Inactive
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 11:17 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 9c5e838 to 2e44eec Compare March 26, 2019 11:47
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 11:48 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 2e44eec to 01758f5 Compare March 26, 2019 11:53
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 11:53 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 01758f5 to 1209d2f Compare March 26, 2019 12:07
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 12:07 Inactive
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 12:22 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from 9306059 to ce70f45 Compare March 26, 2019 12:31
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 12:31 Inactive
@alex-ju alex-ju force-pushed the add-cookie-banner-component branch from ce70f45 to 6c96da0 Compare March 26, 2019 12:36
@benthorner benthorner temporarily deployed to govuk-publishing-compon-pr-795 March 26, 2019 12:36 Inactive
@alex-ju
Copy link
Contributor Author

alex-ju commented Mar 26, 2019

Thanks for the review @andysellick! I addressed all the comments including tests for the cookie banner visibility and the cookie state.

@alex-ju alex-ju merged commit 525d473 into master Mar 26, 2019
@alex-ju alex-ju deleted the add-cookie-banner-component branch March 26, 2019 14:19
@alex-ju alex-ju mentioned this pull request Mar 26, 2019
@fofr
Copy link
Contributor

fofr commented Apr 2, 2019

Can I ask why a "hide this message" link has been added?

Edit: Looks like the cookie bar is now persistent until it’s dismissed, which explains the new link. For services who will want to be consistent, what's the reasoning behind this?

@alex-ju
Copy link
Contributor Author

alex-ju commented Apr 2, 2019

This update is part of a GOV.UK Cookie & Tracking Policy mission in an effort to align it with the spirit of the EU cookie and tracking policy legislation. To play back team's PMs reply to this question: "it’s a temporary change to increase visibility of the message while we consider other options in the next few months. that work will include looking at how this could be expanded across government."

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