alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.13k stars 319 forks source link

Fix whitespace affecting text alignment in pagination block variant #5066

Open querkmachine opened 2 weeks ago

querkmachine commented 2 weeks ago

An attempt at fixing the issue where whitespace (or the lack of it) changes the alignment of the title text in the Pagination component's 'block' variant, as described in #3554.

Changes

Thoughts

Focus style difference

This PR does slightly change the focus state on these links, with the focus state now stretching to fit the longest line of text, rather than separately 'shrinkwrapping' the title and label lines.

Before After
Screenshot of some links, with the first one focused. The focused text has a yellow background that follows the length of each line of text and a thick black border under the last line. Screenshot of some links, with the first one focused. The top link has a rectangular yellow background that is sized according to the longer second line, with a thick black border underneath it.

I was unable to find a simple way around this using only CSS and wanted to avoid a breaking HTML change.

I considered removing the focus style from the actual link and re-applying it to the two child elements, but doing so introduced more complexity than I'd like, so I'm gonna 🤞 and hope this is okay until we're content with making a breaking change.

0.326 is the magic number

This also introduces a magic number, the margin-top on the arrows themselves. Being floated means they no longer automatically align with the text baseline, so some manual offsetting was required.

This could potentially be worked out programmatically: it's $(lineHeight-arrowHeight)\div2$, specifically $(25-13)\div2=6px$ with numbers added, which is ≅0.326em in this context.

However, we don't presently have a function for getting just the line-height for a specific type size and breakpoint, and I was still trying to avoid adding unnecessary complexity, and the pagination's line-height doesn't change per breakpoint anyway, so I've hardcoded it. This value is precise enough to visually match the arrow's previous position on even high zoom levels.

A two-frame animation showing before and after screenshots. In the before, the first line of title text is slightly aligned too far to the left and the second line is entirely to the left. In the after, both lines are aligned with the label text below it. Neither the arrow nor label text move between the before/after.

These comparison screenshots were made in Firefox at 300% zoom on a 72 DPI display.

github-actions[bot] commented 2 weeks ago

:clipboard: Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 112.99 KiB
dist/govuk-frontend-development.min.js 41.88 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 87.42 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 82.11 KiB
packages/govuk-frontend/dist/govuk/all.mjs 981 B
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 112.98 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 41.87 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 4.86 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 79.24 KiB 39.84 KiB
accordion.mjs 23.5 KiB 12.39 KiB
button.mjs 5.98 KiB 2.69 KiB
character-count.mjs 22.4 KiB 9.92 KiB
checkboxes.mjs 5.83 KiB 2.83 KiB
error-summary.mjs 7.89 KiB 3.46 KiB
exit-this-page.mjs 17.1 KiB 9.26 KiB
header.mjs 4.46 KiB 2.6 KiB
notification-banner.mjs 6.26 KiB 2.62 KiB
password-input.mjs 15.15 KiB 7.25 KiB
radios.mjs 4.83 KiB 2.38 KiB
skip-link.mjs 4.39 KiB 2.18 KiB
tabs.mjs 10.13 KiB 6.11 KiB

View stats and visualisations on the review app


Action run for dca5a1039d244743a49c2c03e2eb4f97b95f9930

github-actions[bot] commented 2 weeks ago

