-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Headings block: add support for font weight #27639
Headings block: add support for font weight #27639
Conversation
Looking good. Do you think it also makes sense to add a default for the block to Bold as @kjellr suggested here: #22641 Another question is how does this interact with the Bold button in the Block Controls? The difference is that the Bold button allows you to set only a portion of the text to bold. The problem is that if the block is set to One option we could explore is to change the behaviour of the |
Yes, I was trying that and I was not getting it to work so I decided to unblock the TT1 issue with this PR and work on the default separately. I'm still unsure about setting the default to bold for the headings as that will affect every theme, I'd like some feedback on that too.
Yes, I would go with controlling what font-weight |
Important to note here that one is a visual appearance change for the font weight, and the other may be that also has semantic value. For that reason, I don't think we should necessarily change what "bold" does to a heading. I personally think that it's fine that pressing "bold" on an already bold header makes it less bold — it does what the theme registers bold to. More importantly, the less CSS we bake in, the more is in the control of the theme. |
Yeah, I agree with @jasmussen here. I also think (hope?) that eventually, Gutenberg could make that "bold" button recognize the "Bold" appearance, and show up as selected by default when that's the case. In the meantime, this seems like a good next step. |
I'm not sure how we could do that, since that Bold only applies to a part of the heading, not all of it (unless we select it). |
I'm not sure, actually. The bold button is for the strong pronunciation in the semantic sense, the other is purely visual and, to Maggie's point, at the block level. |
Yeah, good points — that all makes sense. In any case, this PR works for me when I set a By comparison, I do see it in the Navigation block's panel: Does that control need to be added separately? |
I think it may be intentionally limited to global styles and the navigation block for now, as the UI quickly gets clunky with current controls, pending the progressive system outlined in #27331 |
I actually looked into this and it's a bug. It only appears if support for font styles is also applied, I asked about it on slack and @jorgefilipecosta said he would look into it (I didn't make an issue about it though) |
I'll take your word for it. Cc also @aaronrobertshaw as I think he worked on it. |
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.
Ok great, thanks for checking. In that case, this PR should be good to go. Thanks for updating the docs too! ⭐
I think we should not merge this PR before #27555 is merged otherwise we will have a complex back-compatibility issue. |
There were concerns regarding the UI getting cluttered as mentioned, so it was suggested that this only be opted into by dynamic blocks, starting with the navigation block as it was required to implement some new block pattern designs. The same reasoning was behind it not initially being added to the global styles. I can't recall off the top of my head who took care of that. @jorgefilipecosta has already fixed the global styles so weight and style can work independently in #27659. Thanks Jorge! |
#27555 was merged, so from the technical point of view, I don't have any blocker to merging this PR. Some places run the Gutenberg plugin in production. Merging this PR makes us need to support the current markup produced for font-weight forever, so we don't break the styles of websites currently running Gutenberg plugin in production. Or the places that run Gutenberg in production will need to have their own back-compatibility code. For reference this is the markup we produce:
Given that markup is not really "experimental" and we (or the places that run production plugin) must support it, I would like to have more thoughts on this. @mtias, @youknowriad Do you think it is ok to merge this PR and add font-weight support to additional blocks (heading in this case), or would you prefer if we wait a little bit more? |
I'd like this one to be customizable globally by the user / theme but not present in the typography panel for headings initially until we bring some of the design updates from #27331. |
@MaggieCabrera Do you have time to refresh this PR? The merge conflicts need to be solved. |
78f01d9
to
21d941f
Compare
@carolinan sorry for the wait, I just got to it, now it's rebased Since I first started this PR the docs have changed quite a bit and I'm not sure if there's a need to mention this change on them anymore (feel free to correct me though). |
Description
This PR is created to partially address #22641 It enables experimental font-weight support for heading blocks.
How has this been tested?
With TT1 blocks, adding this in experimental-theme.json:
Screenshots
Frontend:
Editor:
Checklist: