-
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
Add default-spacing-sizes
and default-font-sizes
options for classic themes
#62252
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/compat/wordpress-6.6/block-editor.php ❔ lib/class-wp-theme-json-gutenberg.php ❔ lib/class-wp-theme-json-resolver-gutenberg.php ❔ lib/load.php ❔ phpunit/class-wp-theme-json-test.php |
lib/block-editor.php
Outdated
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.
Copied from core. See https://github.com/WordPress/wordpress-develop/pull/6616/files#diff-edfc853e670c7bfc5a838dc74f75ba5d8b95cb4adbddf0644d9227bcb692e344 for a better diff.
Flaky tests detected in 2dcd295. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9369758211
|
I am trying to follow the logic. Reading this it sounds to me like the default font sizes will be disabled by default? So the font sizes will not be selectable in the editor unless the theme is updated to set it to true? |
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.
I believe this is testing nicely for me. Just double-checking that when I test with Twenty Twenty theme, the after here is the intention as that theme currently sets 'editor-font-sizes'
with its own values for small
, normal
, large
, and larger
sizes. So prior to this PR, using the unexpected extra sizes, we'd get the following mismatched sizes:
Before
But with this PR applied (and no other changes to the theme), then we get the correct sizes that are balanced to the theme:
After
If I then go in and add add_theme_support( 'default-font-sizes' );
to the theme, then I can get to the Before
image above again. I haven't had a close look at the code yet, but behaviour-wise, this is feeling good to me so far 👍
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 is looking good to me! All testing well:
✅ If the theme sets no editor font sizes and doesn't opt into default font sizes, then default font sizes are available.
✅ If the theme sets editor font sizes but not default font sizes, no default font sizes are available.
✅ If the theme sets both, default font sizes will override.
✅ Same as above for spacing sizes.
I agree the logic might look a bit tricky to follow, but in practice, I think this strikes the right balance, and I haven't been able to fault it while testing, and it resolves a current issue for classic themes such as Twenty Twenty.
This LGTM 🚀, but do feel free to seek a second opinion if there is any doubt about the feature.
The only thing to change from my perspective is a typo in the docs.
I am not able to reproduce the missing font sizes in classic themes (+ the JavaScript error) with Gutenberg active, with or without this PR; so I can't test this: I want to clarify that I don't mean to block the approval of this PR. |
See #6616. See also the original Gutenberg PRs: * WordPress/gutenberg#58409 * WordPress/gutenberg#61328 * WordPress/gutenberg#61842 * WordPress/gutenberg#62199 * WordPress/gutenberg#62252 Fixes #61282. Props ajlende, talldanwp, ramonopoly, ellatrix. git-svn-id: https://develop.svn.wordpress.org/trunk@58328 602fd350-edb4-49c9-b593-d223f7449a82
See WordPress/wordpress-develop#6616. See also the original Gutenberg PRs: * WordPress/gutenberg#58409 * WordPress/gutenberg#61328 * WordPress/gutenberg#61842 * WordPress/gutenberg#62199 * WordPress/gutenberg#62252 Fixes #61282. Props ajlende, talldanwp, ramonopoly, ellatrix. Built from https://develop.svn.wordpress.org/trunk@58328 git-svn-id: http://core.svn.wordpress.org/trunk@57785 1a063a9b-81f0-0310-95a4-ce76da25c4cd
See WordPress/wordpress-develop#6616. See also the original Gutenberg PRs: * WordPress/gutenberg#58409 * WordPress/gutenberg#61328 * WordPress/gutenberg#61842 * WordPress/gutenberg#62199 * WordPress/gutenberg#62252 Fixes #61282. Props ajlende, talldanwp, ramonopoly, ellatrix. Built from https://develop.svn.wordpress.org/trunk@58328 git-svn-id: https://core.svn.wordpress.org/trunk@57785 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…dd/on-async-directives * 'trunk' of https://github.com/WordPress/gutenberg: (72 commits) Top toolbar: fix half a pixel artifacting of the bottom border. (#62225) Fix UI order for theme.json spacing sizes (#62199) Chore: Simplify a padding style on global styles. (#62291) Editor: Avoid remounts of `DocumentBar` (#62214) Add `default-spacing-sizes` and `default-font-sizes` options for classic themes (#62252) Editor: Cleanup styles and classnames (#62237) Scripts: Pin the @wordpress/scripts version to a version supported by WP 6.5 (#62234) Documentation: Better changelogs for the JSX transform upgrade (#62265) Chore: Simplify a padding style on dataviews. (#62276) MediaUpload: Remove dialog markup on close (#62168) Tabs: Prevent accidental overflow in indicator (#61979) Make edit site pagination buttons accessibly disabled. (#62267) Fix: Remove unused code from dataviews styles. (#62275) Re-enable React StrictMode (#61943) Inserter: Return the same items when the state and parameters don't change (#62263) Update instances of text-wrap: pretty to fall back to balance (#62233) Update: Slotfill documentation samples (links, code, and rephrase). (#62271) Try: Fix mover positioning. (#62226) [Mobile] - Image corrector - Check the path extension is a valid one (#62190) Update: Block styles documentation. ...
…sic themes (#62252) Co-authored-by: ajlende <ajlende@git.wordpress.org> Co-authored-by: carolinan <poena@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
…sic themes (#62252) Co-authored-by: ajlende <ajlende@git.wordpress.org> Co-authored-by: carolinan <poena@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
…sic themes (WordPress#62252) Co-authored-by: ajlende <ajlende@git.wordpress.org> Co-authored-by: carolinan <poena@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
This was cherry-picked to the wp/6.6 branch. |
What?
Fixes an issue where defaults were generated in classic themes when they should not have been.
Why?
Bug fix for #58409 and #61842.
How?
Set
defaultFontSizes
anddefaultSpacingSizes
tofalse
for classic themes when they havefontSizes
orspacingSizes
defined, respectively.Additionally, since it was just a couple extra lines, I added the
editor-spacing-sizes
,default-font-sizes
, anddefault-spacing-sizes
theme supports for parity with other presets.Testing Instructions
editor-font-sizes
that match the default slugs. (The default slugs are:small
,medium
,large
,x-large
, andxx-large
.)default-font-sizes
and see that the default font sizes are generated and they override the theme font sizes.The same should work for
editor-spacing-sizes
anddefault-spacing-sizes
.Testing Instructions for Keyboard
Screenshots or screencast