Update details to use new link styles

Because we want to apply the hover state when any part of the summary is hovered (including the marker), refactor the `govuk-link-decoration` mixin to split out the contents of the `:hover` selector so that we can apply it to components seperately.
alphagov · Apr 30, 2021 · 934a684 · 934a684
1 parent 471c817
commit 934a684
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 37 deletions.
diff --git a/src/govuk/components/details/_index.scss b/src/govuk/components/details/_index.scss
@@ -34,7 +34,11 @@
 
   // ...but only underline the text, not the arrow
   .govuk-details__summary-text {
-    text-decoration: underline;
+    @include govuk-link-decoration;
+  }
+
+  .govuk-details__summary:hover .govuk-details__summary-text {
+    @include govuk-link-hover-decoration;
   }
 
   // Remove the underline when focussed to avoid duplicate borders

diff --git a/src/govuk/helpers/_links.scss b/src/govuk/helpers/_links.scss
@@ -12,6 +12,10 @@
   @include govuk-typography-common;
   @include govuk-link-decoration;
 
+  &:hover {
+    @include govuk-link-hover-decoration;
+  }
+
   &:focus {
     @include govuk-focused-text;
   }
@@ -33,12 +37,19 @@
     @if ($govuk-link-underline-offset) {
       text-underline-offset: $govuk-link-underline-offset;
     }
+  }
+}
 
-    @if ($govuk-link-hover-underline-thickness) {
-      &:hover {
-        text-decoration-thickness: $govuk-link-hover-underline-thickness;
-      }
-    }
+/// Link hover decoration mixin
+///
+/// Provides the text decoration for links in their hover state, intended to
+/// be used within a `:hover` pseudo-selector
+///
+/// @access public
+
+@mixin govuk-link-hover-decoration {
+  @if ($govuk-new-link-styles and $govuk-link-hover-underline-thickness) {
+    text-decoration-thickness: $govuk-link-hover-underline-thickness;
   }
 }
 

diff --git a/src/govuk/helpers/links.test.js b/src/govuk/helpers/links.test.js
@@ -33,19 +33,6 @@ describe('@mixin govuk-link-decoration', () => {
 
       expect(results.css.toString()).not.toContain('text-underline-offset')
     })
-
-    it('does not set a hover state', async () => {
-      const sass = `
-      @import "base";
-  
-      .foo {
-          @include govuk-link-decoration;
-      }`
-
-      const results = await renderSass({ data: sass, ...sassConfig })
-
-      expect(results.css.toString()).not.toContain(':hover')
-    })
   })
 
   describe('when $govuk-new-link-styles are enabled', () => {
@@ -79,21 +66,6 @@ describe('@mixin govuk-link-decoration', () => {
       expect(results.css.toString()).toContain('text-underline-offset: 0.1em;')
     })
 
-    it('sets text-decoration-thickness on hover', async () => {
-      const sass = `
-        $govuk-new-link-styles: true;
-        $govuk-link-hover-underline-thickness: 10px;
-        @import "base";
-  
-        .foo {
-          @include govuk-link-decoration;
-        }`
-
-      const results = await renderSass({ data: sass, ...sassConfig })
-
-      expect(results.css.toString()).toContain('.foo:hover { text-decoration-thickness: 10px; }')
-    })
-
     describe('when $govuk-link-underline-thickness is falsey', () => {
       it('does not set text-decoration-thickness', async () => {
         const sass = `
@@ -107,7 +79,7 @@ describe('@mixin govuk-link-decoration', () => {
 
         const results = await renderSass({ data: sass, ...sassConfig })
 
-        expect(results.css.toString()).not.toMatch(/\.foo {.*text-decoration-thickness.*}/)
+        expect(results.css.toString()).not.toContain('text-decoration-thickness')
       })
     })
 
@@ -127,6 +99,42 @@ describe('@mixin govuk-link-decoration', () => {
         expect(results.css.toString()).not.toContain('text-underline-offset')
       })
     })
+  })
+})
+
+describe('@mixin govuk-link-hover-decoration', () => {
+  describe('by default', () => {
+    it('does not set 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;
+      }`
+
+      const results = await renderSass({ data: sass, ...sassConfig })
+
+      expect(results.css.toString()).not.toContain('.foo:hover')
+    })
+  })
+
+  describe('when $govuk-new-link-styles are enabled', () => {
+    it('sets text-decoration-thickness on hover', async () => {
+      const sass = `
+        $govuk-new-link-styles: true;
+        $govuk-link-hover-underline-thickness: 10px;
+        @import "base";
+
+        .foo:hover {
+          @include govuk-link-hover-decoration;
+        }`
+
+      const results = await renderSass({ data: sass, ...sassConfig })
+
+      expect(results.css.toString()).toContain('.foo:hover { text-decoration-thickness: 10px; }')
+    })
 
     describe('when $govuk-link-hover-underline-thickness is falsey', () => {
       it('does not set a hover state', async () => {
@@ -135,8 +143,10 @@ describe('@mixin govuk-link-decoration', () => {
         $govuk-link-hover-underline-thickness: false;
         @import "base";
     
-        .foo {
-            @include govuk-link-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;
         }`
 
         const results = await renderSass({ data: sass, ...sassConfig })