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

Show focus outlines when customising colours settings on Radios and Checkboxes #854

Merged
merged 3 commits into from
Jul 9, 2018

Conversation

NickColley
Copy link
Contributor

@NickColley NickColley commented Jul 2, 2018

Adds invisible outlines that will be shown when colours are overridden.

Checkboxes

Before overriden colours

before-checkboxes

After overridden colours

after-checkboxes

Radios

Before overridden colours

before-radios

After overridden colours

after-radios

Cross browser testing

Internet Explorer 11

screen shot 2018-07-09 at 10 58 20 screen shot 2018-07-09 at 10 59 07

Internet Explorer 10

screen shot 2018-07-09 at 11 00 57 screen shot 2018-07-09 at 11 01 33

Internet Explorer 9

screen shot 2018-07-09 at 11 02 54 screen shot 2018-07-09 at 11 02 24

Internet Explorer 8

screen shot 2018-07-09 at 11 05 34 screen shot 2018-07-09 at 11 04 38

Firefox latest, Chrome latest, Chrome Android latest, Safari latest, Safari iOS latest

No change

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-854 July 2, 2018 11:40 Inactive
@@ -10,6 +10,7 @@
@include govuk-exports("govuk/component/radios") {
$govuk-radios-size: govuk-spacing(7);
$govuk-radios-label-padding-left-right: govuk-spacing(3);
$govuk-radios-focus-width: 4px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming we need to add 1 pixel can this be $govuk-focus-width + 1 so that we understand where this value came from?

@@ -99,7 +100,12 @@

// Focused state
.govuk-radios__input:focus + .govuk-radios__label::before {
box-shadow: 0 0 0 4px $govuk-focus-colour;
top: -($govuk-radios-focus-width);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes sense, but could definitely do with a comment to explain what's going on here.

@NickColley NickColley added this to the v1.1.0 milestone Jul 4, 2018
@NickColley NickColley force-pushed the visible-focus-when-overriding-colours branch from 5d87149 to a93e93e Compare July 4, 2018 11:23
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-854 July 4, 2018 11:23 Inactive
@NickColley NickColley force-pushed the visible-focus-when-overriding-colours branch from a93e93e to cea238c Compare July 4, 2018 15:41
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-854 July 4, 2018 15:41 Inactive
@NickColley NickColley changed the title Visible focus when overriding colours [Do not merge] Visible focus when overriding colours Jul 4, 2018
@NickColley NickColley force-pushed the visible-focus-when-overriding-colours branch from cea238c to bb05940 Compare July 4, 2018 15:49
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-854 July 4, 2018 15:49 Inactive
@NickColley NickColley changed the title [Do not merge] Visible focus when overriding colours [Do not merge] Show focus outlines when customising colours settings on Radios and Checkboxes Jul 4, 2018
@NickColley NickColley removed this from the v1.1.0 milestone Jul 4, 2018
@36degrees
Copy link
Contributor

This is ace 👏 – @dashouse are you happy with the focus behaviour from a design perspective?

@36degrees 36degrees requested a review from dashouse July 5, 2018 15:09
@dashouse
Copy link

dashouse commented Jul 5, 2018

nice!

@NickColley NickColley force-pushed the visible-focus-when-overriding-colours branch from bb05940 to 1160ccb Compare July 9, 2018 10:15
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-854 July 9, 2018 10:15 Inactive
@NickColley NickColley force-pushed the visible-focus-when-overriding-colours branch from 1160ccb to d551da0 Compare July 9, 2018 10:28
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-854 July 9, 2018 10:29 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-854 July 9, 2018 10:31 Inactive
@NickColley NickColley force-pushed the visible-focus-when-overriding-colours branch from 4456f51 to 3e0d43c Compare July 9, 2018 10:32
@NickColley NickColley changed the title [Do not merge] Show focus outlines when customising colours settings on Radios and Checkboxes Show focus outlines when customising colours settings on Radios and Checkboxes Jul 9, 2018
@NickColley NickColley merged commit 4cbdaed into master Jul 9, 2018
@NickColley NickColley deleted the visible-focus-when-overriding-colours branch July 9, 2018 12:07
@36degrees 36degrees added this to the v1.1.0 milestone Jul 11, 2018
@NickColley NickColley mentioned this pull request Jul 13, 2018
// We set a transparent outline that is shown instead.
// https://accessibility.blog.gov.uk/2017/03/27/how-users-change-colours-on-websites/
outline: $govuk-focus-width solid transparent;
outline-offset: $govuk-focus-width;
Copy link

@timja timja Jul 23, 2018

Choose a reason for hiding this comment

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

I assume you're aware but outline-offset doesn't exist in IE (just came across that recently)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the offset is for mainly aesthetic reasons as this approach does not rely on the gap between the focus and control, having the offset just makes it a bit more clearer that this is a focus and not a thicker border.

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.

5 participants