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 Warning Text font weight when <strong> styles are reset #5331

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

romaricpascal
Copy link
Member

@romaricpascal romaricpascal commented Sep 17, 2024

There's no guarantee browser defaults of bold or bolder have not been overriden so we need to set the font-weight explicitely.

Thoughts

Rather than reverting to having both the .govuk-warning-text__icon and .govuk-warning-text__text each have a font-weight (as they used to before v5.0.0), I took the opportunity to regroup the font configuration in .govuk-warning-text and explicitely solve the problem that strong may not have the right font-weight set.

I think this clarifies intentions that 'we want this component to be in 19px bold' but <strong> gets in the way so we solve that particular issue.

Closes #5279

@romaricpascal romaricpascal requested a review from a team September 17, 2024 13:34
Copy link

github-actions bot commented Sep 17, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 118.54 KiB
dist/govuk-frontend-development.min.js 43.63 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 90.19 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 84.69 KiB
packages/govuk-frontend/dist/govuk/all.mjs 1.05 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 118.52 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 43.62 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 4.97 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 81.8 KiB 41.48 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
service-navigation.mjs 4.46 KiB 2.69 KiB
skip-link.mjs 4.39 KiB 2.18 KiB
tabs.mjs 10.05 KiB 6.06 KiB

View stats and visualisations on the review app


Action run for 87b7566

@romaricpascal romaricpascal force-pushed the warning-text-font-weight branch from ea4f2f9 to 9927286 Compare September 17, 2024 13:35
@romaricpascal romaricpascal marked this pull request as draft September 17, 2024 13:36
Copy link

github-actions bot commented Sep 17, 2024

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 1bbb7da0a..7fbe9acd4 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -6389,6 +6389,7 @@ screen and (-ms-high-contrast:active) {
     font-size: 1rem;
     line-height: 1.25;
     margin-bottom: 20px;
+    font-weight: 700;
     position: relative;
     padding: 10px 0
 }
@@ -6420,7 +6421,6 @@ screen and (-ms-high-contrast:active) {
 }
 
 .govuk-warning-text__icon {
-    font-weight: 700;
     box-sizing: border-box;
     display: inline-block;
     position: absolute;
@@ -6458,7 +6458,8 @@ screen and (-ms-high-contrast:active) {
 .govuk-warning-text__text {
     color: #0b0c0c;
     display: block;
-    padding-left: 45px
+    padding-left: 45px;
+    font-weight: inherit
 }
 
 @media print {

Action run for 87b7566

Copy link

github-actions bot commented Sep 17, 2024

Other changes to npm package

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 4e79791f2..9ee4807d2 100644
--- a/packages/govuk-frontend/dist/govuk/components/warning-text/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/warning-text/_index.scss
@@ -2,14 +2,12 @@
   .govuk-warning-text {
     @include govuk-font($size: 19);
     @include govuk-responsive-margin(6, "bottom");
+    @include govuk-typography-weight-bold;
     position: relative;
     padding: govuk-spacing(2) 0;
   }
 
   .govuk-warning-text__icon {
-    // We apply this here and not at the parent level because the actual text is
-    // a <strong> and so will always be bold
-    @include govuk-typography-weight-bold;
     box-sizing: border-box;
 
     display: inline-block;
@@ -59,6 +57,9 @@
     @include govuk-text-colour;
     display: block;
     padding-left: 45px;
+    // While `<strong>` is styled `bold` or `bolder` by user-agents
+    // this can be reset by the app's stylesheet
+    font-weight: inherit;
   }
 }
 

Action run for 87b7566

There's no guarantee browser defaults of `bold` or `bolder` have not been overriden
so we need to set the font-weight explicitely.

Closes #5279
@romaricpascal romaricpascal force-pushed the warning-text-font-weight branch from 9927286 to 5620565 Compare September 17, 2024 14:16
@romaricpascal romaricpascal changed the title Explicitely set Warning Text font weight Fix Warning Text font weight when <strong> styles are reset Sep 17, 2024
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5331 September 17, 2024 14:22 Inactive
@romaricpascal romaricpascal marked this pull request as ready for review September 17, 2024 14:27
Copy link
Contributor

@owenatgov owenatgov left a comment

Choose a reason for hiding this comment

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

From testing, trying to set a font-weight at the strong level is overrided by this, meaning it's working!

@romaricpascal romaricpascal merged commit 838ab44 into main Sep 19, 2024
51 checks passed
@romaricpascal romaricpascal deleted the warning-text-font-weight branch September 19, 2024 09:51
@owenatgov owenatgov mentioned this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warning text does not explicitly set font-weight
3 participants