Add defaultSpacingSizes option (theme.json v3) #61842
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ajlende
added
Needs Dev Note
ajlende
requested review from
tellthemachines,
spacedmonkey,
ellatrix,
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
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/class-wp-theme-json-gutenberg.php ❔ lib/class-wp-theme-json-resolver-gutenberg.php ❔ lib/class-wp-theme-json-schema-gutenberg.php ❔ lib/theme.json ❔ phpunit/class-wp-theme-json-schema-test.php ❔ phpunit/class-wp-theme-json-test.php |
|
Size Change: +406 B (+0.02%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
ajlende
commented
May 22, 2024
| @@ -603,7 +603,6 @@ public static function get_merged_data( $origin = 'custom' ) { | |||
| $result = new WP_Theme_JSON_Gutenberg(); | |||
| $result->merge( static::get_core_data() ); | |||
| if ( 'default' === $origin ) { | |||
| $result->set_spacing_sizes(); | |||
There was a problem hiding this comment.
Moved to WP_Theme_JSON_Gutenberg->__construct() so that the generated presets for each origin are merged properly rather than always being set as default origin.
| @@ -40,6 +40,7 @@ import { | |||
| import { requiresWrapperOnCopy } from './components/writing-flow/utils'; | |||
| import { PrivateRichText } from './components/rich-text/'; | |||
| import { PrivateBlockPopover } from './components/block-popover'; | |||
| import useSpacingSizes from './components/spacing-sizes-control/hooks/use-spacing-sizes'; | |||
There was a problem hiding this comment.
The spacer block was using the list of presets to conditionally show UI elements. For the sake of not changing too many things to get this into 6.6, I just exported the same useSpacingSizes hook that the block editor package uses as a private API rather than refactoring the spacer block to not need it.
packages/block-editor/src/components/spacing-sizes-control/hooks/use-spacing-sizes.js
Outdated
Show resolved
Hide resolved
|
Flaky tests detected in 1a3d0c2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9305751937
|
|
Regarding the "Verify Core Backport Changlog", it'd be good to make a backport PR with that combines these and the previous changes, then add them as you see here: https://github.com/WordPress/gutenberg/blob/trunk/backport-changelog/readme.md |
ajlende
changed the title
ajlende
force-pushed
the
add/theme-json-default-spacing-sizes
branch
from
2bbcf50
to
e5f3860
Compare
|
Given the nature of this change, I think this "feature" should be blessed, because 6.6 is already going to ship with theme.json v3, this change should make it in too, but we need to make sure we are not breaking anything with it. /cc @priethor |
ajlende
force-pushed
the
add/theme-json-default-spacing-sizes
branch
from
d376aeb
to
433b54a
Compare
ajlende
commented
May 30, 2024
|
|
||
| return useMemo( () => { | ||
| const sizes = [ | ||
| { name: __( 'None' ), slug: '0', size: 0 }, |
There was a problem hiding this comment.
I updated a small UX oddity where the dropdown would show the number 0 when the rest of the generated presets used t-shirt sizing names.
ajlende
commented
May 31, 2024
| 'slug' => '60', | ||
| 'size' => '6rem', | ||
| ), | ||
| array( | ||
| 'name' => '5', | ||
| 'name' => 'X-Large', |
There was a problem hiding this comment.
These renames are all done in the UI instead in case the UI wants to show the numbers in the tooltips in the slider, but an actually useful name elsewhere. See #44247.
ajlende
commented
May 31, 2024
Comment on lines
-339
to
-348
| - `spacing.spacingScale`: used to generate an array of spacing preset sizes for use with padding, margin, and gap settings. | ||
| - `operator`: specifies how to calculate the steps with either `*` for multiplier, or `+` for sum. | ||
| - `increment`: the amount to increment each step by. Core by default uses a 'perfect 5th' multiplier of `1.5`. | ||
| - `steps`: the number of steps to generate in the spacing scale. The default is 7. To prevent the generation of the spacing presets, and to disable the related UI, this can be set to `0`. | ||
| - `mediumStep`: the steps in the scale are generated descending and ascending from a medium step, so this should be the size value of the medium space, without the unit. The default medium step is `1.5rem` so the mediumStep value is `1.5`. | ||
| - `unit`: the unit the scale uses, eg. `px, rem, em, %`. The default is `rem`. | ||
| - `spacing.spacingSizes`: themes can choose to include a static `spacing.spacingSizes` array of spacing preset sizes if they have a sequence of sizes that can't be generated via an increment or multiplier. | ||
| - `name`: a human readable name for the size, eg. `Small, Medium, Large`. | ||
| - `slug`: the machine readable name. In order to provide the best cross site/theme compatibility the slugs should be in the format, "10","20","30","40","50","60", with "50" representing the `Medium` size value. | ||
| - `size`: the size, including the unit, eg. `1.5rem`. It is possible to include fluid values like `clamp(2rem, 10vw, 20rem)`. |
There was a problem hiding this comment.
This isn't the documentation page where keys are defined, just the one that explains what gets generated in the stylesheet, so I removed this chunk. The actual documentation exists in the Theme Handbook which isn't part of Gutenberg source code.
ajlende
added
has dev note
9 tasks
|
This shouldn't have any effect on V2 I'm noting a few issues with the "Powder" theme for example, which registers it's own spacing values: Trunk (from this PR)CleanShot.2024-05-31.at.14.38.15.mp4Without GutenbergCleanShot.2024-05-31.at.14.39.50.mp4Notice the values and labels are not accurate in trunk. |
|
All of these are rendered, which still properly include the theme's custom slugs (which seem to be the issue). The control is applying the presets inaccurately, with missing
|
|
Wanted to note that I created an issue #62194 |
|
Responded on @bgardner's issue #62194 (comment). Thanks for raising this. 😄 |
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jun 4, 2024
Co-authored-by: ajlende <ajlende@git.wordpress.org> Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: bgardner <bgardner@git.wordpress.org> Co-authored-by: carolinan <poena@git.wordpress.org> Co-authored-by: dabowman <davidabowman@git.wordpress.org> Co-authored-by: hanneslsm <hanneslsm@git.wordpress.org>
pento
pushed a commit
to WordPress/wordpress-develop
that referenced
this pull request
Jun 4, 2024
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
markjaquith
pushed a commit
to markjaquith/WordPress
that referenced
this pull request
Jun 4, 2024
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
github-actions bot
pushed a commit
to platformsh/wordpress-performance
that referenced
this pull request
Jun 4, 2024
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
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
Co-authored-by: ajlende <ajlende@git.wordpress.org> Co-authored-by: MaggieCabrera <onemaggie@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: bgardner <bgardner@git.wordpress.org> Co-authored-by: carolinan <poena@git.wordpress.org> Co-authored-by: dabowman <davidabowman@git.wordpress.org> Co-authored-by: hanneslsm <hanneslsm@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dev Note 📝
Since #58409 is so similar, I've updated the dev note there to include these changes too. See #58409 (comment).
What?
As a follow-up to #58409, to get the migration in the same theme.json version bump.
Adds a
defaultSpacingSizesoption to disable showing the default spacing sizes.When
defaultSpacingSizes === falsethe options are removed from the UI.In theme.json v3, by default
defaultSpacingSizesistrue.In theme.json v2,
defaultSpacingSizesisn't a valid key, but is treated asfalsewhere it would be used when a theme has spacing sizes defined for backwards compatibility of generated CSS.In v3, the sizes generated with
spacingScaleare merged with the customspacingSizeswith the customspacingSizestaking precedence when it uses the same slug as one of the generated ones.Why?
Fixes #55636. See #55636 (comment) since the suggested resolution changed from the original description.
This option brings consistency to spacing sizes to match colors, gradients, duotone filters, and shadow presets.
How?
Goals are to migrate the behavior of the spacing sizes presets to be like other presets that can be disabled via
default*settings.Testing Instructions
Test various combinations of the following settings:
versionset to2or3defaultSpacingSizesset totrueorfalsespacing.spacingScaleset to{ "steps": 0 }or emptyspacingScalevsspacingSizesor bothTesting Instructions for Keyboard
N/A
Screenshots or screencast
TODO