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

Update focus styles to meet WCAG 2.1 level AA non-text contrast requirements #360

Merged
merged 3 commits into from Sep 19, 2019

Conversation

NickColley
Copy link
Contributor

@NickColley NickColley commented Sep 11, 2019

This is a modest attempt at updating the autocomplete to address the WCAG 2.1 non-text related issues around the current focus by making it align with GOV.UK Frontend version 3.0.0.

Makes the component's focus styles contrast without relying on colour alone which means the new non-text contrast guideline in WCAG 2.1 level AA.

Also updates other colours from the GOV.UK palette to the new colours.

Screenshots

Before After

There are some more interesting ideas about the future of theming for this component here: #329 but this aims to be a small change so we can release it soon.

Fixes #359

@NickColley NickColley added this to the Next milestone Sep 11, 2019
@NickColley NickColley marked this pull request as ready for review September 11, 2019 16:48
@NickColley
Copy link
Contributor Author

Labelled this as a breaking change as I think the styling could catch people out who are importing this directly from npm

@NickColley NickColley changed the title Update focus styles Update focus styles to meet WCAG 2.1 level AA non-text contrast requirements Sep 11, 2019
@NickColley NickColley added this to Needs review in Design System Sprint Board via automation Sep 11, 2019
Copy link

@dashouse dashouse left a comment

Choose a reason for hiding this comment

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

Looks great, thanks 👍

@@ -2,6 +2,30 @@

## [Unreleased]

## Breaking changes
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll use this as the basis for discussion with Mark when we put together the final release notes.

Copy link
Member

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Makes sense – one minor comment, which it'd be good to have @dashouse's opinion on if possible…

.autocomplete__input--default{
padding: 4px;
.autocomplete__input--default {
padding: 5px;
Copy link
Member

Choose a reason for hiding this comment

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

Should the autocomplete have a height: 40px to match our text inputs? At the minute it's 39px which is frustratingly close.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried not to change that since this component relies on layering of inputs (placeholder, typeahead hint) and overlays for some features and it felt risky to change, so wanted to do a simple update and maybe do something better if we decide to do the stuff Dave was proposing in the other PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd do it :shipit:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This pull request isn't meant to be making the component 'proper GOV.UK Frontend' so would be nice to get this in and improve it later in the way Dave has suggested, will see what Ollie thinks 😬

CHANGELOG.md Outdated

#### If you have custom additional CSS

You should check that your custom CSS does not remove the improvements to the focus state, if you do need to change the appearence keep in mind the WCAG 2.1 level AA requirements.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You should check that your custom CSS does not remove the improvements to the focus state, if you do need to change the appearence keep in mind the WCAG 2.1 level AA requirements.
You should check that your custom CSS does not remove the improvements to the focus state, if you do need to change the appearance keep in mind the WCAG 2.1 level AA requirements.

Copy link
Member

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

🐝

This matches the guidance on focus styles in the GOV.UK Design System.

Makes the component's focus styles contrast without relying on colour alone which means the new non-text contrast guideline in WCAG 2.1 level AA

https://design-system.service.gov.uk/get-started/focus-states/
@NickColley NickColley merged commit 2e0f44f into master Sep 19, 2019
Design System Sprint Board automation moved this from Needs review to Done Sep 19, 2019
@NickColley NickColley deleted the update-focus-styles branch September 19, 2019 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Update focus styles to match GOV.UK Frontend 3.0.0
4 participants