-
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
Block supports: allow "all" as a value for __experimentalDefaultControls #38972
Conversation
Size Change: +96 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
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.
Nice idea @ramonjd! It's working pretty well for me in testing, just noticed one missing item from the Typography support.
This should make it a little simpler in block.json
for those cases where we want to opt-in to expose all controls by default for the block (as we do for the border support in the Group block, for example).
I was wondering if this is a feature that we should allow themes to potentially opt-in to? So, say for paragraph blocks we currently make the assumption that we'd like only font size to be shown by default (to de-prioritise all the other settings). Should a theme be able to say which of the opted-in controls the theme would like to make visible by default (and therefore be able to add __experimentalDefaultControls
to the settings in theme.json
?) — just an idea, though, I'm not sure if it'd be valuable or not?
…ontrols when __experimentalDefaultControls is true
Co-authored-by: Andrew Serong <[email protected]>
Adding missing textTransform property for typography
a35175e
to
64f5992
Compare
Thanks for the 👀 @andrewserong ! I've fixed the bogeys. 😄
This is a good question. Currently we don't, which is what made this PR surprisingly easy 😄. Still, part of the motivation behind this PR was to open up more control to theme developers over the way the editor behaves. See: #38219 (comment) I had a look around for any conversations on this topic in related PRs and issues (mostly by @aaronrobertshaw), and couldn't find any opinions. I would tend to say "yes, let's open it up in theme.json", but there might be constraints or UX considerations I'm ignorant of. cc @jasmussen for advice. |
Attribution fixed in the description 😄 |
Nice one, glad to see this is a simplish PR! Just to be sure I get this right: this is simply a shorthand for something you can already accomplish, right? Or is it a blanket opt-in to every future design tool? I think there's a careful balance for Global Styles to find, between making it easy for theme developers to lock things down, as we continually hear requests for, while at the same time offering ways to opening up to users, which we also hear requests for. Balancing those ends of the spectrum with a good user experience is enough of a challenge that I'd be reluctant to introduce new flags like this at least for now. But it could definitely be something to land once we reach a point of stability on that spectrum. What do you think? |
Yes, it will only show the default controls that are available now (and also those that a block opts into). We'd need to add future design tools to the "all" list in order for them to work. It's not a ground-breaking change, just a convenience, and, to be honest, it only saves a bit of typing right now. So the difference between: "spacing": {
"margin": [ "top", "bottom" ],
"padding": true,
"blockGap": true,
"__experimentalDefaultControls": {
"margin": true,
"padding": true,
"blockGap": true,
}
}, and "spacing": {
"margin": [ "top", "bottom" ],
"padding": true,
"blockGap": true,
"__experimentalDefaultControls": "all"
}, The motivation was to eventually provide theme authors with a convenience flag to display all options, and remove frustration with having to toggle optional controls every time one inserts a block.
This makes sense. The flag is still |
I assumed the problem was expanding and using controls on one block, then adding the same block again and not seeing those controls automatically expanded? Rather than it just being a blanket expansion of all controls. |
Exactly. That's the more nuanced version 😄 I suppose, stripped away, this PR just proposes a convenience flag. It's something we can already do with a few extra lines of JSON, and it adds a new value to the API, so I'm beginning to come around to the realization that this one might be better off left alone and closed. |
I'm new to this, so sorry for the apparent question 😅: would there be compatibility issues in the future? Let's say we add one more default control to the list, block authors who only expect their blocks to have the previous default controls will all get this new one when upgraded. Is this expected behavior? |
Thanks for thinking about this @kevin940726 This is a very good point, and another reason why I think we should close this one for now. |
Description
This PR introduces a method for each block support that shows all supported controls when
__experimentalDefaultControls
istrue
.It will only show the controls where the block.json has opted into the support.
This PR is mainly to test the appetite for such a feature.
Props to @andrewserong for the idea
Testing Instructions
Where a block's
block.json
opts into a support, swap the object for"__experimentalDefaultControls" for "all"
to show all controls in the panel by default.For example:
Testing using the Group Block is convenient. Here's an example
block.json
:Group block.json
Screenshots
Opting into all controls for the Group Block:
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).