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

Updates to link styles and link hover states #2183

Merged
merged 11 commits into from
May 5, 2021
Merged

Conversation

36degrees
Copy link
Contributor

@36degrees 36degrees commented Apr 9, 2021

This is a tidied up version of #2089.

We're making changes to link styles in order to improve their readability and to make the hover state clearer.

These changes will be behind a feature flag and 'opt-in' for their initial release. This is because:

  • there's a a rendering bug in Chromium that affects links inside CSS columns (see 'known issues')
  • users that have implemented their own link styles will likely need to make changes to their CSS in order for their links behave consistently.

In a future major version, we'll flip the feature flag so that this behaviour is opt-out, and then deprecate and remove the feature flag.

Improve readability of links

Reduce the thickness of link underlines and offset them from the text in order to improve readability. This should help especially when multiple links are included in long lists (for example, search results pages) or at larger sizes.

Before After
Screenshot before changes Screenshot after changes

Allow the thickness and offset to be overridden, so that anyone using GOV.UK Frontend outside of GOV.UK can opt out of these changes.

Make link hover state clearer with thick underline

We have a long-standing issue (#1417) from an audit in May 2019 about the link hover state not being clear:

The colour change when a user hovers over a link is not clear and this was especially difficult for low vision users to determine.

Ensuring that the state change is distinctive would benefit low vision users in particular, while benefiting all mouse users in general.

This was raised as a usability issue rather than a WCAG failure.

We’ve been keeping an eye on browser support for the text-decoration-thickness property, and its introduction into Chrome 87 means that all evergreen browsers now support it. We can therefore use text-decoration-thickess as a way to differentiate the link on hover.

Before After
Screenshot before changes Screenshot after changes
Screen.Recording.2021-04-09.at.15.50.50.mov

Some of the rationale for this is included in a comment around the time of the previous working group review. The broad intention is that the thicker underline is a state change that can work across any combination of text colour and background colour, as long as the link has enough contrast in its default state.

We set the underline to a minimum width to 3px with a max to 0.12em. The aim is to make the state change clear whilst avoiding the underline crashing into the text below at larger sizes. A minimum 3px width ensures at least a 2px change from the default 1px thickness. It’s a bit close with 16px body-s text, but if you set the minimum width too low, the hover thickness at 19px stays too thin (original comment).

We're retaining the existing colour change on hover, as without the colour change there would be no state change in browsers that don't support text-decoration-thickness, including IE11. We can revisit this decision as and when our browser support requirements change (original comment).

Browser testing

  • IE 11
  • Edge
  • Chrome
  • Firefox
  • Safari
  • Safari (iOS)
  • Chrome (iOS)
  • Chrome (Android)
  • Samsung Internet (Android)
  • Edge (Windows High Contrast Mode)
  • Firefox (Forced colours)
  • Firefox (Zoomed to 400%, zoom text only)

Known issues

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 9, 2021 14:58 Inactive
src/govuk/helpers/_links.scss Outdated Show resolved Hide resolved
src/govuk/helpers/_links.scss Show resolved Hide resolved
@@ -103,3 +103,27 @@ $govuk-focus-width: 3px !default;
/// @access public

$govuk-hover-width: 10px !default;

/// Link underline thickness
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is measurements the right place for these settings? I also considered creating a new settings/_links.scss file for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Worth thinking about the effect this has on the Sass docs as well – at the minute it'd be under measurements which doesn't feel right. Equally if we did have a settings/links it feels a little odd that all of the link colour settings will still be under settings/colours.

@include govuk-link-decoration;

&:hover {
@include govuk-link-hover-decoration;
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 separation is just to support details, which may be changing soon anyway – the alternative would be to duplicate more of the CSS in the details component? But this might be a helpful separation for anyone building their own components where the hover interaction is a little more complex.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 12, 2021 13:49 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 12, 2021 14:09 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 13, 2021 08:43 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 13, 2021 09:45 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 16, 2021 10:49 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 19, 2021 11:03 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 19, 2021 14:26 Inactive
@36degrees 36degrees marked this pull request as ready for review April 19, 2021 14:49
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 19, 2021 14:56 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 20, 2021 15:29 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 20, 2021 15:31 Inactive
@36degrees 36degrees linked an issue Apr 21, 2021 that may be closed by this pull request
2 tasks
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 21, 2021 10:48 Inactive
src/govuk/components/accordion/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/footer/_index.scss Outdated Show resolved Hide resolved
@include govuk-text-colour;
}

// Force a colour change on hover to work around a bug in Safari
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👌

src/govuk/helpers/_links.scss Outdated Show resolved Hide resolved
Split into separate `text-decoration` and `text-decoration-thickness` properties because Safari doesn’t support `text-decoration-thickness` in the `text-decoration` shorthand.

Safari (and older browsers that don’t support text-decoration-thickness) ignore the property entirely when it contains a thickness, so splitting it also ensures that older browsers still get the underline on elements that wouldn’t normally have an underline (like the details component)
Ensure that link underlines scale appropriately if a user has a root font size other than 16px, for example if they’ ‘zoom text size only’.

1px / 16px root font size = 0.0625rem
3px / 16px root font size = 0.1875rem
There’s a bug in Safari that means that changes to text-decoration-thickness on hover are not applied above certain font sizes, unless there is also another change – like a change to the text colour. [1]

Add a text colour change to the muted link style by making it black (text-coloured) on hover.

Force a text colour change for text and inverse links (which currently don’t change colour) by setting the alpha channel to 99% on hover. This should generally be imperceivable, but is enough to force Safari to paint the hover style correctly.

[1]: https://bugs.webkit.org/show_bug.cgi?id=224483
When GOV.UK Frontend is used in compatibility mode alongside GOV.UK Elements, Elements sets the root font size to 10px.

In rem, the thickness of underlines in the hover state with a 10px root font size is 1.875px (10/16ths of 3) rather than 3px and so the hover state does not display correctly.

Include both px and rem, as this means that in situations where the root font size is less than 16px, the value in px will 'win' and so the underlines are displayed correctly.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2183 April 30, 2021 16:11 Inactive
@36degrees
Copy link
Contributor Author

36degrees commented Apr 30, 2021

This PR has been reworked so that it now contains only the changes to the core link styles.

Commits that refactored components to use the link mixins, removed redundant code or made other minor design changes have been cherry-picked out into separate PRs (all of which have already been merged):

The two new link mixins and modifier classes were also split out:

Finally, changes to components that rely on the changes in this PR (because they update the design, or because they rely on the new 'decoration' mixins introduced) have been split out into #2215, which builds on top of the commits in this PR.

You can see the changes introduced by the restructure (mostly incorporating changes from master bought in through rebasing) by diffing the head of the #2215 against the original head of this PR (d83d853)

Diff output
git diff link-style-component-updates d83d853
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7eae5910..c322c391 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,11 @@ This was added in [pull request #2164: Enable cookie banner to set link styled a
 - [#2132: Improve vertical alignment of phase banner tag on mobile devices](https://github.com/alphagov/govuk-frontend/pull/2132) – thanks to [@matthewmascord](https://github.com/matthewmascord) for contributing this.
 - [#2157: Use pointer cursor for 'Menu' button in header](https://github.com/alphagov/govuk-frontend/pull/2157) – thanks to [@MalcolmVonMoJ](https://github.com/MalcolmVonMoJ) for contributing this.
 - [#2171: Fix padding on GOV.UK logo affecting hover and focus states](https://github.com/alphagov/govuk-frontend/pull/2171)
+- [#2186: Fix display of warning text in Edge when Windows High Contrast Mode is enabled](https://github.com/alphagov/govuk-frontend/pull/2186)
+- [#2192: Add data-nosnippet to prevent cookie banner text appearing in Google Search snippets](https://github.com/alphagov/govuk-frontend/pull/2192)
+- [#2201: Set -webkit-appearance: button on file upload so text is aligned in Safari](https://github.com/alphagov/govuk-frontend/pull/2201)
+- [#2205: Stop file upload component 'jumping' on focus](https://github.com/alphagov/govuk-frontend/pull/2205)
+- [#2212: Add underline to crown copyright link in footer](https://github.com/alphagov/govuk-frontend/pull/2212)
 
 ## 3.11.0 (Feature release)
 
diff --git a/app/assets/scss/app.scss b/app/assets/scss/app.scss
index 5990db57..89769afa 100644
--- a/app/assets/scss/app.scss
+++ b/app/assets/scss/app.scss
@@ -1,4 +1,5 @@
 $govuk-show-breakpoints: true;
+$govuk-new-link-styles: true;
 
 @import "../../../src/govuk/all";
 @import "partials/app";
diff --git a/app/views/examples/links/index.njk b/app/views/examples/links/index.njk
index b41ad6c5..a82ddfb8 100644
--- a/app/views/examples/links/index.njk
+++ b/app/views/examples/links/index.njk
@@ -15,7 +15,7 @@
   'Link with no visited state': 'govuk-link--no-visited-state',
   'Text link': 'govuk-link--text-colour',
   'Muted link': 'govuk-link--muted',
-  'Inverse link': 'govuk-link--inverse'
+  'Inverse': 'govuk-link--inverse'
 } %}
 
 {% set states = {
@@ -51,7 +51,7 @@
     {% for variant_description, variant_class in variants %}
 
       <section class="govuk-!-margin-top-8"{% if variant_class == "govuk-link--inverse"%} style="background: #1d70b8; padding: 15px; margin: -15px;"{% endif %}>
-        <h2 class="govuk-heading-l">{{ variant_description }}</h2>
+        <h2 class="govuk-heading-l"{% if variant_class == "govuk-link--inverse"%} style="color: #fff;"{% endif %}>{{ variant_description }}</h2>
 
       {% for state_description, state_class in states %}
         <p class="govuk-body">
diff --git a/src/govuk/components/cookie-banner/README.md b/src/govuk/components/cookie-banner/README.md
new file mode 100644
index 00000000..1c919808
--- /dev/null
+++ b/src/govuk/components/cookie-banner/README.md
@@ -0,0 +1,15 @@
+# Cookie banner
+
+## Installation
+
+See the [main README quick start guide](https://github.com/alphagov/govuk-frontend#quick-start) for how to install this component.
+
+## Guidance and Examples
+
+Find out when to use the cookie banner component in your service in the [GOV.UK Design System](https://design-system.service.gov.uk/components/cookie-banner).
+
+## Component options
+
+Use options to customise the appearance, content and behaviour of a component when using a macro, for example, changing the text.
+
+See [options table](https://design-system.service.gov.uk/components/cookie-banner/#options-default-cookie-banner-example) for details.
diff --git a/src/govuk/components/cookie-banner/cookie-banner.yaml b/src/govuk/components/cookie-banner/cookie-banner.yaml
index 295f7f45..bfb24dd5 100644
--- a/src/govuk/components/cookie-banner/cookie-banner.yaml
+++ b/src/govuk/components/cookie-banner/cookie-banner.yaml
@@ -2,7 +2,7 @@ params:
 - name: ariaLabel
   type: string
   required: false
-  description: The text for the `aria-label` which labels the cookie banner region. This region applies to all messages that the cookie banner includes. For example, the cookie message and the replacement message. Defaults to "Cookie banner".
+  description: The text for the `aria-label` which labels the cookie banner region. This region applies to all messages that the cookie banner includes. For example, the cookie message and the confirmation message. Defaults to "Cookie banner".
 - name: hidden
   type: boolean
   required: false
@@ -18,7 +18,7 @@ params:
 - name: messages
   type: array
   required: true
-  description: The different messages you can pass into the cookie banner. For example, the cookie message and the replacement message.
+  description: The different messages you can pass into the cookie banner. For example, the cookie message and the confirmation message.
   params:
     - name: headingText
       type: string
@@ -72,11 +72,11 @@ params:
     - name: hidden
       type: boolean
       required: false
-      description: Defaults to `false`. If you set it to `true`, the message is hidden. You can use `hidden` for client-side implementations where the replacement message HTML is present, but hidden on the page.
+      description: Defaults to `false`. If you set it to `true`, the message is hidden. You can use `hidden` for client-side implementations where the confirmation message HTML is present, but hidden on the page.
     - name: role
       type: string
       required: false
-      description: Set `role` to `alert` on replacement messages to allow assistive tech to automatically read the message. You will also need to move focus to the replacement message using JavaScript you have written yourself.
+      description: Set `role` to `alert` on confirmation messages to allow assistive tech to automatically read the message. You will also need to move focus to the confirmation message using JavaScript you have written yourself.
     - name: classes
       type: string
       required: false
diff --git a/src/govuk/components/cookie-banner/template.njk b/src/govuk/components/cookie-banner/template.njk
index 8e016f91..c539b5b3 100644
--- a/src/govuk/components/cookie-banner/template.njk
+++ b/src/govuk/components/cookie-banner/template.njk
@@ -1,6 +1,6 @@
 {% from "../button/macro.njk" import govukButton -%}
 
-<div class="govuk-cookie-banner {{ params.classes if params.classes }}" role="region" aria-label="{{ params.ariaLabel | default("Cookie banner") }}"
+<div class="govuk-cookie-banner {{ params.classes if params.classes }}" data-nosnippet role="region" aria-label="{{ params.ariaLabel | default("Cookie banner") }}"
   {%- if params.hidden %} hidden{% endif %}
   {%- for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}>
 
diff --git a/src/govuk/components/cookie-banner/template.test.js b/src/govuk/components/cookie-banner/template.test.js
index e9c2276a..6b757935 100644
--- a/src/govuk/components/cookie-banner/template.test.js
+++ b/src/govuk/components/cookie-banner/template.test.js
@@ -248,6 +248,13 @@ describe('Cookie Banner', () => {
       const $actions = $('.govuk-cookie-banner__message[hidden]')
       expect($actions.length).toEqual(2)
     })
+
+    it('has a data-nosnippet attribute to hide it from search result snippets', () => {
+      const $ = render('cookie-banner', examples['client-side implementation'])
+
+      const $parentContainer = $('.govuk-cookie-banner')
+      expect($parentContainer.attr('data-nosnippet')).toEqual('')
+    })
   })
 
   describe('full cookie banner hidden', () => {
diff --git a/src/govuk/components/file-upload/_index.scss b/src/govuk/components/file-upload/_index.scss
index 0fae18e0..97ffc19e 100644
--- a/src/govuk/components/file-upload/_index.scss
+++ b/src/govuk/components/file-upload/_index.scss
@@ -8,17 +8,21 @@
   .govuk-file-upload {
     @include govuk-font($size: 19);
     @include govuk-text-colour;
-    padding-top: $component-padding;
-    padding-bottom: $component-padding;
+    margin-left: -$component-padding;
+    padding: $component-padding;
+
+    // The default file upload button in Safari does not
+    // support setting a custom font-size. Set `-webkit-appearance`
+    // to `button` to drop out of the native appearance so the
+    // font-size is set to 19px
+    // https://bugs.webkit.org/show_bug.cgi?id=224746
+    &::-webkit-file-upload-button {
+      -webkit-appearance: button;
+      color: inherit;
+      font: inherit;
+    }
 
     &:focus {
-      // "Yank" the padding with negative margin to avoid a jump
-      // when element is focused
-      margin-right: -$component-padding;
-      margin-left: -$component-padding;
-      padding-right: $component-padding;
-      padding-left: $component-padding;
-
       outline: $govuk-focus-width solid $govuk-focus-colour;
       // Use `box-shadow` to add border instead of changing `border-width`
       // (which changes element size) and since `outline` is already used for the
@@ -37,11 +41,6 @@
     // to recognise `focus-within` and don't set any styles from the block
     // when it's a selector.
     &:focus-within {
-      margin-right: -$component-padding;
-      margin-left: -$component-padding;
-      padding-right: $component-padding;
-      padding-left: $component-padding;
-
       outline: $govuk-focus-width solid $govuk-focus-colour;
 
       box-shadow: inset 0 0 0 4px $govuk-input-border-colour;
diff --git a/src/govuk/components/warning-text/_index.scss b/src/govuk/components/warning-text/_index.scss
index 12b5d3fa..a0d5edb9 100644
--- a/src/govuk/components/warning-text/_index.scss
+++ b/src/govuk/components/warning-text/_index.scss
@@ -43,6 +43,16 @@
     // Prevent the exclamation mark from being included when the warning text
     // is copied, for example.
     user-select: none;
+
+    // Improve rendering in Windows High Contrast Mode (Edge), where a
+    // readability backplate behind the exclamation mark obscures the circle
+    forced-color-adjust: none;
+
+    @media screen and (forced-colors: active) {
+      border-color: windowText;
+      color: windowText;
+      background: transparent;
+    }
   }
 
   .govuk-warning-text__text {
diff --git a/src/govuk/helpers/links.test.js b/src/govuk/helpers/links.test.js
index fe881ab2..dbe54847 100644
--- a/src/govuk/helpers/links.test.js
+++ b/src/govuk/helpers/links.test.js
@@ -79,7 +79,7 @@ describe('@mixin govuk-link-decoration', () => {
 
         const results = await renderSass({ data: sass, ...sassConfig })
 
-        expect(results.css.toString()).not.toContain('text-decoration-thickness;')
+        expect(results.css.toString()).not.toContain('text-decoration-thickness')
       })
     })
 
diff --git a/src/govuk/settings/_links.scss b/src/govuk/settings/_links.scss
index d966fff0..bd516adf 100644
--- a/src/govuk/settings/_links.scss
+++ b/src/govuk/settings/_links.scss
@@ -11,16 +11,17 @@ $govuk-new-link-styles: false !default;
 
 /// Link underline thickness
 ///
-/// Default is a 1px underline for all links regardless of size, but is defined
-/// in rem so that it scales accordingly if the user has a different root font
-/// size ('zoom text only')
+/// The default is, for a given link, whichever is thicker:
+///  - an absolute 1px underline
+///  - .0625rem, equivalent to 1px but adjusts with the root font size if the
+///.   user has zoomed in 'text only'
 ///
 /// Set to false to disable setting a thickness
 ///
 /// @type Number
 /// @access public
 
-$govuk-link-underline-thickness: .0625rem !default;
+$govuk-link-underline-thickness: unquote("max(1px, .0625rem)") !default;
 
 /// Link underline offset.
 ///
@@ -33,8 +34,10 @@ $govuk-link-underline-offset: .1em !default;
 
 /// Link hover state underline thickness
 ///
-/// Default is, for a given link, whichever is the thicker:
-///  - a 3px underline (defined in rem to scale with the root font size)
+/// The default is, for a given link, whichever is the thicker of:
+///  - an absolute 3px underline
+///  - .1875rem, which equivalent to 3px (at 16px root font size) but adjusts
+///    with the root font size if the user has zoomed in 'text only'
 ///  - .12em relative to the link's text size
 ///
 /// Set to false to disable setting a thickness
@@ -42,4 +45,4 @@ $govuk-link-underline-offset: .1em !default;
 /// @type Number
 /// @access public
 
-$govuk-link-hover-underline-thickness: unquote("max(.1875rem, .12em)") !default;
+$govuk-link-hover-underline-thickness: unquote("max(3px, .1875rem, .12em)") !default;

@chao-xian
Copy link

I've spotted that the offset doesn't seem to be working in FF...?
Screenshot 2021-05-01 at 01 04 31

@36degrees
Copy link
Contributor Author

I've spotted that the offset doesn't seem to be working in FF...?

@chao-xian I think it is working, but the baseline for the offset is not consistent across browsers so it looks different in Firefox and Chrome. Is the hover state working correctly? And where is that screenshot from?

I think actually it's Chrome that's wrong (at least according to http://crbug.com/1172623) but we've had to find a value that looks alright in all browsers. As and when Chrome fix their baseline, we'll probably tweak the offset slightly to work better in all browsers.

@36degrees 36degrees requested a review from hannalaakso May 4, 2021 16:17
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Awesome work @36degrees and @christopherthomasdesign, these new link styles are a big improvement in readability and usability 🎉

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.

Improve the readability of links Link hover states are not clear
6 participants