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

Code block: Font size does not work for TwentyTwentyOne #27827

Closed
mkaz opened this issue Dec 19, 2020 · 10 comments · Fixed by #27862
Closed

Code block: Font size does not work for TwentyTwentyOne #27827

mkaz opened this issue Dec 19, 2020 · 10 comments · Fixed by #27862
Labels
[Block] Code Affects the Code Block

Comments

@mkaz
Copy link
Member

mkaz commented Dec 19, 2020

Describe the bug

Setting the font size for the code block will add the class has-large-font-size to the outer pre block but does not set the class for the inner code block. So for the theme TwentyTwentyOne the code block size appears to not work.

Markup ends up being:

<pre class="wp-block-code has-large-font-size">
<code> // Foo </code>
</pre>

The CSS from the theme:

.wp-block-code code {
    font-size: var(--global--font-size-xs);
    white-space: pre;
    overflow-x: auto;
    display: block;
}

Is it only a theme issue? If the class is attached to the code tag then it would work too.

@mkaz mkaz added the [Block] Code Affects the Code Block label Dec 19, 2020
@carolinan
Copy link
Contributor

carolinan commented Dec 19, 2020

No, Twenty Twenty-One does not have built in support for features created after the theme was released.
🤔 It also needs to be backwards compatible for sites where Gutenberg is not installed and for older versions of WP.

Does it work if the font-size: var(--global--font-size-xs); is removed?

@mkaz
Copy link
Member Author

mkaz commented Dec 19, 2020

@carolinan Yes, if the class is removed it works as expected.

@nosolosw How should a theme specify a default value, that still allows for user override? (and works with/without Gutenberg plugin)

@oandregal
Copy link
Member

@mkaz this is a regression and I've got a fix for the Gutenberg side at #27862

@mkaz
Copy link
Member Author

mkaz commented Dec 22, 2020

@nosolosw Thanks for the quick fix, I left a review and comment on the PR, but I don't think it fixes it because the theme still hard codes the font size for the code block.

I think there's still an issue on allowing the theme to provide a default value and then the user to override.

@oandregal
Copy link
Member

@nosolosw Thanks for the quick fix, I left a review and comment on the PR, but I don't think it fixes it because the theme still hard codes the font size for the code block.

I think there's still an issue on allowing the theme to provide a default value and then the user to override.

My thinking is that this is one of those cases where the theme needs to account for user styles. For example, the issue you refer here with TwentyTwentyOne could be fixed by only applying the font-size declaration to the block if it doesn't have the font-styles applied, something like :not(.font-size-class-here).

@carolinan
Copy link
Contributor

carolinan commented Dec 23, 2020

It is not realistic to expect theme and plugin developers to keep up with the frequent changes to the editor.
On the WordPress Trac there are 255 open issues related to bundled themes. Some are related to changes in the editor, like:
https://core.trac.wordpress.org/ticket/52009

They are not being fixed because it is not effective use of the contributors time.
It would be more effective to wait until a block has all its global styles and then update it, rather than update a default themes CSS every two to four weeks.

This is also bothersome when the theme adjusts for a change that is then reverted and the theme needs to be updated again.
(search block with button only?)

We are going to see this problem for a long transition period, even after global styles are out of the experimental phase, and, there is also no contributor that keeps track of all changes to Gutenberg and open trac tickets saying that something needs to be updated in the bundled themes.
Which role in the organization would need to take that on? The person who commits the change to the editor? The bundled themes component maintainer? With the number of themes, this is not something that should rest on one or two people.

@mkaz
Copy link
Member Author

mkaz commented Dec 23, 2020

@carolinan Thank you, you raise many good questions, many of which still need to be worked out for sure. The theme_support function is intended to address this, by a theme specifying what it does or does not support, it may need improvement and finer grain controls to address all needs.

In this case I think there is a slightly different issue at hand. The TwentyTwentyOne theme is explicitly setting a font size for .wp-block-code code because I assume they want to specify a default style that looks best for the theme. This is perfectly reasonable, most themes will want to do this for certain blocks, the code block is just one example here. However, the user should also be allowed to override that value by selecting a font size in the editor, this is not working in this case, because the theme specified a size.

So the open question, how should a theme specify a default style that allows for a block to override on a global and/or an individual post or block basis. Also, who should get the final say on how something is styled, the theme or the user/editor?

For a large publication, I can see them wanting the theme to get the final say, while an individual personal blog maybe the user. Either way we need to figure out how to design the system in a way that works for both, and then create themes (and document how) that allow both.

In this case, the theme specified to use var(--global--font-size-xs); and adding has-large-font on the parent block couldn't override. What would be the recommended way to allow for a theme default with user override?

Plus please note, as you stated earlier this is an experimental feature at an early stage - this bug report is intended to highlight an issue and start a discussion, it was not intended as a high priority bug that all themes must fix right now. I think it only occurs if you have the plugin enabled, so the impact is still low.

@carolinan
Copy link
Contributor

carolinan commented Dec 23, 2020

I am not debating whether the theme should support the feature.
But you can't expect the theme to support the font size feature because it was created after the theme was released.

You would need to create a Trac ticket and with some effort support would be added by the time the font size feature for the code block is added to Core.
Together with:
Border radius
Color
Spacing

@oandregal
Copy link
Member

This was closed automatically by #27862 but happy to re-open and help fix whatever issue remains in Gutenberg.

So the open question, how should a theme specify a default style that allows for a block to override on a global and/or an individual post or block basis. Also, who should get the final say on how something is styled, the theme or the user/editor?

If the user has the tools to change a property, that change has preference over the theme's or core's style preferences ― this is for themes with or without theme.json support and my understanding is that it has been like that since the beginning.

As to how to make sure theme & user styles play well, my view is that it's a delicate CSS dance and this needs to be adjusted over time as the blocks get support for new style properties. Personally, I think it's ok that themes are not updated bi-weekly to account for all new changes in the Gutenberg plugin, the struggle to keep up with everything is real for many of us. From the Gutenberg point of view, I'm not sure what else is to be done other than communicating what's new and adding dev notes when these kinds of things are released in a WordPress version. Happy to consider any other alternatives.

A related topic is when a user should be presented with design tools and when not and whether this is something themes should control. So far, the mechanism Gutenberg had was add_theme_support, which was quite coarse-grained (enable/disable for the whole editor). With the introduction of theme.json themes will gain a more fine-grained ability to control the editor, so it'd be possible to control whether something is visible or not at a block level.

@carolinan
Copy link
Contributor

Trac ticket created
https://core.trac.wordpress.org/ticket/52431

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Code Affects the Code Block
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants