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.17k stars 320 forks source link

Add break-word typography helper #5159

Closed querkmachine closed 2 months ago

querkmachine commented 2 months ago

Teeny little PR to add a new typography helper for breaking long words, such as email addresses or 50% of the German language, across multiple lines when a container is likely to be too narrow to contain it.

This suggestion was originally raised in #2233. It felt useful to create it as a mixin, rather than have service teams implement it as-needed, due to older Microsoft browsers only supporting a non-standard, unprefixed version of the property.

Comments on the issue noted that we probably cannot apply a 'one size fits all' CSS rule to text generally, as this would interfere with components that are intrinsically sized according to their contents, such as the Tag component.

Resolves #2233.

Changes

Todo

github-actions[bot] commented 2 months ago

:clipboard: Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 112.61 KiB
dist/govuk-frontend-development.min.js 41.83 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 87.33 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 82.03 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.6 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 41.82 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.16 KiB 39.79 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.05 KiB 6.06 KiB

View stats and visualisations on the review app


Action run for e87fcee7df4b912a31468edae0b7958e4a0e0444

github-actions[bot] commented 2 months 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 801fafa64..3e1eaeb38 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -7553,6 +7553,11 @@ screen and (-ms-high-contrast:active) {
     font-variant-numeric: tabular-nums !important
 }

+.govuk-\!-text-break-word {
+    word-wrap: break-word !important;
+    overflow-wrap: break-word !important
+}
+
 .govuk-\!-width-full,
 .govuk-\!-width-three-quarters {
     width: 100% !important

Action run for e87fcee7df4b912a31468edae0b7958e4a0e0444

github-actions[bot] commented 2 months ago

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/helpers/_typography.scss b/packages/govuk-frontend/dist/govuk/helpers/_typography.scss
index 57b9959e9..49a0d06d5 100644
--- a/packages/govuk-frontend/dist/govuk/helpers/_typography.scss
+++ b/packages/govuk-frontend/dist/govuk/helpers/_typography.scss
@@ -79,6 +79,23 @@
   font-variant-numeric: tabular-nums if($important, !important, null);
 }

+/// Word break helper
+///
+/// Forcibly breaks long words that lack spaces, such as email addresses,
+/// across multiple lines when they wouldn't otherwise fit.
+///
+/// @param {Boolean} $important [false] - Whether to mark declarations as
+///   `!important`. Generally used to create override classes.
+/// @access public
+
+@mixin govuk-text-break-word($important: false) {
+  // IE 11 and Edge 16–17 only support the non-standard `word-wrap` property
+  word-wrap: break-word if($important, !important, null);
+
+  // All other browsers support `overflow-wrap`
+  overflow-wrap: break-word if($important, !important, null);
+}
+
 /// Convert line-heights specified in pixels into a relative value, unless
 /// they are already unit-less (and thus already treated as relative values)
 /// or the units do not match the units used for the font size.
diff --git a/packages/govuk-frontend/dist/govuk/overrides/_typography.scss b/packages/govuk-frontend/dist/govuk/overrides/_typography.scss
index 8afef6c1c..3b542c713 100644
--- a/packages/govuk-frontend/dist/govuk/overrides/_typography.scss
+++ b/packages/govuk-frontend/dist/govuk/overrides/_typography.scss
@@ -28,11 +28,15 @@
     @include govuk-typography-weight-bold($important: true);
   }

-  // Tabular numbers
+  // Typography helpers

   .govuk-\!-font-tabular-numbers {
     @include govuk-font-tabular-numbers($important: true);
   }
+
+  .govuk-\!-text-break-word {
+    @include govuk-text-break-word($important: true);
+  }
 }

 /*# sourceMappingURL=_typography.scss.map */

Action run for e87fcee7df4b912a31468edae0b7958e4a0e0444

romaricpascal commented 2 months ago

Another non-blocking question: what do we reckon to filtering govuk-font-break-word into the parent govuk-font mixin?

Not sure I understand what you mean by this, are you talking about having govuk-font have an option to also apply govuk-font-break-word?

This brough to my mind that I'd also associate breaking words with 'text' setting rather than 'font' setting, so the `govuk-font' prefix feels a bit odd to me, but this may be my brain being too picky.

romaricpascal commented 2 months ago

This brough to my mind that I'd also associate breaking words with 'text' setting rather than 'font' setting, so the `govuk-font' prefix feels a bit odd to me, but this may be my brain being too picky.

Had a quick check and that distinction goes all the way down to CSS specs:

I'm not sure it's a distinction that'll be in the mind of people looking for the feature, though. I'm wondering if we need the font mention at all and could just go with:

@querkmachine what drove the govuk-font and govuk-!-font prefix?

querkmachine commented 2 months ago

@romaricpascal Typically I would try to name a mixin/override to be the same as the CSS property and value being defined, but didn't do that here as multiple properties are being defined and overflow-wrap just doesn't strike me as being a very intuitive name for what the styles do (it sounds more like something to do with the overflow property.)

Otherwise it's close to what you thought: There doesn't seem to be a clear distinction between 'font', 'type' and 'typography' in the minds of most people, and we've generally seemed to gravitate towards using the word 'font' for all mixins relating to text in recent changes.

I'm open to changing the names. From my perspective, having some word to faux namespace the mixin seems like a nice-to-have, just so it's obvious it's in the font/text group of mixins and classes—though the mixins will all be namespaced under typography once Sass modules come into play, anyway.

romaricpascal commented 2 months ago

@querkmachine cheers for the explanation. Agree that namespacing sound nice and that overflow wouldn't make a great for the reason you mention.

font doesn't seem quite right a namespace either (for example, we don't force a font-text-align either to namespace the text-align utilities). How about:

querkmachine commented 2 months ago

Renamed 🎉

querkmachine commented 2 months ago

Another non-blocking question: what do we reckon to filtering govuk-font-break-word into the parent govuk-font mixin?

I don't have any particular reasons against making it available via govuk-font other than that my assumption is that the override class is going to be used very specifically and contextually—either on pre-composed content where a developer knows a long, unbroken string will appear, or when playing back particular user-provided content that has the potential to contain long, unbroken strings.

In my mind, that means that only single words (in prose) or elements (in a component) will have the mixin/class applied, rather than it being part of a wider reaching font configuration. That's entirely assumptive though—it (probably) doesn't really make any difference where the break-word styles are placed.