-
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
Sidebar Tabs: Use editor settings to override display #46321
Conversation
Size Change: +110 B (0%) Total Size: 1.33 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.
Pretty happy with this approach.
I personally don't think removing tabs from the link block is the long term solution to the nav block's List View interface, but it does solve the immediate problem and provides some time for more exploration into the right solution.
Nice work!
if ( tabSettings[ blockName ] !== undefined ) { | ||
return tabSettings[ blockName ]; | ||
} | ||
|
||
if ( tabSettings.default !== undefined ) { | ||
return tabSettings.default; | ||
} |
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.
So at the moment you can set default: false
and something like 'core/paragraph': true
to enable tabs only on one block. Or default: true
and 'core/paragraph': false
to disable only one block.
That's pretty flexible, and honors the individual tab setting, which is nice.
The other option might be having 'default' as a separate setting entirely instead of a property, but I don't really have a strong preference either way. I wonder if anyone has ever made a block with the name 'default', as that may cause an issue. It would be going against the block naming guidelines, which recommend namespaced block names (my-plugin/default), so probably not something to worry about.
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.
Thanks. I'll be happy to trial the alternative if others have stronger opinions or different thoughts.
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.
Thank you so much for working on this.
Some things I spotted. Will test manually shortly...
packages/block-editor/src/components/inspector-controls-tabs/use-inspector-controls-tabs.js
Show resolved
Hide resolved
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.
Screen.Capture.on.2022-12-06.at.11-17-23.mp4
I tested in both Site and Post editors and this worked as expected for the Nav block experiment 👏
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.
LGTM
Lets address the feedback in a followup... |
A PR addressing the majority of feedback is up in #46346 |
* Sidebar Tabs: Use editor settings to override display * Roll check of tab display into use tabs hook * Disable display of tabs for Nav Submenu block
Related:
What?
Why?
We need a means of providing plugins and third party blocks with the ability to disable the display of block inspector tabs.
How?
__experimentalBlockInspectorTabs
block_editor_settings_all
filter to the Navigation Link block to disable tabs for itAlternative Approaches
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screen.Recording.2022-12-06.at.2.09.04.pm.mp4