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.

Sign up for GitHub

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

Jump to bottom

[Spike] Option 1: Replace existing mixins with new focus styles #1354

Closed
wants to merge 6 commits into from
Closed

[Spike] Option 1: Replace existing mixins with new focus styles #1354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
dcab584
Replace focusable-fill with new focus styles
NickColley May 16, 2019
4f924cc
Update skip-link to avoid new mixin
NickColley May 16, 2019
40060d5
Rename focusable-text to focusable-fill-css
NickColley May 16, 2019
63ab9e9
Update govuk-focusable mixin to include box-shadow
NickColley May 16, 2019
67da8d9
Update header to use correct fill mixin
NickColley May 16, 2019
272722f
Ensure error summary does not have box-shadow focus
NickColley May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/accordion/_accordion.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@
// We also want to avoid styling when the secttion is pressed or `:active`
// to avoid the different focus styles from flashing quickly while the user interacts with the section.
.govuk-accordion__section:not(.govuk-accordion__section--expanded) .govuk-accordion__section-button:focus:not(:active) {
@include govuk-focusable-text;
@include govuk-focusable-fill-css;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original mixins assume you always want the state on a regular :focus but you can see here It is useful to be able to customise when to show the focus.

At the moment we'd have to have an additional mixin without the :focus selector.

You can also see this in the file upload component.

}

.govuk-accordion__controls {
Expand Down
2 changes: 1 addition & 1 deletion src/components/details/_details.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
color: $govuk-link-hover-colour;
}

@include govuk-focusable-text-link;
@include govuk-focusable-fill;
}

// ...but only underline the text, not the arrow
Expand Down
7 changes: 6 additions & 1 deletion src/components/error-summary/_error-summary.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
@include govuk-focusable;

border: $govuk-border-width solid $govuk-error-colour;

&:focus {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some components we have to override the new focusable state

// Remove `box-shadow` inherited from `:focus`.
box-shadow: none;
}
}

.govuk-error-summary__title {
Expand All @@ -38,7 +43,7 @@
}

.govuk-error-summary__list a {
@include govuk-focusable-text-link;
@include govuk-focusable-fill;
@include govuk-typography-weight-bold;

// Override default link styling to use error colour
Expand Down
9 changes: 0 additions & 9 deletions src/components/file-upload/_file-upload.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,6 @@
margin-left: -$component-padding;
padding-right: $component-padding;
padding-left: $component-padding;
// Use `box-shadow` to add border instead of changing `border-width`
// (which changes element size) and since `outline` is already used for the
// yellow focus state.
box-shadow: inset 0 0 0 4px $govuk-input-border-colour;

@include govuk-if-ie8 {
// IE8 doesn't support `box-shadow` so add an actual border
border: 4px solid $govuk-input-border-colour;
}
}

// Set "focus-within" to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1430196
Expand Down
2 changes: 1 addition & 1 deletion src/components/footer/_footer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
}
}

@include govuk-focusable-text-link;
@include govuk-focusable-fill;

// alphagov/govuk_template includes a specific a:link:focus selector
// designed to make unvisited links a slightly darker blue when focussed, so
Expand Down
4 changes: 2 additions & 2 deletions src/components/header/_header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
text-decoration: underline;
}

@include govuk-focusable-text-link;
@include govuk-focusable-fill;

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
Expand Down Expand Up @@ -176,7 +176,7 @@
margin-left: govuk-spacing(1);
}

@include govuk-focusable;
@include govuk-focusable-fill;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #1346


@include mq ($from: tablet) {
top: govuk-spacing(3);
Expand Down
14 changes: 0 additions & 14 deletions src/components/input/_input.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,6 @@

// Disable inner shadow and remove rounded corners
appearance: none;

&:focus {
// Double the border by adding its width again. Use `box-shadow` for this // instead of changing `border-width` - this is for consistency with
// components such as textarea where we avoid changing `border-width` as
// it will change the element size. Also, `outline` cannot be utilised
// here as it is already used for the yellow focus state.
box-shadow: inset 0 0 0 $govuk-border-width-form-element;

@include govuk-if-ie8 {
// IE8 doesn't support `box-shadow` so double the border with
// `border-width`.
border-width: $govuk-border-width-form-element * 2;
}
}
}

