-
Notifications
You must be signed in to change notification settings - Fork 320
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 styling for the cookie banner component #2126
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 28, 2021 09:13
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 28, 2021 09:20
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 28, 2021 11:02
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 28, 2021 12:07
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 28, 2021 15:31
Inactive
hannalaakso
changed the title
WIP cookie banner styles
Add styling for the cookie banner component
Jan 28, 2021
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
January 28, 2021 17:14
b855dbe
to
76c2fef
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 28, 2021 17:14
Inactive
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
January 28, 2021 17:34
76c2fef
to
d94b17f
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 28, 2021 17:34
Inactive
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.
3 tasks
2 tasks
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 29, 2021 12:03
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 29, 2021 15:24
Inactive
vanitabarrett
force-pushed
the
cookie-banner-styling
branch
from
January 29, 2021 15:34
9511f1c
to
478fd7c
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 29, 2021 15:34
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 29, 2021 16:11
Inactive
vanitabarrett
force-pushed
the
cookie-banner-styling
branch
from
January 29, 2021 16:17
c5415bf
to
16a914f
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
January 29, 2021 16:17
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 1, 2021 09:52
Inactive
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
February 1, 2021 15:55
5b05681
to
d94b17f
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 1, 2021 15:55
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 1, 2021 17:03
Inactive
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
February 1, 2021 17:09
27a17d5
to
840b7e3
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 1, 2021 17:09
Inactive
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
February 1, 2021 18:14
840b7e3
to
0c6d5f7
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 1, 2021 18:14
Inactive
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
February 1, 2021 19:12
0c6d5f7
to
879997f
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 1, 2021 19:12
Inactive
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
February 2, 2021 14:49
879997f
to
d3aae5b
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 2, 2021 14:49
Inactive
The component does not set bottom spacing. The bottom spacing should be created by the items inside the component. We're going to use the `hidden` attribute to hide sections as this will hide the content even if CSS fails to load. `display: none;` will act as a fallback for legacy browsers (IE 8-10) that don't support the `hidden` attribute. Unfortunately it looks like Lynx doesn't support `hidden` but that's a vendor implementation issue. It also appears that Lynx hasn't been in active development for some time. Using the `hidden` attribute seems like the semantically correct thing to do here and it makes the component usable when CSS fails to load. (The alternative would be to insert text into the DOM at relevant points which could also be a suitable solution in some contexts. We decided against it as the Design System components need to be really flexible so we're trying to avoid patterns where HTML markup needs to be maintained inside JavaScript which will also simplify localisation etc. where relevant.) Visually separate the cookie banner from content underneath with a transparent bottom border when user changes colours in their browser.
when the element is programmatically focused. The focused cookie banner is the first element on the page and the last thing the user interacted with prior to it gaining focus. We therefore assume that moving focus to it is not going to surprise users, and that giving it a visible focus indicator could be more confusing than helpful, especially as the element is not normally keyboard operable. We have flagged this in the research section of the guidance as something to monitor. A related discussion: w3c/wcag#1001 It's also worth noting that this is different to what we do with the notification banner which does have a visible focus indicator. However, the notification banner is further down the page and the user might not have interacted with it prior to it gaining focus so it is helpful to indicate to users that the focus has moved to it (even though the element, like the cookie banner, isn’t normally keyboard operable).
hannalaakso
force-pushed
the
cookie-banner-styling
branch
from
February 2, 2021 16:05
d3aae5b
to
f77c2f6
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2126
February 2, 2021 16:05
Inactive
vanitabarrett
approved these changes
Feb 2, 2021
Great work, looks awesome! 🎉 |
36degrees
approved these changes
Feb 2, 2021
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.
🎉
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Following on from #2114 and #2114, add the styling for the cookie banner component.
The designs are shown here: #2107 (comment) Additionally, the heading and content should be constrained to 2/3 grid column, whilst the button group should be full width on desktop.
Spacing
The component does not set bottom spacing. The bottom spacing should be created by the items inside the component.
Hiding sections
We're going to use the
hidden
attribute to hide sections as this will hide the content even if CSS fails to load.display: none;
will act as a fallback for legacy browsers (IE 8-10) that don't support thehidden
attribute. Unfortunately it looks like Lynx doesn't supporthidden
but that's a vendor implementation issue. It also appears that Lynx hasn't been in active development for some time. Using thehidden
attribute seems like the semantically correct thing to do here and it makes the component usable when CSS fails to load.(The alternative would be to insert text into the DOM at relevant points which could also be a suitable solution in some contexts. We decided against it as the Design System components need to be really flexible so we're trying to avoid patterns where HTML markup needs to be maintained inside JavaScript which will also simplify localisation etc. where relevant.)
Border
Visually separate the cookie banner from content underneath with a transparent bottom border when user changes colours in their browser.
Remove visible focus indicator from cookie banner
when the element is programmatically focused.
The focused cookie banner is the first element on the page and the last thing the user interacted with prior to it gaining focus. We therefore assume that moving focus to it is not going to surprise users, and that giving it a visible focus indicator could be more confusing than helpful, especially as the element is not normally keyboard operable.
We will flag this in the research section of the guidance as something to monitor.
A related discussion: w3c/wcag#1001
(It's also worth noting that this is different to what we do with the notification banner which does have a visible focus indicator. However, the notification banner is further down the page and the user might not have interacted with it prior to it gaining focus. It is therefore helpful to indicate to users that the focus has moved to it, even though the element, like the cookie banner, isn’t normally keyboard operable.)
Fixes #2098 and #2112
Tested on
✅ Chrome 88 (Mac)
✅ Firefox 84 (Mac)
✅ Safari 14 (Mac)
✅ Chrome 85 (Windows)
✅ Firefox 85 (Windows)
✅ Edge 88 (Windows)
✅ Chrome (Android 10)
✅ Firefox (Android 9)
✅ Samsung Internet (Android 10)
✅ Safari (iPhone 12 , iOS 14)
✅ Chrome (iPhone 8, iOS 13)
✅ IE11 (Windows 10)
✅ IE10 (Windows 7)
✅ IE9 (Windows 7)
✅ IE8 (Windows 7)
When users change colours in their browser (Firefox 84)