Spacing presets: Implement disabling of custom space sizes #43216
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
glendaviesnz
added
the
Global Styles
|
Size Change: +7 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
18 tasks
andrewserong
approved these changes
Aug 15, 2022
There was a problem hiding this comment.
This is testing nicely for me @glendaviesnz:
✅ Padding in the block editor defaults to allowing custom values, but hides the toggle when customSpacingSize is set to false
✅ Padding in the site editor / global styles defaults to allowing custom values, but hides the toggle when customSpacingSize is set to false
This isn't blocking for merging, but something I wondered is how should the UI behave when there are blocks that already exist (or are added via block patterns) that contain custom values? With this PR applied, since the control can't be toggled to custom, with the block selected, it doesn't display the current padding value, even though there is one. Here's a screenshot:
In this example, it's a Group block with 10px padding. Here's the markup:
Group block markup
<!-- wp:group {"style":{"spacing":{"padding":{"top":"10px","right":"10px","bottom":"10px","left":"10px"}}},"backgroundColor":"vivid-red"} -->
<div class="wp-block-group has-vivid-red-background-color has-background" style="padding-top:10px;padding-right:10px;padding-bottom:10px;padding-left:10px"><!-- wp:paragraph -->
<p>A paragraph in an existing Group</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
<!-- wp:group -->
<div class="wp-block-group"><!-- wp:group {"style":{"spacing":{"padding":{"top":"var:preset|spacing|60","right":"var:preset|spacing|60","bottom":"var:preset|spacing|60","left":"var:preset|spacing|60"}}},"backgroundColor":"vivid-green-cyan"} -->
<div class="wp-block-group has-vivid-green-cyan-background-color has-background" style="padding-top:var(--wp--preset--spacing--60);padding-right:var(--wp--preset--spacing--60);padding-bottom:var(--wp--preset--spacing--60);padding-left:var(--wp--preset--spacing--60)"><!-- wp:paragraph -->
<p>A paragraph in a new Group</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group -->From my perspective, I don't think this is a blocking issue, because I imagine that themes that use this setting would likely be attempting to prevent users from using values outside of the presets anyway, so might just be a bit of an edge case?
Good question. I think we should leave it with custom values being disabled even if a block has them set and revisit it if there is any user feedback that this is causing issues. |
Sounds good 👍 |
57 tasks
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?
Implements the ability to disable custom sizes - at this point just for padding, via the new spacing presets UI.
Why?
Theme and plugin authors have been asking for the ability to prevent users from adding custom sizes and only allowing them to select from a list of presets
How?
Wires the
settings.spacing.customSpacingSizeproperty that was prepared earlier through to the new spacing presets UI.Testing Instructions
settings.spacing.customSpacingSizeto falseScreenshots or screencast
Before:
no-space-before.mp4
After:
no-space-after.mp4