Stylesheets changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
index ceac53d84..8fcc945a1 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -4490,8 +4490,7 @@ only screen and (min-resolution:2dppx) {
     text-decoration: underline;
     text-decoration-thickness: max(1px, .0625rem);
     text-underline-offset: .1578em;
-    display: inline-block;
-    padding-left: 30px
+    display: inline-block
 }

 .govuk-pagination__icon {
@@ -4514,59 +4513,43 @@ only screen and (min-resolution:2dppx) {
     display: block
 }

-.govuk-pagination--block .govuk-pagination__item {
-    padding: 15px;
-    float: none
-}
-
 .govuk-pagination--block .govuk-pagination__next,
 .govuk-pagination--block .govuk-pagination__prev {
     padding-left: 0;
     float: none
 }

-.govuk-pagination--block .govuk-pagination__next {
-    padding-right: 15px
+.govuk-pagination--block .govuk-pagination__next .govuk-pagination__link,
+.govuk-pagination--block .govuk-pagination__prev .govuk-pagination__link {
+    display: inline-block
 }

-.govuk-pagination--block .govuk-pagination__next .govuk-pagination__icon {
-    margin-left: 0
+.govuk-pagination--block .govuk-pagination__next {
+    padding-right: 15px
 }

 .govuk-pagination--block .govuk-pagination__prev+.govuk-pagination__next {
     border-top: 1px solid #b1b4b6
 }

-.govuk-pagination--block .govuk-pagination__link,
-.govuk-pagination--block .govuk-pagination__link-title {
-    display: inline
-}
-
 .govuk-pagination--block .govuk-pagination__link-title:after {
     content: "";
     display: block
 }

 .govuk-pagination--block .govuk-pagination__link {
+    padding-left: 30px;
     text-align: left
 }

-.govuk-pagination--block .govuk-pagination__link:focus .govuk-pagination__link-label {
-    outline: 3px solid transparent;
-    color: #0b0c0c;
-    background-color: #fd0;
-    box-shadow: 0 -2px #fd0, 0 4px #0b0c0c;
-    text-decoration: none;
-    -webkit-box-decoration-break: clone;
-    box-decoration-break: clone
-}
-
 .govuk-pagination--block .govuk-pagination__link:not(:focus) {
     text-decoration: none
 }

 .govuk-pagination--block .govuk-pagination__icon {
-    margin-right: 10px
+    margin-top: .326em;
+    margin-left: -30px;
+    float: left
 }

 .govuk-panel {

Action run for dca5a1039d244743a49c2c03e2eb4f97b95f9930

github-actions[bot] commented 2 weeks ago

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/components/pagination/_index.scss b/packages/govuk-frontend/dist/govuk/components/pagination/_index.scss
index 9bd247007..7ae869ac4 100644
--- a/packages/govuk-frontend/dist/govuk/components/pagination/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/pagination/_index.scss
@@ -151,7 +151,6 @@
     @include govuk-typography-weight-regular;
     @include govuk-link-decoration;
     display: inline-block;
-    padding-left: govuk-spacing(6);
   }

   .govuk-pagination__icon {
@@ -175,23 +174,18 @@
   .govuk-pagination--block {
     display: block;

-    .govuk-pagination__item {
-      padding: govuk-spacing(3);
-      float: none;
-    }
-
     .govuk-pagination__next,
     .govuk-pagination__prev {
       padding-left: 0;
       float: none;
+
+      .govuk-pagination__link {
+        display: inline-block;
+      }
     }

     .govuk-pagination__next {
       padding-right: govuk-spacing(3);
-
-      .govuk-pagination__icon {
-        margin-left: 0;
-      }
     }

     // Only apply a border between prev and next if both are present
@@ -199,13 +193,6 @@
       border-top: 1px solid $govuk-border-colour;
     }

-    // Reset both these elements to their inline default, both to ensure that
-    // the focus state for block mode "shrink wraps" text as expected
-    .govuk-pagination__link,
-    .govuk-pagination__link-title {
-      display: inline;
-    }
-
     // Set the after pseudo element to a block which makes the title visually
     // display as block level whilst programmatically being inline. We do this
     // to get around an NVDA quirk where adjacent block level elements are
@@ -216,24 +203,24 @@
     }

     .govuk-pagination__link {
+      padding-left: govuk-spacing(6);
       text-align: left;

-      &:focus {
-        // Apply focus styling to the label within the link as if it were being
-        // focused to get around a display issue with a focusable inline element
-        // containing a mixture of inline and inline-block level elements
-        .govuk-pagination__link-label {
-          @include govuk-focused-text;
-        }
-      }
-
       &:not(:focus) {
         text-decoration: none;
       }
     }

     .govuk-pagination__icon {
-      margin-right: govuk-spacing(2);
+      // This magic number is brought to you by the following equation:
+      // ((lineHeight − arrowHeight) ÷ 2) ÷ fontSize
+      // ((25 − 13) ÷ 2) ÷ 19 = 0.326em
+      //
+      // This could have been done programmatically but we don't have functions
+      // for grabbing the line-height of specific typography sizes just yet.
+      margin-top: 0.326em;
+      margin-left: govuk-spacing(6) * -1;
+      float: left;
     }
   }
 }

Action run for dca5a1039d244743a49c2c03e2eb4f97b95f9930