-
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 cookie banner component and button groups #2131
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
Add a new button groups object which can be used to group buttons and links together. Within a button group: - links are styled to line up visually with the buttons, including being centre-aligned on mobile - spacing between the buttons and links is handled automatically, including when they wrap across multiple lines The button group object will then be used to group the 'Accept cookies', 'Reject cookies' and 'View and set cookie preferences' buttons and links in the cookie banner component. The button group object uses flexbox and align-items to align the buttons and links along the 'cross axis' – centre-aligning on mobile and aligning rows along the baseline on tablet and above. align-items is in theory supported in IE11 and above, but our use of autoprefixer means that IE10-compatible flexbox properties are also included, and in testing it works as well in IE10 as it does in IE11. In browsers that do not support flexbox [1] or align-items [2] the spacing between buttons and links is different (as whitespace between inline blocks is not handled) and they are not aligned along the cross-axis, but the general layout is still applied. [1]: https://caniuse.com/flexbox [2]: https://caniuse.com/mdn-css_properties_align-items_flex_context
Set max-width to fix links overflowing in IE10/11 in the mobile viewport. https://github.com/philipwalton/flexbugs#2-column-flex-items-set-to-align-itemscenter-overflow-their-container
Add button groups for use in cookie banner
The original intent was to use a modifier on links to align them with buttons, but we ended up finding another approach that used flexbox and didn’t require the modifier. However, the classes were never removed from the example page in the review app. Remove the redundant classes as they are not doing anything and will only cause confusion.
Remove redundant align-button modifiers in example
To make it clear that role region cannot be overridden or removed Co-authored-by: Oliver Byford <[email protected]>
Add Nunjucks macro template, tests and examples for cookie banner
The cookie banner components needs to be flexible enough to work for both server-side and client-side implementations. Add an example of a typical server-side cookie banner, where a page navigation occurs when the user chooses to accept or reject the cookies, and the choice to render either the original ‘accept/reject’ banner or the confirmation banner is made on the server. We do include a small amount of client-side JavaScript to handle the ‘Hide’ button in the example, as I think this would typically be handled client-side as nothing needs to be recorded.
Add full page examples of cookie banners
Allow the whole cookie banner component to be hidden
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2131
February 1, 2021 16:11
Inactive
We refer to 'question message' as 'cookie message' in the guidance. This commit tweaks the macro options to reflect that.
Rename banners to messages
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2131
February 2, 2021 13:23
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).
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2131
February 2, 2021 16:37
Inactive
- Remove duplicated ‘required’ on `messages` and `actions.text` – ‘Required’ is automatically added as a prefix to the description in the Design System when `required` is `true`, so it does not need to be included in the description as well. - Mark `messages.text`, `messages.html` as required and remove ‘Required’ from their descriptions - Mark `false` as inline code in desecription for `hidden` and `messages.hidden`
Co-authored-by: Eoin Shaughnessy <[email protected]>
Co-authored-by: EoinShaughnessy <[email protected]>
Tweaks to macro option documentation
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2131
February 5, 2021 15:40
Inactive
36degrees
changed the title
Add cookie banner
Add cookie banner component and button groups
Feb 8, 2021
36degrees
approved these changes
Feb 8, 2021
Merged
This was referenced Mar 16, 2021
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.
This is a combination of several PRs which have already been reviewed and merged into the
feature/cookie-banner
branch:The cookie banner work has been done using a 'feature branch' so that we can keep code reviews small whilst avoiding merging unreleasable work to the master branch. This PR merges that feature branch into master now that the cookie banner work is ready to release.
This means that all of the individual changes have already been through one round of code review. To review this pull request, really all we want to check is that the changes are what we expect based on the above.