WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.49k stars 4.19k forks source link

Site Title block conflicts between typography and link styles #65169

Closed richtabor closed 1 month ago

richtabor commented 1 month ago

https://github.com/WordPress/gutenberg/pull/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 https://github.com/WordPress/gutenberg/pull/64911#issuecomment-2339018022.

aaronrobertshaw commented 1 month ago

I've had a play around with this and it does seem that https://github.com/WordPress/gutenberg/pull/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:

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 https://github.com/WordPress/gutenberg/pull/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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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:

@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 commented 1 month ago

I have a tentative fix up (https://github.com/WordPress/gutenberg/pull/65307) for this issue and a second one I found after https://github.com/WordPress/gutenberg/pull/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 commented 1 month ago

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

Sure, whatever works :)