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: Add support for padding, color & border styles #27582

Merged
merged 2 commits into from
Oct 26, 2021

Conversation

pbking
Copy link
Contributor

@pbking pbking commented Dec 8, 2020

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?

		"core/code": {
			"typography": {
				"fontSize": "var(--wp--preset--font-size--extra-small)"
			},
			"spacing": {
				"padding": {
					"top": "var(--wp--custom--spacing--unit)",
					"bottom": "var(--wp--custom--spacing--unit)",
					"left": "var(--wp--custom--spacing--unit)",
					"right": "var(--wp--custom--spacing--unit)"
				}
			},
			"border": {
				"radius": "0px",
				"color": "var(--wp--preset--color--dark-gray)",
				"style": "solid",
				"width": "1.5px"
			}
	
		}

Resulting in a wp-block-code class that includes the padding variables as expected
image

  • Add and select a CODE block in the block editor
    • note that the SPACING panel has been added to the interface
  • Using the SPACING panel modify the padding values for the CODE block and save the page/post.
    • observe the rendered page and observe that the border properties defined in the interface are set on the block's style attribute.

image

<pre class="wp-block-code" style="padding-bottom:20px;padding-left:20px;padding-right:20px;padding-top:20px"><code>This is a code block.
&lt;div fancy="true"&gt;
{ super: "duper" } </code></pre>

Types of changes

New feature

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@pbking pbking marked this pull request as draft December 8, 2020 16:11
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Dec 8, 2020
pbking added a commit to WordPress/theme-experiments that referenced this pull request Dec 8, 2020
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.
@pbking pbking changed the title Enhance CODE block to support customizations of TT1-blocks theme. Enhance CODE block to support padding customizations Dec 8, 2020
@pbking pbking marked this pull request as ready for review December 8, 2020 21:08
pbking added a commit to WordPress/theme-experiments that referenced this pull request Dec 15, 2020
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.
@scruffian
Copy link
Contributor

I notice that the theme.scss contains default padding values. Do we have a way to set these without CSS?

@pbking pbking force-pushed the enhance/code-block branch 2 times, most recently from 84cb149 to aeaa472 Compare December 17, 2020 20:41
@pbking pbking changed the title Enhance CODE block to support padding customizations Enhance CODE block to support padding, color & border stylings Dec 17, 2020
@pbking pbking marked this pull request as draft December 17, 2020 21:40
@scruffian
Copy link
Contributor

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.

Screenshot 2020-12-18 at 08 45 04

I opened an issue for this: #27796

@pbking
Copy link
Contributor Author

pbking commented Dec 18, 2020

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'

@scruffian
Copy link
Contributor

There is a lot more discussion on this on #27579

@pbking pbking force-pushed the enhance/code-block branch 4 times, most recently from fc7a9b6 to 6a4851a Compare February 16, 2021 16:07
@pbking pbking marked this pull request as ready for review February 16, 2021 16:56
@aristath
Copy link
Member

Should the code block even have these controls?
twentytwentyone has some pretty opinionated styles, and not all of them should be exposed to users. Some things should still be in the theme's stylesheets...
We need to have a balance between functionality and noise, if we add all available options to all blocks it becomes too noisy and harder to use 🤔

@MaggieCabrera
Copy link
Contributor

We need to have a balance between functionality and noise, if we add all available options to all blocks it becomes too noisy and harder to use 🤔

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?

@aristath
Copy link
Member

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?

@MaggieCabrera
Copy link
Contributor

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

@scruffian
Copy link
Contributor

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.

@carolinan
Copy link
Contributor

I suggest exposing text color, background, and font family.
Because I think it is common for themes to set a font family for code blocks that is different from a regular paragraph, and
some users may want to change that font.

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.

@aristath
Copy link
Member

and font family.

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.

@pbking
Copy link
Contributor Author

pbking commented Feb 22, 2021

and font family.

It is semantically a code block so it should (anything could go here and I would disagree)

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?)

@carolinan
Copy link
Contributor

carolinan commented Feb 22, 2021

For example, this is an image of a system font stack and "monospace", for me these fonts are too similar and I can't distinguish the code.

Especially when the code is inline (which is why I only now noticed that the <code> does not wrap the last letter in palette).

image

Same fonts but with a code block:
image

@talldan talldan removed their request for review February 23, 2021 01:22
@aristath
Copy link
Member

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:

The HTML <pre> element represents preformatted text which is to be presented exactly as written in the HTML file. The text is typically rendered using a non-proportional ("monospace") font. Whitespace inside this element is displayed as written.

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.
The fact that we can do something doesn't mean we should, and we should at least try not to make it easy for end-users who don't necessarily understand the semantic value of a code block to break the expected behavior of the web.

@mkaz
Copy link
Member

mkaz commented Oct 14, 2021

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
@pbking
Copy link
Contributor Author

pbking commented Oct 25, 2021

Rebased as requested.

Copy link
Member

@mkaz mkaz left a 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.

@scruffian scruffian merged commit 9e2974d into WordPress:trunk Oct 26, 2021
@scruffian scruffian deleted the enhance/code-block branch October 26, 2021 12:59
@github-actions github-actions bot added this to the Gutenberg 11.9 milestone Oct 26, 2021
@andrewserong andrewserong added [Block] Code Affects the Code Block [Type] Enhancement A suggestion for improvement. labels Nov 4, 2021
@andrewserong andrewserong changed the title Enhance CODE block to support padding, color & border stylings Code block: Add support for padding, color & border styles Nov 5, 2021
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 First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TT1 Blocks: Migrate CSS for code block
7 participants