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 links (and things that look like links) to use the new focus style #1309

Merged
merged 9 commits into from
May 8, 2019
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

💥 Breaking changes:

- Update links (and things that look like links) to use the new focus style

To migrate: [TODO add migration instructions before we ship v3.0.0]

([PR #1309](https://github.com/alphagov/govuk-frontend/pull/1309))

- Rename `$govuk-border-width-mobile` to `$govuk-border-width-narrow`

To migrate: If you are using `$govuk-border-width-mobile` in your own custom code, you need to rename any instances to `$govuk-border-width-narrow`.
Expand Down
6 changes: 6 additions & 0 deletions src/components/back-link/_back-link.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
// Underline is provided by a bottom border
text-decoration: none;

// When the back link is focused, hide the bottom link border as the
// focus styles has a bottom border.
&:focus {
border-bottom-color: transparent;
}

// Prepend left pointing arrow
&:before {
@include govuk-shape-arrow($direction: left, $base: 10px, $height: 6px);
Expand Down
23 changes: 10 additions & 13 deletions src/components/details/_details.scss
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,22 @@
// Style the summary to look like a link...
color: $govuk-link-colour;
cursor: pointer;

&:hover {
color: $govuk-link-hover-colour;
}

@include govuk-focusable-text-link;
}

// ...but only underline the text, not the arrow
.govuk-details__summary-text {
text-decoration: underline;
}

.govuk-details__summary:hover {
color: $govuk-link-hover-colour;
}

.govuk-details__summary:focus {
// -1px offset fixes gap between background and outline in Firefox
outline: ($govuk-focus-width + 1px) solid $govuk-focus-colour;
outline-offset: -1px;
// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
color: $govuk-focus-text-colour;
background: $govuk-focus-colour;
// Remove the underline when focussed to avoid duplicate borders
.govuk-details__summary:focus .govuk-details__summary-text {
text-decoration: none;
}

// Remove the default details marker so we can style our own consistently and
Expand All @@ -59,7 +56,7 @@
content: "";
position: absolute;

top: 0;
top: -1px;
bottom: 0;
left: 0;

Expand Down
11 changes: 1 addition & 10 deletions src/components/error-summary/_error-summary.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
}

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

// Override default link styling to use error colour
Expand All @@ -54,15 +54,6 @@
&:focus {
color: $govuk-focus-text-colour;
}

// alphagov/govuk_template includes a specific a:link:focus selector
// designed to make unvisited links a slightly darker blue when focussed, so
// we need to override the text colour for that combination of selectors.
@include govuk-compatibility(govuk_template) {
&:link:focus {
color: $govuk-error-colour;
}
}
}

}
9 changes: 7 additions & 2 deletions src/components/header/_header.scss
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@
}

.govuk-header__link {
@include govuk-focusable-fill;

text-decoration: none;

&:link,
Expand All @@ -78,6 +76,8 @@
text-decoration: underline;
}

@include govuk-focusable-text-link;

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
Expand Down Expand Up @@ -116,6 +116,11 @@
// specified currentColor explicitly IE8 would ignore this rule.
border-bottom: 1px solid;
}

// Remove any borders that show when focused and hovered.
&:focus {
border-bottom: 0;
}
}

.govuk-header__link--service-name {
Expand Down
3 changes: 2 additions & 1 deletion src/components/skip-link/_skip-link.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
@include govuk-exports("govuk/component/skip-link") {
.govuk-skip-link {
@include govuk-visually-hidden-focusable;
@include govuk-link-common;
@include govuk-typography-common;
@include govuk-focusable-fill;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this using govuk-focusable-fill and why is the skip link styled differently to other links?

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 is a design decision from @dashouse to not use the heavy underline focus state for skip-link.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 ok, currently this component and Footer are the only two components still using govuk-focusable-fill mixin. When we come to review the public mixing perhaps we can just hard code the mixins output here instead of having it hanging around

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 once we've got everything production ready we can (after the audit potentially) decide on how the mixins should work, with the new guidance in mind...

@include govuk-link-style-text;
@include govuk-typography-responsive($size: 16);

Expand Down
32 changes: 32 additions & 0 deletions src/helpers/_focusable.scss
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,35 @@
background-color: $govuk-focus-colour;
}
}

/// 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 {
Copy link
Member

Choose a reason for hiding this comment

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

This mixin seems to be performing a task very similar to @mixin govuk-focusable-fill. Do we need both? It would be good to have a comment to indicate what the difference between the two is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed on Slack we're going to wait until we have everything merged before deciding how the mixins should look.

Copy link
Member

Choose a reason for hiding this comment

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

👍

&:focus {
// 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.

// Since Internet Explorer 8 does not support box-shadow, we want to force the user-agent outlines
@include govuk-not-ie8 {
outline: $govuk-focus-width solid transparent;
outline-offset: 0;
}
color: $govuk-text-colour;
background-color: $govuk-focus-colour;
// sass-lint:disable indentation
box-shadow: -5px -1px 0 1px $govuk-focus-colour,
5px -1px 0 1px $govuk-focus-colour,
-3px 1px 0 3px $govuk-text-colour,
3px 1px 0 3px $govuk-text-colour;
// sass-lint:enable indentation
// When link is focussed, hide the default underline since the
// box shadow adds the "underline"
text-decoration: none;
}
}
2 changes: 1 addition & 1 deletion src/helpers/_links.scss
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-fill;
@include govuk-focusable-text-link;

This comment was marked as resolved.

}

/// Default link style mixin
Expand Down