-
Notifications
You must be signed in to change notification settings - Fork 324
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
Add break-word typography helper #5159
Conversation
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for e87fcee |
Stylesheets changes to npm packagediff --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 e87fcee |
Other changes to npm packagediff --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 e87fcee |
ab8633d
to
00947bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of little comments, nothing blocking. Looks grand!
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 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
@querkmachine what drove the |
@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 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 |
00947bc
to
30f154c
Compare
@querkmachine cheers for the explanation. Agree that namespacing sound nice and that
|
30f154c
to
66ad8f5
Compare
66ad8f5
to
e87fcee
Compare
Renamed 🎉 |
I don't have any particular reasons against making it available via 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. |
Add break-word typography helper
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
govuk-text-break-word
mixin, which sets both the nonstandardword-wrap
and standardoverflow-wrap
CSS properties.govuk-!-text-break-word
override class.Todo