Skip to content
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

Site Title block conflicts between typography and link styles #65169

Closed
richtabor opened this issue Sep 9, 2024 · 6 comments · Fixed by #65307
Closed

Site Title block conflicts between typography and link styles #65169

richtabor opened this issue Sep 9, 2024 · 6 comments · Fixed by #65307
Assignees
Labels
[Block] Heading Affects the Headings Block [Block] Site Title Affects the Site Title Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@richtabor
Copy link
Member

#64911 resolved the application of typography with blocks that have an a as a child, like the core/site-title block.

In the case of the core/site-title block, any heading styles applied via theme.json will still effect the block's heading element.

To replicate, in blocks:

"core/site-title": {
                "elements": {
                    "link": {
                        "typography": {
                            "textDecoration": "none"
                        }
                    }
                },
                "typography": {
                    "fontSize": "var(--wp--preset--font-size--medium)",
                    "fontWeight": "550"
                }
            }

and in elements:

"h1": {
                "typography": {
                    "fontSize": "var(--wp--preset--font-size--xx-large)",
                    "lineHeight": "1"
                }
            },

CleanShot 2024-09-09 at 16 26 25

Originally reported #64911 (comment).

@richtabor richtabor added [Type] Bug An existing feature does not function as intended [Block] Heading Affects the Headings Block labels Sep 9, 2024
@richtabor richtabor moved this to 📥 Todo in WordPress 6.7 Editor Tasks Sep 9, 2024
@vcanales vcanales self-assigned this Sep 9, 2024
@mtias mtias added the [Block] Site Title Affects the Site Title Block label Sep 9, 2024
@aaronrobertshaw
Copy link
Contributor

I've had a play around with this and it does seem that #64911 just flipped the issue around from the inner link element being a problem to, the wrapping heading element.

It's a tricky problem to solve so I'm just spitballing here but did we consider simply treating these title blocks as "headings" only? Whether they contain a link or not, they're a heading. Of course, you could flip this logic around but the block names themselves hint that they are title/headings, the link behaviour is secondary.

If these blocks were treated only as headings, the inner link element could just be forced to inherit the appropriate typography styles. There'd be no double-dipping on line-height/font-size etc.

FWIW the current situation where global link element styles are not preventing block-type styles from applying is better than the previous situation.

So I guess my questions are:

  • How common is it to not style Site/Post Title blocks globally and want them to still pick up global link element styles?
  • Given it is common to globally style both links and headings, is it more likely to want the Site/Post Titles to lean into the global heading element styles?

With all that said, could we just enforce the inheritance of typography styles on a title block's inner link element? A style in the block library with 0-1-0 specificity would counter global element styles

The only other left field option I can think of would be to allow some form of conditional styles via Global Styles (e.g. reset the block's wrapper's typography styles when the block type styles are present). Global Styles are already gnarly enough that this sounds like a lot of added complexity for little gain.

Happy to be educated on the reasoning behind the latest approach in #64911.

The extra height illustrated in this issue does seem to be the sort of thing that would mess with layouts and be very frustrating for the user to address. Much like previously not being able to override global element styles 😅

@andrewserong
Copy link
Contributor

With all that said, could we just enforce the inheritance of typography styles on a title block's inner link element? A style in the block library with 0-1-0 specificity would counter global element styles

That's sounding compelling to me. I think it would resolve some of the conceptual complexity here, especially since folks select the heading level of these blocks, so I'd think their styling should always be in relation to that.

How common is it to not style Site/Post Title blocks globally and want them to still pick up global link element styles?

Personally I doubt it'd be all that common. Typically the Site Title isn't used in more than a couple of places (e.g. header and footer). The main case I can think of for post titles picking up element styles might be within the body of a pattern, but even then, they're still using headings...

At the very least this sounds worth trying out in a PR to me!

@aaronrobertshaw
Copy link
Contributor

If @richtabor is on board, I can throw up a PR in the next few days. Then everyone can have a play with it and make a final call.

@vcanales
Copy link
Member

I'm trying to understand this: What we are trying to achieve is that styles applied to Site/Post Title blocks should take precedence over styles applied to their children. Am I correct?

If so, I agree with @aaronrobertshaw that treating Site and Post Titles as headings, regardless of the presence of links, is a sensible approach. It would make sense in a few ways:

  • Semantically: Titles are typically headings and should follow heading structure.
  • Logically: It simplifies the styling hierarchy, ensuring consistent application of typography styles.
  • Structurally: The heading level is already a key feature of these blocks, and aligning the link behavior with that heading logic would prevent double-application of font-size and line-height properties, which is causing layout issues.

@aaronrobertshaw With all that said, could we just enforce the inheritance of typography styles on a title block's inner link element? A style in the block library with 0-1-0 specificity would counter global element styles

I think that makes the most sense, especially if we move towards treating the title block as a Heading. Currently, AFAIK, headings don't support links within them, so forcing the link to take the block's styles wouldn't conflict with anything in existence.

That said, we should be mindful of backward compatibility; could theme builders be relying on the current behavior? This quirk could be a feature for some, although I haven't found specific examples.

@aaronrobertshaw
Copy link
Contributor

I have a tentative fix up (#65307) for this issue and a second one I found after #64911.

What we are trying to achieve is that styles applied to Site/Post Title blocks should take precedence over styles applied to their children. Am I correct?

That's my understanding.

I believe the intent of #64911 was to ensure block type styles override global element styles. The catch coming from that solution is that we have three (or more) sets of styles but they would target different elements.

That is, global heading element styles would target the block's wrapper, while global link element styles would target the inner element, then finally the global styles for the block type would target one or the other. Post #64911 its the inner link element, before that it was the wrapper.

As discussed, the combination of both causes layout issues when font size and line height are applied to both elements.

It would make sense in a few ways:

I agree with each of those points. Thanks for listing them out.

That said, we should be mindful of backward compatibility; could theme builders be relying on the current behavior? This quirk could be a feature for some, although I haven't found specific examples.

This is my only concern as well. Especially after the 6.6 specificity issues.

@richtabor
Copy link
Member Author

If @richtabor is on board, I can throw up a PR in the next few days.

Sure, whatever works :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Heading Affects the Headings Block [Block] Site Title Affects the Site Title Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants