Enable new link styles by default

In July 2021, we made some improvements to new link styles [1] which were released as part of v3.12.0. We introduced these changes behind a feature flag `$govuk-new-link-styles: true` which service teams needed to set before importing GOV.UK Frontend in order to opt-in. This was in case users: - needed to make changes to their service to introduce these new link styles consistently - wanted to avoid opting in if they have links within a multiple-column layout in your CSS, due to Known issue: hover state is mis-painted on links within CSS multiple-column layouts #2204 Now that #2204 has been fixed in Chromium, we can make the new link styles ‘opt-out’. [1]: #2350
alphagov · May 10, 2023 · e28fa1a · e28fa1a
1 parent 33fc54c
commit e28fa1a
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 91 deletions.
diff --git a/packages/govuk-frontend-review/src/stylesheets/app.scss b/packages/govuk-frontend-review/src/stylesheets/app.scss
@@ -1,5 +1,4 @@
 $govuk-show-breakpoints: true;
-$govuk-new-link-styles: true;
 
 @import "govuk/all";
 @import "partials/app";

diff --git a/packages/govuk-frontend-review/src/stylesheets/full-page-examples/campaign-page.scss b/packages/govuk-frontend-review/src/stylesheets/full-page-examples/campaign-page.scss
@@ -1,5 +1,4 @@
 @import "govuk/base";
-$govuk-new-link-styles: true;
 @import "govuk/core/all";
 
 .app-header--campaign {

diff --git a/packages/govuk-frontend-review/src/stylesheets/full-page-examples/search.scss b/packages/govuk-frontend-review/src/stylesheets/full-page-examples/search.scss
@@ -1,5 +1,4 @@
 @import "govuk/base";
-$govuk-new-link-styles: true;
 @import "govuk/core/all";
 
 .app-document-list > li {

diff --git a/packages/govuk-frontend/src/govuk-prototype-kit/init.scss b/packages/govuk-frontend/src/govuk-prototype-kit/init.scss
@@ -7,6 +7,5 @@ $govuk-assets-path: if(
 ) !default;
 
 $govuk-global-styles: true !default;
-$govuk-new-link-styles: true !default;
 
 @import "../govuk/all";
diff --git a/packages/govuk-frontend/src/govuk/helpers/links.test.js b/packages/govuk-frontend/src/govuk/helpers/links.test.js
@@ -1,9 +1,40 @@
 const { compileSassString } = require('govuk-frontend-helpers/tests')
 
 describe('@mixin govuk-link-decoration', () => {
-  describe('by default', () => {
+  it('sets text-decoration-thickness', async () => {
+    const sass = `
+      $govuk-link-underline-thickness: 1px;
+      @import "base";
+
+      .foo {
+        @include govuk-link-decoration;
+      }
+    `
+
+    await expect(compileSassString(sass)).resolves.toMatchObject({
+      css: expect.stringContaining('text-decoration-thickness: 1px;')
+    })
+  })
+
+  it('sets text-underline-offset', async () => {
+    const sass = `
+      $govuk-link-underline-offset: .1em;
+      @import "base";
+
+      .foo {
+        @include govuk-link-decoration;
+      }
+    `
+
+    await expect(compileSassString(sass)).resolves.toMatchObject({
+      css: expect.stringContaining('text-underline-offset: 0.1em;')
+    })
+  })
+
+  describe('when $govuk-new-link-styles are disabled', () => {
     it('does not set text-decoration-thickness', async () => {
       const sass = `
+        $govuk-new-link-styles: false;
         @import "base";
 
         .foo {
@@ -18,6 +49,7 @@ describe('@mixin govuk-link-decoration', () => {
 
     it('does not set text-underline-offset', async () => {
       const sass = `
+        $govuk-new-link-styles: false;
         @import "base";
 
         .foo {
@@ -31,11 +63,10 @@ describe('@mixin govuk-link-decoration', () => {
     })
   })
 
-  describe('when $govuk-new-link-styles are enabled', () => {
-    it('sets text-decoration-thickness', async () => {
+  describe('when $govuk-link-underline-thickness is falsey', () => {
+    it('does not set text-decoration-thickness', async () => {
       const sass = `
-        $govuk-new-link-styles: true;
-        $govuk-link-underline-thickness: 1px;
+        $govuk-link-underline-thickness: false;
         @import "base";
 
         .foo {
@@ -44,117 +75,78 @@ describe('@mixin govuk-link-decoration', () => {
       `
 
       await expect(compileSassString(sass)).resolves.toMatchObject({
-        css: expect.stringContaining('text-decoration-thickness: 1px;')
+        css: expect.not.stringContaining('text-decoration-thickness')
       })
     })
+  })
 
-    it('sets text-underline-offset', async () => {
+  describe('when $govuk-link-underline-offset is falsey', () => {
+    it('does not set text-decoration-offset ', async () => {
       const sass = `
-        $govuk-new-link-styles: true;
-        $govuk-link-underline-offset: .1em;
-        @import "base";
+      $govuk-link-underline-offset: false;
+      @import "base";
 
-        .foo {
+      .foo {
           @include govuk-link-decoration;
-        }
-      `
+      }
+    `
 
       await expect(compileSassString(sass)).resolves.toMatchObject({
-        css: expect.stringContaining('text-underline-offset: 0.1em;')
-      })
-    })
-
-    describe('when $govuk-link-underline-thickness is falsey', () => {
-      it('does not set text-decoration-thickness', async () => {
-        const sass = `
-          $govuk-new-link-styles: true;
-          $govuk-link-underline-thickness: false;
-          @import "base";
-
-          .foo {
-            @include govuk-link-decoration;
-          }
-        `
-
-        await expect(compileSassString(sass)).resolves.toMatchObject({
-          css: expect.not.stringContaining('text-decoration-thickness')
-        })
-      })
-    })
-
-    describe('when $govuk-link-underline-offset is falsey', () => {
-      it('does not set text-decoration-offset ', async () => {
-        const sass = `
-        $govuk-new-link-styles: true;
-        $govuk-link-underline-offset: false;
-        @import "base";
-
-        .foo {
-            @include govuk-link-decoration;
-        }
-      `
-
-        await expect(compileSassString(sass)).resolves.toMatchObject({
-          css: expect.not.stringContaining('text-underline-offset')
-        })
+        css: expect.not.stringContaining('text-underline-offset')
       })
     })
   })
 })
 
 describe('@mixin govuk-link-hover-decoration', () => {
-  describe('by default', () => {
-    it('does not set a hover state', async () => {
-      const sass = `
+  it('sets a hover state', async () => {
+    const sass = `
       @import "base";
 
-      // The mixin shouldn't return anything, so this selector ends up empty and
-      // is omitted from the CSS
       .foo:hover {
-          @include govuk-link-hover-decoration;
+        @include govuk-link-hover-decoration;
       }
     `
 
-      await expect(compileSassString(sass)).resolves.toMatchObject({
-        css: expect.not.stringContaining('.foo:hover')
-      })
+    await expect(compileSassString(sass)).resolves.toMatchObject({
+      css: expect.stringContaining('.foo:hover')
     })
   })
 
-  describe('when $govuk-new-link-styles are enabled', () => {
-    it('sets a hover state', async () => {
+  describe('when $govuk-new-link-styles are disabled', () => {
+    it('does not set a hover state', async () => {
       const sass = `
-        $govuk-new-link-styles: true;
-        $govuk-link-hover-underline-thickness: 10px;
+        $govuk-new-link-styles: false;
         @import "base";
 
+        // The mixin shouldn't return anything, so this selector ends up empty and
+        // is omitted from the CSS
         .foo:hover {
-          @include govuk-link-hover-decoration;
+            @include govuk-link-hover-decoration;
         }
       `
 
       await expect(compileSassString(sass)).resolves.toMatchObject({
-        css: expect.stringContaining('.foo:hover')
+        css: expect.not.stringContaining('.foo:hover')
       })
     })
+  })
 
-    describe('when $govuk-link-hover-underline-thickness is falsey', () => {
-      it('does not set a hover state', async () => {
-        const sass = `
-        $govuk-new-link-styles: true;
-        $govuk-link-hover-underline-thickness: false;
-        @import "base";
+  describe('when $govuk-link-hover-underline-thickness is falsey', () => {
+    it('does not set a hover state', async () => {
+      const sass = `
+      $govuk-link-hover-underline-thickness: false;
+      @import "base";
 
-        // The mixin shouldn't return anything, so this selector ends up empty and
-        // is omitted from the CSS
-        .foo:hover {
-            @include govuk-link-hover-decoration;
-        }
-      `
+      // The mixin shouldn't return anything, so this selector ends up empty and
+      // is omitted from the CSS
+      .foo:hover {
+          @include govuk-link-hover-decoration;
+      }
+    `
 
-        await expect(compileSassString(sass)).resolves.toMatchObject({
-          css: expect.not.stringContaining('.foo:hover')
-        })
+      await expect(compileSassString(sass)).resolves.toMatchObject({
+        css: expect.not.stringContaining('.foo:hover')
       })
     })
   })

diff --git a/packages/govuk-frontend/src/govuk/settings/_links.scss b/packages/govuk-frontend/src/govuk/settings/_links.scss
@@ -10,16 +10,10 @@
 /// - have a clearer hover state, where the underline gets thicker to make the
 ///   link stand out to users
 ///
-/// You should only enable the new link styles if both:
-///
-/// - you've made sure your whole service will use the new style consistently
-/// - you do not have links in a multi-column CSS layout - there's [a Chromium
-///   bug that affects links](https://github.com/alphagov/govuk-frontend/issues/2204)
-///
 /// @type Boolean
 /// @access public
 
-$govuk-new-link-styles: false !default;
+$govuk-new-link-styles: true !default;
 
 /// Thickness of link underlines
 ///