.govuk-input::-webkit-outer-spin-button,
Expand Down
13 changes: 0 additions & 13 deletions src/components/select/_select.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,6 @@
height: 40px;
padding: govuk-spacing(1); // was 5px 4px 4px - size of it should be adjusted to match other form elements
border: $govuk-border-width-form-element solid $govuk-input-border-colour;

&:focus {
// Double the border by adding its width again. Use `box-shadow` to do
// this instead of changing `border-width` (which changes element size) and
// since `outline` is already used for the yellow focus state.
box-shadow: inset 0 0 0 $govuk-border-width-form-element;

@include govuk-if-ie8 {
// IE8 doesn't support `box-shadow` so double the border with
// `border-width`.
border-width: $govuk-border-width-form-element * 2;
}
}
}

.govuk-select option:active,
Expand Down
7 changes: 6 additions & 1 deletion src/components/skip-link/_skip-link.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
.govuk-skip-link {
@include govuk-visually-hidden-focusable;
@include govuk-typography-common;
@include govuk-focusable-fill;
@include govuk-link-style-text;
@include govuk-typography-responsive($size: 16);

Expand All @@ -23,5 +22,11 @@
padding-right: unquote("max(#{govuk-spacing(3)}, #{$padding-safe-area-right})");
padding-left: unquote("max(#{govuk-spacing(3)}, #{$padding-safe-area-left})");
}

&:focus {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Skip link is different enough we remove the mixin entirely...

outline: $govuk-focus-width solid $govuk-focus-colour;
outline-offset: 0;
background-color: $govuk-focus-colour;
}
}
}
13 changes: 0 additions & 13 deletions src/components/textarea/_textarea.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,6 @@
border-radius: 0;

-webkit-appearance: none;

&:focus {
// Double the border by adding its width again. Use `box-shadow` to do
// this instead of changing `border-width` (which changes element size) and
// since `outline` is already used for the yellow focus state.
box-shadow: inset 0 0 0 $govuk-border-width-form-element;

@include govuk-if-ie8 {
// IE8 doesn't support `box-shadow` so double the border with
// `border-width`.
border-width: $govuk-border-width-form-element * 2;
}
}
}

.govuk-textarea--error {
Expand Down
27 changes: 10 additions & 17 deletions src/helpers/_focusable.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,18 @@
///
/// @access public

$govuk-focus-width-inner: 2px;

@mixin govuk-focusable {
&:focus {
outline: $govuk-focus-width solid $govuk-focus-colour;
outline-offset: 0;
box-shadow: inset 0 0 0 $govuk-focus-width-inner $govuk-text-colour;

@include govuk-if-ie8 {
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 feels too specific to the input components, and would likely break anything else.

Perhaps IE8 could use user-agent styles instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Radios and checkboxes set the width for their styles so it'd be a bit weird to not do it on inputs.

// IE8 doesn't support `box-shadow` so add an actual border
border: $govuk-focus-width-inner solid $govuk-text-colour;
}
}
}

Expand All @@ -27,13 +35,11 @@

@mixin govuk-focusable-fill {
&:focus {
outline: $govuk-focus-width solid $govuk-focus-colour;
outline-offset: 0;
background-color: $govuk-focus-colour;
@include govuk-focusable-fill-css;
}
}

@mixin govuk-focusable-text {
@mixin govuk-focusable-fill-css {
// When colours are overridden, for example when users have a dark mode,
// backgrounds and box-shadows disappear, so we need to ensure there's a
// transparent outline which will be set to a visible colour.
Expand All @@ -55,16 +61,3 @@
// box shadow adds the "underline"
text-decoration: none;
}

/// Focusable with box-shadow
///
/// Removes the visible outline and replace with box-shadow and background colour.
/// Used for interactive text-based elements.
///
/// @access public

@mixin govuk-focusable-text-link {
&:focus {
@include govuk-focusable-text;
}
}
2 changes: 1 addition & 1 deletion src/helpers/_links.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

@mixin govuk-link-common {
@include govuk-typography-common;
@include govuk-focusable-text-link;
@include govuk-focusable-fill;
}

/// Default link style mixin
Expand Down