-
Notifications
You must be signed in to change notification settings - Fork 81
Add text-wrap: balance to all headings
#949
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 text-wrap: balance to all headings
#949
Conversation
|
@janbrasna Do you remember what the problem is with |
|
@stephaniehobson So the issue is what risk this might pose for downstream consumers, where balancing text IIUC actually competes with (For just bedrock this should be a non-issue, as balancing headings has been around for some time now.) |
|
Seems like it follows the cascade in Firefox and Chrome so any custom styles they have applied should take president if they're importing their styles after the Protocol one. I think we're safe to go ahead but I'd add something to the CHANGELOG's migration notes to search for any place where |
|
It might be a nice change to include with V20 which will have other font updates. |
|
@janbrasna Do you want to add some CHANGELOG notes to this PR so I can merge it? I'm thinking V20 will go sometime next week and I'd like to have this in it. |
|
@stephaniehobson Yup, on it. I'll try to add some reasonable heads up to the migration notes too. (I haven't checked this is not also included in the wholesale font update in the other PR, porting other related values battle tested in bedrock, but will do so! — I actually found this to be a solution to an "unrelated" whitespace / hyphens / wordwrapping issue where having balanced headings helps avoiding a regression when allowing implicit hyphens, so can't wait to have this in and start removing weird forced word wrapping.) Landing this in a major / breaking release makes the most sense; I was mainly cautious about slipping this in a minor update regarding any impact to consumers, but v20 is great to have this included in, being full of similar changes. 🚀 |
4f3aa31 to
f724684
Compare
stephaniehobson
left a comment
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.
R+ ⚖️
Description
https://caniuse.com/css-text-wrap-balance is pretty well supported now with no real downside, known bugs or performance issues documented.
I have documented this change in the design system.CHANGELOG.md.Issue
Resolves #910
Testing
Safari 16 vs. 17.5+
Firefox 115 (ESR) vs. 122+
PR: https://preview--mzp-dev.netlify.app/components/detail/headings
(before:)

(after:)
