-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Add support for padding, color & border styles #27582
Conversation
As expressed by TT1's CODE block styles: https://github.com/WordPress/twentytwentyone/blob/trunk/assets/sass/05-blocks/code/_style.scss The default padding value of the CODE block is now the default spacing unit. WordPress/gutenberg#27582 is necessary for this enhancement to work.
358ef81
to
8e4767f
Compare
8e4767f
to
91a63f1
Compare
As expressed by TT1's CODE block styles: https://github.com/WordPress/twentytwentyone/blob/trunk/assets/sass/05-blocks/code/_style.scss The default padding value of the CODE block is now the default spacing unit. WordPress/gutenberg#27582 is necessary for this enhancement to work.
I notice that the theme.scss contains default padding values. Do we have a way to set these without CSS? |
84cb149
to
aeaa472
Compare
When I set the border radius to 0, it picks up the 4px border radius from the CSS. I think this is why we need to have these default values move to the JSON when they are default. I opened an issue for this: #27796 |
Looked at another way, the value of '0' (in this context especially) doesn't mean "unset" even though that is what is implied. Another way to address this would be to have this radius control differentiate between '0' and 'unset/reset' |
There is a lot more discussion on this on #27579 |
fc7a9b6
to
6a4851a
Compare
Should the code block even have these controls? |
I agree that not all blocks need all options available to the user, but why not to the theme developer? If we are building tools that let the theme dev control the block in such a way by using theme.json, why are we not giving them the tools just because we may not want the end-user to have access to them as well? |
My apologies, I thought that the plan was to eventually expose all these things to the user. Are these only dev-facing with no plans to expose the related controls to users? |
Sorry I think I came up a bit strong there. This PR does surface the options to the users, I just wanted to express that it shouldn't have to be that way every single time, and this case in particular is a good example where the user shouldn't see the controls but the themer should have the ability to modify them on the theme.json file |
Yeah the problem is that at the moment making it possible to modify these things in the theme.json also enables the user controls. This needs to be decoupled. |
I suggest exposing text color, background, and font family. Being able to set colors can make a code block stand out more, and this can be combined with the inline text color for highlighting parts of a code. |
Code blocks should normally use a code/monospace font-family. If the user is able to select a normal serif/sans-serif font-family, then it will simply be used as a paragraph block with weird styling... It is semantically a code block so it should use a code font-family. I fear that if we add too many options there (including font-family) it will be abused for its appearance instead of its semantic value. |
If I as a User, or I as a Theme Author want to style code blocks to be in comic-sans or cursive I'm not sure why that should be something dictated by Gutenberg. It can still be semantic, expressing "code" using whatever stylistic opinion. If I can't do that (or the like) via configuration then we chase that into CSS. Is it our understanding then that those decisions SHOULD be expressed in CSS instead of configuration? (If it doesn't align with Gutenberg opinion then CSS is the avenue?) |
They should be expressed in the config file and in CSS, but not necessarily exposed to users in a prominent fashion like in a paragraph block or a heading... From the MDN docs:
If a designer wants to use comic-sans then they can write CSS to do that, but in that case the code block no longer represents "code" visually. |
This PR looks still to be valid, ignoring the font-family discussion which is not included in this PR in the first place, so that can be tabled and discussed elsewhere. @pbking If you can rebase and update, we can move this forward with the border, spacing, and color additions. |
Added support for "padding" controls and styling of CODE block. Added support for "border" styling of CODE block (including color, style, width, radius. Added color support to code block
6a4851a
to
01513a2
Compare
Rebased as requested. |
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.
This tests well and looks good. 👍
I added top-bottom margin support that is another useful feature to give additional space around a code block.
This is needed to resolve WordPress/theme-experiments#94 to support TwentyTwentyOne-blocks theme.
Description
Added spacing.padding support option to core/code block's block.json to signal support of customizing the padding values of the block allowing themes and users to customize the padding values of CODE blocks.
Added color supports options to core/code block's JSON to allow support of customizing color values in theme.json and by users.
Added border supports options to core/code blocks's JSON to allow theme.json support for customizing that block's border
Updated documentation to reflect that the CODE block includes padding controls.
Added style.border controls to CODE block's block.json to signal support of border settings
Added style.color to CODE block's block.json to signal support of color settings
How has this been tested?
Resulting in a wp-block-code class that includes the padding variables as expected
Types of changes
New feature
Checklist: