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

Fix LibSass calc() compatibility in Radios and Checkboxes #4784

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Feb 20, 2024

This PR interpolates Sass calc() contents for compatibility with LibSass

It fixes #4782 using the suggestion from @oscarduignan

LibSass compatibility

Our Sass tests didn't catch LibSass calc() differences in #4093

This was unintentional and compatibility with LibSass and Ruby Sass should be maintained

Although GOV.UK Frontend currently supports LibSass (version 3.3.0 and above) and Ruby Sass (version 3.4.0 and above), we will remove support in future. If you’re using either of these Sass compilers, you should migrate to Dart Sass as soon as you reasonably can.

Updated tests

I've added new checks for $govuk-* in Sass output, using grep with ! to throw when matches are not found:

! grep "\$govuk-" .tmp/all.css

This ensures we break the build and would output the following with $govuk-* matches:

  max-width: calc(100% - (($govuk-checkboxes-label-padding-left-right * 2) + $govuk-touch-target-size));
  max-width: calc(100% - ($govuk-radios-label-padding-left-right + $govuk-touch-target-size + govuk-spacing(3)));

This partially closes #4783 but does not check for govuk-spacing() etc

colinrotherham and others added 2 commits February 20, 2024 16:15
See GitHub issue: #4782

Co-authored-by: Oscar Duignan <100650+oscarduignan@users.noreply.github.com>
@colinrotherham colinrotherham requested a review from a team as a code owner February 20, 2024 16:43
@colinrotherham colinrotherham changed the title Interpolate contents of calc() for LibSass Fix LibSass calc() compatibility in Radios and Checkboxes Feb 20, 2024
Copy link

github-actions bot commented Feb 20, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 112.47 KiB
dist/govuk-frontend-development.min.js 38.58 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 78.74 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 73.99 KiB
packages/govuk-frontend/dist/govuk/all.mjs 3.86 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 112.46 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 38.57 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.38 KiB

Modules

File Size
all.mjs 70.32 KiB
components/accordion/accordion.mjs 21.67 KiB
components/button/button.mjs 4.7 KiB
components/character-count/character-count.mjs 21.24 KiB
components/checkboxes/checkboxes.mjs 5.83 KiB
components/error-summary/error-summary.mjs 6.57 KiB
components/exit-this-page/exit-this-page.mjs 16.08 KiB
components/header/header.mjs 4.46 KiB
components/notification-banner/notification-banner.mjs 4.93 KiB
components/radios/radios.mjs 4.83 KiB
components/skip-link/skip-link.mjs 4.39 KiB
components/tabs/tabs.mjs 10.16 KiB

View stats and visualisations on the review app


Action run for 8715b9a

@romaricpascal
Copy link
Member

romaricpascal commented Feb 20, 2024

Just to confirm the understanding of the grep flags, for future documentation. Is the following correct?

  • --invert-match means grep will exit 1 when it finds the regexp
  • --null-data makes grep treat the file as a whole rather than line by line
  • --quiet prevents outputting the whole file when it is OK

EDIT: Last push made this irrelevant

Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad we ended with a nice output in the end by splitting the grep in its own step 🥳 ⛵

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4784 February 20, 2024 17:36 Inactive
Copy link

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/components/checkboxes/_index.scss b/packages/govuk-frontend/dist/govuk/components/checkboxes/_index.scss
index eb2ce95b0..d34fc39f5 100644
--- a/packages/govuk-frontend/dist/govuk/components/checkboxes/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/checkboxes/_index.scss
@@ -41,7 +41,7 @@
     // container minus the input width minus the padding on either side of
     // the label. This prevents the label from going onto the next line due to
     // __item using flex-wrap because we want hints on a separate line.
-    max-width: calc(100% - (($govuk-checkboxes-label-padding-left-right * 2) + $govuk-touch-target-size));
+    max-width: calc(100% - #{(($govuk-checkboxes-label-padding-left-right * 2) + $govuk-touch-target-size)});
     margin-bottom: 0;
     padding: (govuk-spacing(1) + $govuk-border-width-form-element) govuk-spacing(3);
     cursor: pointer;
diff --git a/packages/govuk-frontend/dist/govuk/components/radios/_index.scss b/packages/govuk-frontend/dist/govuk/components/radios/_index.scss
index edb04303b..51fc8055d 100644
--- a/packages/govuk-frontend/dist/govuk/components/radios/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/radios/_index.scss
@@ -43,7 +43,7 @@
     // container minus the input width minus the padding on either side of
     // the label. This prevents the label from going onto the next line due to
     // __item using flex-wrap because we want hints on a separate line
-    max-width: calc(100% - ($govuk-radios-label-padding-left-right + $govuk-touch-target-size + govuk-spacing(3)));
+    max-width: calc(100% - #{($govuk-radios-label-padding-left-right + $govuk-touch-target-size + govuk-spacing(3))});
     margin-bottom: 0;
     padding: (govuk-spacing(1) + $govuk-border-width-form-element) govuk-spacing(3);
     cursor: pointer;

Action run for 8715b9a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants