Site Title, Post Title: Fix typography for blocks with a children
#64911
Conversation
|
Size Change: +2.15 kB (+0.12%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
| @@ -3,6 +3,29 @@ | |||
| // This block has customizable padding, border-box makes that more predictable. | |||
| box-sizing: border-box; | |||
|
|
|||
| &[style*="font-weight"] :where(a) { | |||
There was a problem hiding this comment.
If we add more styling options we need to create another selector following the same logic. Plus these selectors don't look ideal and we are sending many bytes over the wise but it seems a fair trade-off.
There was a problem hiding this comment.
Thank you for working on this @rafaelgallani 👍
This solution works and personally, I'm ok with it, it is not ideal but there is not ideal solution in this case.
I would love a second option on this one to know if we can merge it. Any thoughts on this @noisysocks, @aaronrobertshaw, or @oandregal ?
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Noting that I tested this PR and confirmed it resolves #41725 |
There was a problem hiding this comment.
Nice work here!
This solution works and personally, I'm ok with it, it is not ideal but there is not ideal solution in this case.
I would love a second option on this one to know if we can merge it.
If we add more styling options we need to create another selector following the same logic. Plus these selectors don't look ideal and we are sending many bytes over the wise but it seems a fair trade-off.
Good points. TL;DR: On balance I think I lean toward the idea of including the additional CSS here to factor in the link state, as it's a pretty common one.
Longer thoughts:
That said, these are both dynamic blocks where their markup is generated at render time. This means that it could be technically possible to inject the desired inline styles into the a tag at render time if need be?
The main problem with doing that is that it would add further complexity to the PHP side of things. For example if we intercepted the attributes from the call to get_block_wrapper_attributes we could (theoretically) either grab the styles we need, or check for the presence of certain inline styles and then dynamically output the inherit rules as needed. That gets pretty messy once other styles like padding are involved, as we wouldn't want them to be duplicated on the a element. (Example wrapper attributes: style="background-color:#f7a1a1; font-size:clamp(0.984rem, 0.984rem + ((1vw - 0.2rem) * 0.86), 1.5rem);line-height:1.1; border-color:#d27373;border-width:15px; padding-top:15px;padding-bottom:15px;padding-left:15px;padding-right:15px;" class="wp-block-post-title has-background has-border-color")
So, a second option (still PHP) could be to skip serialization for the Typography support, and call the relevant Typography and/or style engine functions from within the PHP version of these blocks, and then inject the results into the a tag instead of the wrapper. This again has the problem of added complexity, and would need to be duplicated in JS, too.
All up, this leans me to think that the approach in this PR is likely the simplest as it only adds some additional rules. My vote would be to go with the current state of this PR, but I mostly wanted to share the thoughts above as a potential alternative in case anyone else thought it worth pursuing.
Testing well for me:
✅ Font-weight at the individual block level overrides anchor styles
✅ Font-family at the individual block level overrides anchor styles
✅ Font-size at the individual block level overrides anchor styles
✅ Line-height at the individual block level overrides anchor styles
✅ Font-style at the individual block level overrides anchor styles
✅ Letter-spacing at the individual block level works as expected (I don't know if you can set it for the anchor in global styles, so I wasn't too sure if the extra rule is needed, but it's working well in testing)
✅ Text-decoration at the individual block level works as expected. I noticed that it didn't always work reliably in global styles, but I don't think that's really related to this PR and not a blocker for proceeding.
LGTM! 🎉
It might be worth getting a second approving PR due to the discussion surrounding the CSS rules, but overall I'm in favour of this approach 👍
andrewserong
added
[Feature] Typography
|
@andrewserong Wow, that's a very detailed overview. Thank you so much for that!
I like this idea, but, yeah, I think we would need to duplicate this in JS. Also, I just wanted to confirm before merging this one - it's been a while since my last contribution to Gutenberg: since we got a review from a |
Yes, I think we're good to go. @jorgefilipecosta is also a
Nope! The next RC will be cut from This looks good to merge, to me! |
Yeah. I haven't noticed that he is also a team member, sorry 😅 Alright then. Merging this one. Thanks! |
rafaelgallani
deleted the
fix/typography-for-blocks-with-anchor-children
branch
andrewserong
added
the
Backport to Gutenberg RC
|
Noting an issue here: From this theme.json: and I'm getting this:
Because the elements.h1 styles are applied to the |
…64911) * Apply styles for element + `a` child when present * Inherit styles from parent element when present * Add missing `-font-family` attribute selector
* Core Data: Fix the 'query._fields' property check inside 'getEntityRecord' resolver (#65079) Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> * Site Title, Post Title: Fix typography for blocks with `a` children (#64911) * Apply styles for element + `a` child when present * Inherit styles from parent element when present * Add missing `-font-family` attribute selector * Interactivity API: Prevent calling `proxifyContext` over an already-proxified context inside the `wp-context` directive (#65090) * Do not store the proxified context inside `currentValue.current`. * Update changelog Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> --------- Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: Rafael Gallani <galani.rafael@gmail.com> Co-authored-by: David Arenas <david.arenas@automattic.com> Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org>
| "style": "wp-block-site-title" | ||
| "style": "wp-block-site-title", | ||
| "selectors": { | ||
| "typography": ".wp-block-site-title > span, .wp-block-site-title > a" |
There was a problem hiding this comment.
This introduces a regression where the editor and frontend apply different styles. The Site Title block doesn't render an inner span on the frontend.
As proposed over on #65169, I have some ideas to address this and the other issue flagged in #64911 (comment).
I'll get a draft PR up soon.
Gutenberg 19.2 broke `theme.json` styles on the Site Title block by changing the selector to specifically target the link rather and not the block itself. This change reverts that selector to its original state. See: WordPress/gutenberg#64911
kevin940726
removed
the
Backport to Gutenberg RC
Related to:
What?
This PR fixes the typography properties for Site Title and Post Title blocks, enabling their specific styles to be respected/take precedence over more generic styles.
Why?
Individual typography styles for the block are not respected if it acts as a link (if the "Make title a link" setting is enabled). Styles-defined settings are being used for these blocks. The text decoration, font family, and a few other attributes (described below) can't be set individually for these blocks, then.
How?
selectorssetting for thetypographywithin theblock.jsonof the blocks needs to be updated - as in the PR - to ensure anytheme.jsonstyle change also affects the inneraelement;achildren, if present; otherwise, use whatever gets defined in Styles/theme.json.Testing Instructions
theme.jsonstyles:emptytheme;core/site-titleelement:theme.jsonis being correctly displayed;Any other typography property can also be used (font was just used as an example):
Screenshots or screencast
Before
before.mov
After
after.mov