Block Editor: Use groups for InspectorControls #34069
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
gziolo
added
[Package] Block editor
gziolo
commented
Aug 13, 2021
7 tasks
|
Size Change: +130 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
gziolo
commented
Aug 16, 2021
packages/block-editor/src/components/inspector-controls/fill.js
Outdated
Show resolved
Hide resolved
|
Given that #34063 is based on this branch, I plan to remove the slot for the group |
7 tasks
gziolo
force-pushed
the
update/inspector-controls-groups
branch
from
435415b
to
e3309e8
Compare
gziolo
commented
Aug 19, 2021
| @@ -374,4 +179,44 @@ registerBlockType( 'my-plugin/inspector-controls-example', { | |||
| } ); | |||
| ``` | |||
|
|
|||
| {% end %} | |||
| ## InspectorAdvancedControls | |||
There was a problem hiding this comment.
Let's keep this section in the documentation until we mark the group as a stable API.
|
Thanks for working on this. From the instructions this appears to be mostly an under-the-hood change for the time being? Let me know if you need any mockup work for the absorption of parent inspector panels in child blocks. |
I want to start another PR to better channel discussion around explorations. This change will also let us exploring new groups for Global Styles related controls. And yes, this change is purely technical 👍🏻 |
|
I tested the PR on an iOS mobile build and everything worked for me as expected. So I think it is good to go from the mobile perspective. |
|
This is looking great @gziolo, I like the addition of groups to InspectorControls and that the advanced controls just becomes another kind of group. I was wondering if it'd be worth adding a |
There was a problem hiding this comment.
This is great! Thanks @gziolo 👍
The code looks good to me and the changes test well for both web and native.
I've also been testing this out by extending this PR to allow injection of controls into block support panels in #34063, #34157 and #34065. This work has made achieving that much simpler and cleaner.
It might be best to get official approval on this from someone else that has more background on how this aligns with other APIs and the bigger picture. For me though, I'd be happy for this to land. 🚢
I was wondering if it'd be worth adding a constants.js file and defining the advanced string once so that we avoid using string literals?
If we do this, I'd like to see it happen for the BlockControls groups as well.
@youknowriad, what do you think. I mostly followed your initial implementation. I don't think we use constants for props in other places. We can solve it with TypeScript types one day though. |
gziolo
force-pushed
the
update/inspector-controls-groups
branch
from
83bc87e
to
8312f54
Compare
I wonder if the extra "import" to retrieve the consts would just add friction for block authors. Or is it just for internal usage? |
I was imagining it as just for internal usage (mostly to avoid typos until we can use TypeScript types). Since the current approach here is consistent with the |
|
Let's move forward with this one and iterate further. |
gziolo
changed the title
gziolo
added
[Type] Experimental
7 tasks
This was referenced Oct 13, 2021
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.
Description
Related to #29247 where @youknowriad added support for groups in block toolbar.
The context for groups in
InspectorControlscan be found in the related comment #34063 (comment):The initial implementation covers only two existing groups:
default(InspectorControls)advanced(InspectorAdvancedControls)How has this been tested?
There should be no changes in how inspector controls work so I expect all e2e tests to pass.
I refactored
InspectorControlsandInspectorAdvancedControlsin React Native implementation so it has to be confirmed it doesn't break anything.Types of changes
Refactoring. There should be no changes in how it works.
Checklist:
*.native.jsfiles for terms that need renaming or removal).