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

Check autoprefixer behaviour within govuk-frontend #1940

Open
vanitabarrett opened this issue Sep 2, 2020 · 5 comments
Open

Check autoprefixer behaviour within govuk-frontend #1940

vanitabarrett opened this issue Sep 2, 2020 · 5 comments
Labels
🕔 hours A well understood issue which we expect to take less than a day to resolve. 🔍 investigation tooling

Comments

@vanitabarrett
Copy link
Contributor

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): postcss/autoprefixer#1355. Not sure where else we’re relying on autoprefixer, but we may want to check/revisit our use of it

@vanitabarrett vanitabarrett added the awaiting triage Needs triaging by team label Sep 2, 2020
@trang-erskine trang-erskine added 🔍 investigation 🕔 hours A well understood issue which we expect to take less than a day to resolve. and removed awaiting triage Needs triaging by team labels Sep 7, 2020
@colinrotherham
Copy link
Contributor

Might be a good one to move to postcss.config.js (Co-op example) during #2714?

@colinrotherham
Copy link
Contributor

@vanitabarrett @36degrees Do you know enough about this issue to say if #2870 has fixed it?

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

@colinrotherham
Copy link
Contributor

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

@colinrotherham
Copy link
Contributor

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
Copy link
Contributor

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)) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕔 hours A well understood issue which we expect to take less than a day to resolve. 🔍 investigation tooling
Projects
None yet
Development

No branches or pull requests

4 participants
@colinrotherham @vanitabarrett @trang-erskine and others