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

Defend against a:link:focus in GOV.UK Template #609

Merged
merged 9 commits into from
Mar 28, 2018
Merged

Defend against a:link:focus in GOV.UK Template #609

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fb4c5af
Defend components from a:link:focus in GOV.UK Template
36degrees Mar 16, 2018
12e1e79
Defend muted links from a:link:focus in GOV.UK Template
36degrees Mar 16, 2018
264028f
fixup! Defend components from a:link:focus in GOV.UK Template
36degrees Mar 21, 2018
e426868
Add ‘compatibility mode’ settings and tool
36degrees Mar 21, 2018
293fcaf
Update footer to use compatibility mixin
36degrees Mar 23, 2018
f82730e
Update error summary to use compatibility mixin
36degrees Mar 23, 2018
eeff255
Use abstracted text style link mixins
36degrees Mar 23, 2018
5d8735e
Add example of text links to links page
36degrees Mar 23, 2018
74e5012
Use alphagov/govuk_template consistently
36degrees Mar 23, 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
10 changes: 9 additions & 1 deletion src/back-link/_back-link.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,18 @@
// Underline is provided by a bottom border
text-decoration: none;

// Override link colour to use text colour
//
// GOV.UK Template includes a specific a:link:focus selector designed to
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of 'GOV.UK Template' should we use 'alphagov/govuk_template' which is less ambiguous for people in the future?

If we intend to deprecate GOV.UK Template there will be people in the future that don't even know what this is.

// make unvisited links a slightly darker blue when focussed, so we need to
// override the text colour for that combination of selectors so so that
// the skip link does not end up with dark blue text when focussed. (1)
&:link,
&:visited,
&:hover,
&:active {
&:active,
&:focus,
&:link:focus {
@include govuk-text-colour;
}

Expand Down
10 changes: 9 additions & 1 deletion src/breadcrumbs/_breadcrumbs.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,18 @@
.govuk-c-breadcrumbs__link {
@include govuk-focusable-fill;

// Override link colour to use text colour
//
// GOV.UK 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 so so that
// the skip link does not end up with dark blue text when focussed. (1)
&:link,
&:visited,
&:hover,
&:active {
&:active,
Copy link
Contributor

@NickColley NickColley Mar 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Could we break these out into their own sections and do something like

.class {
  &:link,
  &:visited,
  &:hover,
  &:active {
    color: $govuk-text-colour;
  }

  if ($with-govuk-template) {
    ... overrides here
  }
}

This would mean if we wanted to do 'pure' builds we could.

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 like this in principle, but not sure how it'll work in practise – especially for users that are not compiling the Sass themselves. I'm not sure what we'd do for the dist build, for example?

Also, I'm not sure that we have the tooling in place to have confidence in this when introducing yet more complexity into the builds. In the example above we'd need to set color: $govuk-text-colour; in two places, and I could e.g. foresee one being changed without the other which could lead to inconsistent and hard to reproduce problems.

Copy link
Contributor Author

@36degrees 36degrees Mar 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Pushed a branch to show what this might look like in practise.

Copy link
Contributor

Choose a reason for hiding this comment

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

For general builds we would set these to on by default, as is the same as your current code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Low-fi is just to make sure each selector that overrides for govuk-template has a comment next to it, which most of these do?

&:focus,
&:link:focus {
color: $govuk-text-colour;
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/button/_button.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,14 @@
}

// Ensure that any global link styles are overridden
//
// GOV.UK 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 so so that
// unvisited links styled as buttons do not end up with dark blue text when
// focussed. (1)
&:link,
&:link:focus, // 1
&:visited,
&:active,
&:hover {
Expand Down
13 changes: 10 additions & 3 deletions src/error-summary/_error-summary.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,20 @@

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

// Override default link styling to use error colour
//
// GOV.UK 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 so so that
// the links do not end up with dark blue text when focussed. (1)
&:link,
&:visited,
&:hover,
&:active {
@include govuk-typography-weight-bold;

&:active,
&:focus,
&:link:focus // 1 {
color: $govuk-error-colour;
text-decoration: underline;
}
Expand Down
11 changes: 11 additions & 0 deletions src/footer/_footer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@
&:active {
color: $govuk-footer-link-hover;
}

// Use text colour when focussed
//
// GOV.UK 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 so so that
// unvisited footer links do not end up with dark blue text when focussed.
&:focus,
&:link:focus {
color: $govuk-text-colour;
}
}

.govuk-c-footer__section-break {
Expand Down
8 changes: 7 additions & 1 deletion src/globals/core/_links.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,13 @@

// When focussed, the text colour needs to be darker to ensure that colour
// contrast is still acceptable
&:focus {
//
// GOV.UK 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 so so that
// muted links does not end up with dark blue text when focussed. (1)
&:focus,
&:link:focus { //1
color: $govuk-black;
}
}
Expand Down
12 changes: 10 additions & 2 deletions src/skip-link/_skip-link.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,16 @@
padding: $govuk-spacing-scale-2 $govuk-spacing-scale-3;

// Default link colour doesn't have enough contrast against the focus colour
&:focus,
&:visited {
//
// GOV.UK 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 so so that
// the skip link does not end up with dark blue text when focussed. (1)
&:link,
&:link:focus, // 1
&:visited,
&:active,
&:hover {
color: $govuk-text-colour;
}
}
Expand Down