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

Update theme styles for the code block #36282

Merged
merged 1 commit into from
Nov 12, 2021
Merged

Conversation

ndiego
Copy link
Member

@ndiego ndiego commented Nov 6, 2021

Description

The theme.json settings for the code block target .wp-block-code > code not .wp-block-code. Therefore, there is no way to override the theme.css styles using theme.json. This update resolves that.

How has this been tested?

  1. Yes, install the new Twenty Twenty-Two theme and the latest version of Gutenberg.
  2. Add a code block with some content. Preview, and you will see the code outlined in a thin grey border.
  3. In theme.json, add a background color and change the border color on the code block.
  4. Preview and you will see the theme.json styles applied to .wp-block-code > code, but the default theme.css styles remain applied to .wp-block-code.
  5. Apply the proposed changed.
  6. Preview again and now the theme.json styles now override the theme.css styles.

Screenshots

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).

The theme.json settings for the code block target `.wp-block-code > code` not `.wp-block-code`. Therefore, there is no way to override the theme.css styles using theme.json. This update resolved that.
@gziolo gziolo added [Block] Code Affects the Code Block CSS Styling Related to editor and front end styles, CSS-specific issues. labels Nov 7, 2021
@bgardner
Copy link

I would love to see this approved and merged as it will allow me to remove CSS that is currently in my theme to unnecessarily override.

@ndiego
Copy link
Member Author

ndiego commented Nov 12, 2021

@jasmussen sorry for the ping, but can I get your eyes on this one as well? It's another theme.json related issue.

@jasmussen jasmussen self-requested a review November 12, 2021 11:56
Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works fine for me, and seems like a good solution. I didn't test with theme.json overrides, so please do verify it works as intended, ideally with another quick test of the PR. But it looks good to me. Thank you!

@ndiego
Copy link
Member Author

ndiego commented Nov 12, 2021

Just ran another test with custom styles in theme.json and TT2 and my own FSE theme. All works as expected. In classic themes, the code block styling is the same as in trunk. 🙌

@jasmussen jasmussen merged commit f2cbf5f into WordPress:trunk Nov 12, 2021
@github-actions github-actions bot added this to the Gutenberg 12.0 milestone Nov 12, 2021
@paaljoachim paaljoachim added [Type] Bug An existing feature does not function as intended Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Nov 17, 2021
@ndiego
Copy link
Member Author

ndiego commented Nov 18, 2021

@jasmussen I believe this was automatically set for the Gutenberg 12.0 milestone. Now that Beta 1 is being pushed back further, do you think we can backport this to WP Beta? It's such a simple fix.

@ndiego ndiego deleted the patch-4 branch November 18, 2021 13:15
@jasmussen
Copy link
Contributor

I think so long as it has the "backport to wp" label, it will be up to the release leads to decide on/make sure it lands in 5.9. Given it's a bugfix, I don't personally see it as problematic, but I'll defer to those making the release!

@noisysocks noisysocks removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 22, 2021
noisysocks pushed a commit that referenced this pull request Nov 22, 2021
The theme.json settings for the code block target `.wp-block-code > code` not `.wp-block-code`. Therefore, there is no way to override the theme.css styles using theme.json. This update resolved that.
@carolinan
Copy link
Contributor

@ndiego This PR changed the text color to be hard coded to color: $gray-900; which means that if the theme has a dark background, we are forced to override the text color of the block.
Is this text color needed? Maybe it can be removed?
(I feel like we have discussed this color already 🤔 )

@ndiego
Copy link
Member Author

ndiego commented Jan 5, 2022

@carolinan that's a good question. That color was already there prior to this PR. The PR just corrected the specificity of the styles so they could be manipulated via theme.json. In many of the themes I am working on, we have customized the code block completely. Here's an example...

"core/code": {
	"border": {
		"color": "var(--wp--preset--color--black)",
		"radius": "0"
	},
	"color": {
		"background": "var(--wp--preset--color--black)",
		"text": "var(--wp--preset--color--white)"
	},
	"spacing": {
		"margin": {
			"bottom": "30px"
		},
		"padding": {
			"top": "25px",
			"right": "30px",
			"bottom": "25px",
			"left": "30px"
		}
	},
	"typography": {
		"fontFamily": "var(--wp--preset--font-family--monospace)",
		"fontSize": "var(--wp--preset--font-size--small)"
	}
},

Personally, I would like there to be no set font color on the code block, but that might be a larger questions about WP style defaults.

@jasmussen
Copy link
Contributor

Followed up on the text color in #37816.

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 CSS Styling Related to editor and front end styles, CSS-specific issues. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants