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

Use text colour on focus for better contrast #982

Merged
merged 6 commits into from
Sep 10, 2018
Merged

Use text colour on focus for better contrast #982

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
3c78f95
Add darker focus color for details component
NickColley Sep 6, 2018
0125d65
Add darker focus colour for error summary links
NickColley Sep 6, 2018
a1922cb
Add darker focus colour for default link styles
NickColley Sep 6, 2018
37421a5
Add darker focus colour to header component
NickColley Sep 6, 2018
45b6d06
Add a new applied variable for focus text colour
NickColley Sep 6, 2018
2c1271d
Add no visited state mixin with new focus styles
NickColley Sep 10, 2018
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
3 changes: 3 additions & 0 deletions src/components/details/_details.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
// -1px offset fixes gap between background and outline in Firefox
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: I wonder if we should make it possible to pass an offset to the focusable helper, to avoid this duplication? As far as I can tell, the 1px offset is the only reason this isn't using the helper.

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 think the width of the outline itself is different too... I think best for another pull request.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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;
}

Expand Down
10 changes: 7 additions & 3 deletions src/components/error-summary/_error-summary.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,14 @@
&:link,
&:visited,
&:hover,
&:active,
&:focus {
&:active {
color: $govuk-error-colour;
text-decoration: underline;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just for consistency with the link styles, relying on the browser default?

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 far as I could tell, this did nothing?

}

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
color: $govuk-focus-text-colour;
}

// alphagov/govuk_template includes a specific a:link:focus selector
Expand Down
5 changes: 3 additions & 2 deletions src/components/footer/_footer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@
color: $govuk-footer-link-hover;
}

// Use text colour when focussed
// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
color: $govuk-text-colour;
color: $govuk-focus-text-colour;
}

// alphagov/govuk_template includes a specific a:link:focus selector
Expand Down
10 changes: 9 additions & 1 deletion src/components/header/_header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@
text-decoration: underline;
}

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
color: govuk-colour("black");
color: $govuk-focus-text-colour;
}

// alphagov/govuk_template includes a specific a:link:focus selector
Expand Down Expand Up @@ -239,6 +241,12 @@
&:visited {
color: $govuk-header-link-active;
}

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
color: $govuk-focus-text-colour;
}
}
}

Expand Down
15 changes: 1 addition & 14 deletions src/core/_links.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,7 @@
@include govuk-link-style-text;
}

// 'No visited state' link mixin
//
// Used in cases where it is not helpful to distinguish between visited and
// non-visited links.
//
// For example, navigation links to pages with dynamic content like admin
// dashboards. The content on the page is changing all the time, so the fact
// that you’ve visited it before is not important.
//
// This is not abstracted as a mixin because it does not provide states for
// all pseudo-selectors so it does not make sense to use it in composition.
.govuk-link--no-visited-state {
&:visited {
color: $govuk-link-colour;
}
@include govuk-link-style-no-visited-state;
}
}
53 changes: 52 additions & 1 deletion src/helpers/_links.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@
&:active {
color: $govuk-link-active-colour;
}

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&: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.

I tried changing the focusable mixin and that led to ordering problems since focus was not the last selector.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think this would be easy to fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just felt fragile having to move lots of code around to get the ordering right, we can explore it more, it would be nice to have it in the mixin.

I think one to do together if we decide to explore that.

color: $govuk-focus-text-colour;
}
}

/// Muted style link mixin
Expand Down Expand Up @@ -73,7 +79,7 @@
// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
color: $govuk-text-colour;
color: $govuk-focus-text-colour;
}

// alphagov/govuk_template includes a specific a:link:focus selector designed
Expand Down Expand Up @@ -122,6 +128,51 @@
}
}


/// No visited state link mixin
///
/// Used in cases where it is not helpful to distinguish between visited and
/// non-visited links.
///
/// For example, navigation links to pages with dynamic content like admin
/// dashboards. The content on the page is changing all the time, so the fact
/// that you’ve visited it before is not important.
///
/// If you use this mixin in a component you must also include the
/// govuk-link-common mixin in order to get the focus state.
///
/// @example scss
/// .govuk-component__link {
/// @include govuk-link-common;
/// @include govuk-link-style-no-visited-state;
/// }
///
/// @access public

@mixin govuk-link-style-no-visited-state {
&:link {
color: $govuk-link-colour;
}

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

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

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

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
color: $govuk-focus-text-colour;
}
}

/// Print friendly link mixin
///
/// When printing, append the the destination URL to the link text, as long
Expand Down
10 changes: 10 additions & 0 deletions src/settings/_colours-applied.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ $govuk-secondary-text-colour: govuk-colour("grey-1") !default;

$govuk-focus-colour: govuk-colour("yellow") !default;

/// Focused text colour
///
/// Ensure that the contrast between the text and background colour passes
/// WCAG Level AA contrast requirements.

This comment was marked as resolved.

Sign in to view

///
/// @type Colour
/// @access public

$govuk-focus-text-colour: govuk-colour("black") !default;

/// Error colour
///
/// Used to highlight error messages and form controls in an error state
Expand Down