Theme.json: Add spacing size presets #41527
Merged
Conversation
This file contains hidden or 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
|
Size Change: +28 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
glendaviesnz
added
[Feature] Themes
glendaviesnz
changed the title
glendaviesnz
requested review from
TimothyBJacobs,
ajitbohra,
ajlende,
fabiankaegy,
geriux,
juanmaguitar,
ryanwelcher and
spacedmonkey
as code owners
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
aaronrobertshaw
requested changes
Jun 8, 2022
Contributor
aaronrobertshaw
left a comment
aaronrobertshaw
left a comment
There was a problem hiding this comment.
Thanks for diving into this @glendaviesnz 👍
This almost works for me. Unfortunately, it generates invalid CSS. I've left an inline comment below where I believe the problem arises.
✅ Presets generate appropriate CSS vars
❌ Preset generated classes contain invalid CSS
✅ Presets CSS vars and classes are included in the editors
✅ Can customize, override, and extend spacing size presets via theme.json
All in all, this is pretty close despite the issue I've noted. Nice work!
andrewserong
reviewed
Jun 8, 2022
aaronrobertshaw
approved these changes
Jun 8, 2022
Contributor
aaronrobertshaw
left a comment
aaronrobertshaw
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround 👍
I can confirm that everything still works and no CSS classes are generated now.
This should be good to go pending a decision on px vs rem for defaults.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
30 tasks
added 8 commits
July 1, 2022 14:57
glendaviesnz
force-pushed
the
add/spacing-sizes-part-1
branch
from
081c1eb to
64479cf
Compare
Member
|
@glendaviesnz just retested after the style engine change merge, and things are working as before 👍 |
Contributor
Author
Thanks @jameskoster I went ahead and merged this, and we can iterate in the number of steps in the scale once we finalise the UI. @WordPress/gutenberg-design feel free to still add feedback here, or over on the main spacing presets issue. |
bph
added
the
Needs Dev Note
Contributor
|
I was testing this with |
Contributor
Author
Thanks @scruffian, will take a look at that today. |
Contributor
Author
|
Have added a PR to fix this @scruffian |
Contributor
|
Thanks! The other thing I noticed is that when I use these variables in a template, they don't work in the site editor. |
37 tasks
oandregal
reviewed
Sep 9, 2022
| $result->merge( static::get_user_data() ); | ||
| } | ||
| // Generate the default spacing sizes presets. | ||
| $result->set_spacing_sizes(); |
Member
There was a problem hiding this comment.
If the goal is to create a new preset, why doesn't go through the normal data layers (both the preset and any option to enable/disable)?
- core can provide spacing presets
- theme can provide spacing presets
- users can provide its own as well
By not respecting the algorithm, things like making global styles data filterable becomes more difficult, see #44015
Contributor
Author
There was a problem hiding this comment.
@oandregal that comment is a little misleading, it isn't actually generating the presets, the spacing presets are generated via the standard flow using PRESETS_METADATA.
This method is actually generating the spacingSizes array, which is the equivalent of the fontSizes array ... but unlike fontSizes the core default for spacing is to autogenerate the array from a set of scale values.
The idea was that themes or users could override some or all of the scale values, eg. at some point in global styles there will be a UI where a user may just update the increment value - so generation of the spacingSizes needs to happen after the core/theme/user spacingScale values are merged - hence it's location here.
But, maybe we need to rethink this if this is going to hinder the filters you mention? I will update the comments to avoid confusion, but will hold off doing so until hearing from you if think this spacingSizes array generation should be happening elsewhere.
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.
What?
As the first step in implementing spacing size presets in theme.json this PR adds the initial theme.json settings. This will be followed up by additional PRs that will allow the selection of these values in the post and site editor.
Why?
Currently spacing design options in the editor like padding, margin, and gap only allow for the setting of custom values. This means that theme authors are not able to specify a set list of spacing sizes in order to keep design consistent in the same way as font sizes can be limited to a list of preset sizes.
Also, without a standard list of space presets it is not possible for user applied spacing to easily transfer across themes, or for spacing values to be maintained when block content and patterns are pasted between sites.
How?
Based on the discussion on the related issue this PR adds:
customSpacingSizeboolean to allow theme authors to disable the setting of custom margin, padding, gap values. The first phase of this is a single toggle for all these values, but if there is demand for independent toggle of these this can be change later to allow either a boolean, or an object with booleans for each of the spacing types.spacingScaleobject which contains a list of values to auto generate an array of spacing sizes. The core theme.json only specifies aspacingScaleebut themes can specify eitherspacingScaleand/or aspacingSizesarray. Values added to a theme'sspacingScalewill overwrite the core values, and any hard coded spacing size in a theme'sspacingSizesarray will be merged with the core generated array. Theme values with the same slugs as core will override the core values. A theme can completely remove the core scale by settingspacing.spaceScale.stepsto0spacingSizesarray contains the list of preset sizes can be added by themes. As withcustomSpacingSizethis is a single array of values for all spacing types. If needed this can also be expanded later to allow for either a single array, or an object with an array of values for each spacing type.Based on the discussions here, it seems to make the most sense to go with a numeric value for the spacing preset slugs and to provide a default set of 9 sizes in the core theme. This allows for 4 settings either side of a
mediumsetting. The slugs for the scale will be10,20,30,40,50,60,70,80,90, with50as themediumpoint. Using a numeric scale like this allows for:50Testing Instructions
--wp--preset--spacingcss custom properties are set in the headerspacingScaleobject to the current themestheme.jsonsettings.spaceswith theoperatorset+` and check that custom properties change correctlyspacingSizesarray to the current themestheme.jsonand make sure this is merged with the values generated by core0and check that all of the core presets are removednpm run test-unit-php /var/www/html/wp-content/plugins/gutenberg/phpunit/class-wp-theme-json-test.phpandnpm run test-unit-php /var/www/html/wp-content/plugins/gutenberg/packages/style-engine/phpunit/class-wp-style-engine-test.phpvar:preset|space|20values intovar(--wp--preset--spacing--20);in the block stylesScreenshots
The following custom css properties should be set:
This 9 point, non-linear scale based on a perfect 5th multiplier results in follow spaces based on 16px base font, which puts
mediumat 24px, which is the current default for block gap: