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.16k stars 319 forks source link

Check autoprefixer behaviour within govuk-frontend #1940

Open vanitabarrett opened 4 years ago

vanitabarrett commented 4 years ago

I’m not 100% sure we’re using autoprefixer as intended. The prefixes are generated correctly for /dist when I condense the mixin down to just min-resolution, but NOT for /package.

Examples

src file with mixin condensed to just min-resolution, with $ratio expected behaviour: autoprefixer adds the additional prefixes needed actual behaviour: prefixes not generated

src file:

@mixin govuk-device-pixel-ratio($ratio: 2) {
  @media only screen and (min-resolution: #{($ratio*96)}dpi),
    only screen and (min-resolution: #{$ratio}dppx) {
      @content;
    }
}

generated package:

@mixin govuk-device-pixel-ratio($ratio: 2) {
  @media only screen and (min-resolution: #{($ratio*96)}dpi),
    only screen and (min-resolution: #{$ratio}dppx) {
      @content;
    }
}

The expected behaviour works ok when we remove the $ratio variable and include a hardcoded number instead

src file (hardcoded numbers):

# Condensed mixin to just min-resolution, with fixed number instead of $ratio
@mixin govuk-device-pixel-ratio() {
  @media only screen and (min-resolution: 2dpi),
    only screen and (min-resolution: 2dppx) {
      @content;
    }
}

generated package:

@mixin govuk-device-pixel-ratio() {
  @media only screen and (-webkit-min-device-pixel-ratio: 0.020833333333333332),
    only screen and (min-resolution: 2dpi),
    only screen and (-webkit-min-device-pixel-ratio: 2),
    only screen and (min-resolution: 2dppx) {
      @content;
    }
}

It looks like it’s tripped up by the use of the $ratio variable. When I raised this as an issue, it's suggested we're not using autoprefixer as intended (on CSS rather than on SCSS): https://github.com/postcss/autoprefixer/issues/1355. Not sure where else we’re relying on autoprefixer, but we may want to check/revisit our use of it

colinrotherham commented 1 year ago

Might be a good one to move to postcss.config.js (Co-op example) during https://github.com/alphagov/govuk-frontend/issues/2714?

colinrotherham commented 1 year ago

@vanitabarrett @36degrees Do you know enough about this issue to say if https://github.com/alphagov/govuk-frontend/pull/2870 has fixed it?

The Sass files were also filtered and may have had the same race condition issue

colinrotherham commented 1 year ago

Update: No change, the output (even with the postcss-scss parser) has stayed the same

colinrotherham commented 9 months ago

Update: Autoprefixer now drops the -o-min-device-pixel-ratio query:

@mixin govuk-device-pixel-ratio($ratio: 2) {
  @media only screen and (-webkit-min-device-pixel-ratio: $ratio),
-   only screen and (-o-min-device-pixel-ratio: #{($ratio * 10)} / 10),
    only screen and (min-resolution: #{($ratio * 96)}dpi),
    only screen and (min-resolution: #{$ratio}dppx) {
    @content;
  }
}

Although because it still can't parse #{$ratio} it would remove more in plain CSS:

@mixin govuk-device-pixel-ratio($ratio: 2) {
- @media only screen and (-webkit-min-device-pixel-ratio: $ratio),
-   only screen and (min-resolution: #{($ratio * 96)}dpi),
-   only screen and (min-resolution: #{$ratio}dppx) {
+ @media only screen and (-webkit-min-device-pixel-ratio: $ratio), only screen and (min-resolution: $ratio * 1dppx) {
    @content;
  }
}
colinrotherham commented 9 months ago

We do still run PostCSS + Autoprefixer on our Sass files with the following changes:

 .../components/exit-this-page/_index.scss     |  1 +
 .../dist/govuk/components/header/_index.scss  |  3 ++-
 .../dist/govuk/components/input/_index.scss   |  3 ++-
 .../govuk/components/warning-text/_index.scss |  4 +++-
 .../dist/govuk/helpers/_device-pixels.scss    |  1 -
 .../dist/govuk/helpers/_focused.scss          |  3 ++-
 .../dist/govuk/helpers/_links.scss            |  6 ++++--
 .../dist/govuk/helpers/_shape-arrow.scss      | 12 +++++++----
 .../dist/govuk/helpers/_visually-hidden.scss  | 21 +++++++++++++------
 .../dist/govuk/objects/_template.scss         |  6 ++++--
 10 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/packages/govuk-frontend/dist/govuk/components/exit-this-page/_index.scss b/packages/govuk-frontend/dist/govuk/components/exit-this-page/_index.scss
index d561e99f5..232edb31f 100644
--- a/packages/govuk-frontend/dist/govuk/components/exit-this-page/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/exit-this-page/_index.scss
@@ -5,6 +5,7 @@

   .govuk-exit-this-page {
     @include govuk-responsive-margin(8, "bottom");
+    position: -webkit-sticky;
     position: sticky;
     z-index: 1000;
     top: 0;
diff --git a/packages/govuk-frontend/dist/govuk/components/header/_index.scss b/packages/govuk-frontend/dist/govuk/components/header/_index.scss
index 0b6c47fb3..933eec30e 100644
--- a/packages/govuk-frontend/dist/govuk/components/header/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/header/_index.scss
@@ -192,7 +192,8 @@
     cursor: pointer;

     &:hover {
-      text-decoration: solid underline $govuk-header-link-underline-thickness;
+      -webkit-text-decoration: solid underline $govuk-header-link-underline-thickness;
+              text-decoration: solid underline $govuk-header-link-underline-thickness;

       @if $govuk-link-underline-offset {
         text-underline-offset: $govuk-link-underline-offset;
diff --git a/packages/govuk-frontend/dist/govuk/components/input/_index.scss b/packages/govuk-frontend/dist/govuk/components/input/_index.scss
index 5a71d64ff..4d72419d1 100644
--- a/packages/govuk-frontend/dist/govuk/components/input/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/input/_index.scss
@@ -17,7 +17,8 @@
     border-radius: 0;

     // Disable inner shadow and remove rounded corners
-    appearance: none;
+    -webkit-appearance: none;
+            appearance: none;

     &:focus {
       outline: $govuk-focus-width solid $govuk-focus-colour;
diff --git a/packages/govuk-frontend/dist/govuk/components/warning-text/_index.scss b/packages/govuk-frontend/dist/govuk/components/warning-text/_index.scss
index bf0cd8422..ac4a64975 100644
--- a/packages/govuk-frontend/dist/govuk/components/warning-text/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/warning-text/_index.scss
@@ -40,7 +40,9 @@

     // Prevent the exclamation mark from being included when the warning text
     // is copied, for example.
-    user-select: none;
+    -webkit-user-select: none;
+        -ms-user-select: none;
+            user-select: none;

     // Improve rendering in Windows High Contrast Mode (Edge), where a
     // readability backplate behind the exclamation mark obscures the circle
diff --git a/packages/govuk-frontend/dist/govuk/helpers/_device-pixels.scss b/packages/govuk-frontend/dist/govuk/helpers/_device-pixels.scss
index 1a6ef9f83..087597fe3 100644
--- a/packages/govuk-frontend/dist/govuk/helpers/_device-pixels.scss
+++ b/packages/govuk-frontend/dist/govuk/helpers/_device-pixels.scss
@@ -29,7 +29,6 @@

 @mixin govuk-device-pixel-ratio($ratio: 2) {
   @media only screen and (-webkit-min-device-pixel-ratio: $ratio),
-    only screen and (-o-min-device-pixel-ratio: #{($ratio * 10)} / 10),
     only screen and (min-resolution: #{($ratio * 96)}dpi),
     only screen and (min-resolution: #{$ratio}dppx) {
     @content;
diff --git a/packages/govuk-frontend/dist/govuk/helpers/_focused.scss b/packages/govuk-frontend/dist/govuk/helpers/_focused.scss
index e5e468e03..dd8c8793e 100644
--- a/packages/govuk-frontend/dist/govuk/helpers/_focused.scss
+++ b/packages/govuk-frontend/dist/govuk/helpers/_focused.scss
@@ -26,7 +26,8 @@

   // When a focused box is broken by e.g. a line break, ensure that the
   // box-shadow is applied to each fragment independently.
-  box-decoration-break: clone;
+  -webkit-box-decoration-break: clone;
+          box-decoration-break: clone;
 }

 /// Focused box
diff --git a/packages/govuk-frontend/dist/govuk/helpers/_links.scss b/packages/govuk-frontend/dist/govuk/helpers/_links.scss
index a20b82111..9dad751bb 100644
--- a/packages/govuk-frontend/dist/govuk/helpers/_links.scss
+++ b/packages/govuk-frontend/dist/govuk/helpers/_links.scss
@@ -52,8 +52,10 @@
     text-decoration-thickness: $govuk-link-hover-underline-thickness;
     // Disable ink skipping on underlines on hover. Browsers haven't
     // standardised on this part of the spec yet, so set both properties
-    text-decoration-skip-ink: none; // Chromium, Firefox
-    text-decoration-skip: none; // Safari
+    -webkit-text-decoration-skip-ink: none;
+            text-decoration-skip-ink: none; // Chromium, Firefox
+    -webkit-text-decoration-skip: none;
+            text-decoration-skip: none; // Safari
   }
 }

diff --git a/packages/govuk-frontend/dist/govuk/helpers/_shape-arrow.scss b/packages/govuk-frontend/dist/govuk/helpers/_shape-arrow.scss
index 68f6eaaf8..8036d4e91 100644
--- a/packages/govuk-frontend/dist/govuk/helpers/_shape-arrow.scss
+++ b/packages/govuk-frontend/dist/govuk/helpers/_shape-arrow.scss
@@ -51,22 +51,26 @@
   }

   @if $direction == "up" {
-    clip-path: polygon(50% 0%, 0% 100%, 100% 100%); // 3
+    -webkit-clip-path: polygon(50% 0%, 0% 100%, 100% 100%);
+            clip-path: polygon(50% 0%, 0% 100%, 100% 100%); // 3

     border-width: 0 $perpendicular $height $perpendicular;
     border-bottom-color: inherit; // 2
   } @else if $direction == "right" {
-    clip-path: polygon(0% 0%, 100% 50%, 0% 100%); // 3
+    -webkit-clip-path: polygon(0% 0%, 100% 50%, 0% 100%);
+            clip-path: polygon(0% 0%, 100% 50%, 0% 100%); // 3

     border-width: $perpendicular 0 $perpendicular $height;
     border-left-color: inherit; // 2
   } @else if $direction == "down" {
-    clip-path: polygon(0% 0%, 50% 100%, 100% 0%); // 3
+    -webkit-clip-path: polygon(0% 0%, 50% 100%, 100% 0%);
+            clip-path: polygon(0% 0%, 50% 100%, 100% 0%); // 3

     border-width: $height $perpendicular 0 $perpendicular;
     border-top-color: inherit; // 2
   } @else if $direction == "left" {
-    clip-path: polygon(0% 50%, 100% 100%, 100% 0%); // 3
+    -webkit-clip-path: polygon(0% 50%, 100% 100%, 100% 0%);
+            clip-path: polygon(0% 50%, 100% 100%, 100% 0%); // 3

     border-width: $perpendicular $height $perpendicular 0;
     border-right-color: inherit; // 2
diff --git a/packages/govuk-frontend/dist/govuk/helpers/_visually-hidden.scss b/packages/govuk-frontend/dist/govuk/helpers/_visually-hidden.scss
index ffdc7d608..827090723 100644
--- a/packages/govuk-frontend/dist/govuk/helpers/_visually-hidden.scss
+++ b/packages/govuk-frontend/dist/govuk/helpers/_visually-hidden.scss
@@ -37,7 +37,8 @@

   overflow: hidden if($important, !important, null);
   clip: rect(0 0 0 0) if($important, !important, null);
-  clip-path: inset(50%) if($important, !important, null);
+  -webkit-clip-path: inset(50%) if($important, !important, null);
+          clip-path: inset(50%) if($important, !important, null);

   border: 0 if($important, !important, null);

@@ -49,7 +50,9 @@
   // Prevent users from selecting or copying visually-hidden text. This prevents
   // a user unintentionally copying more text than they intended and needing to
   // manually trim it down again.
-  user-select: none;
+  -webkit-user-select: none;
+      -ms-user-select: none;
+          user-select: none;
 }

 /// Hide an element visually, but have it available for screen readers whilst
@@ -74,7 +77,8 @@

   overflow: hidden if($important, !important, null);
   clip: rect(0 0 0 0) if($important, !important, null);
-  clip-path: inset(50%) if($important, !important, null);
+  -webkit-clip-path: inset(50%) if($important, !important, null);
+          clip-path: inset(50%) if($important, !important, null);

   // For long content, line feeds are not interpreted as spaces and small width
   // causes content to wrap 1 word per line:
@@ -84,7 +88,9 @@
   // Prevent users from selecting or copying visually-hidden text. This prevents
   // a user unintentionally copying more text than they intended and needing to
   // manually trim it down again.
-  user-select: none;
+  -webkit-user-select: none;
+      -ms-user-select: none;
+          user-select: none;

   &:active,
   &:focus {
@@ -96,11 +102,14 @@

     overflow: visible if($important, !important, null);
     clip: auto if($important, !important, null);
-    clip-path: none if($important, !important, null);
+    -webkit-clip-path: none if($important, !important, null);
+            clip-path: none if($important, !important, null);

     white-space: inherit if($important, !important, null);

     // Allow the text to be selectable now it's visible
-    user-select: text;
+    -webkit-user-select: text;
+        -ms-user-select: text;
+            user-select: text;
   }
 }
diff --git a/packages/govuk-frontend/dist/govuk/objects/_template.scss b/packages/govuk-frontend/dist/govuk/objects/_template.scss
index ac246829c..34c50b804 100644
--- a/packages/govuk-frontend/dist/govuk/objects/_template.scss
+++ b/packages/govuk-frontend/dist/govuk/objects/_template.scss
@@ -9,7 +9,9 @@

     // Prevent automatic text sizing, as we already cater for small devices and
     // would like the browser to stay on 100% text zoom by default.
-    text-size-adjust: 100%;
+    -webkit-text-size-adjust: 100%;
+       -moz-text-size-adjust: 100%;
+            text-size-adjust: 100%;

     // Add scroll padding to the top of govuk-template but remove it if the
     // exit this page component is present.
@@ -23,7 +25,7 @@
     // a "wrong way round" way as we hypothesise that the risks of having
     // scroll-padding unnecessarily is better than risking not having scroll-padding
     // and needing it to account for exit this page.
-    @supports (position: sticky) {
+    @supports ((position: -webkit-sticky) or (position: sticky)) {
       scroll-padding-top: govuk-spacing(9);

       &:not(:has(.govuk-exit-this-page)) {