Sidebar: Make display of tabs filterable #46026
Closed
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
aaronrobertshaw
added
[Type] Enhancement
aaronrobertshaw
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
|
|
Size Change: +25 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
Base automatically changed from
try/conditionally-display-sidebar-tabs
to
trunk
November 25, 2022 05:19
aaronrobertshaw
force-pushed
the
update/make-sidebar-tabs-filterable
branch
from
4b55f86 to
f041f13
Compare
glendaviesnz
approved these changes
Nov 28, 2022
aaronrobertshaw
force-pushed
the
update/make-sidebar-tabs-filterable
branch
2 times, most recently
from
3ba1309 to
0110da3
Compare
aaronrobertshaw
force-pushed
the
update/make-sidebar-tabs-filterable
branch
from
0110da3 to
b0c62ae
Compare
|
I'm not sure if using filters for changing the tab display is the best option here is the best option. But I've been wrong before 😅 The editor has a bit different philosophy when it comes to extensibility. Some of that is being documented here #30963. I would love to hear more feedback from @WordPress/gutenberg-core, before we land this approach. |
Thanks for advising caution here @Mamaduka 👍 While I'm aware of our general aversion to adding new filters, the need for this proposed filter arose during a recent walkthrough of the sidebar tabs Gutenberg experiment. In that discussion with @mtias, @priethor, and @jasmussen, there was a clear desire for only 3rd party plugin authors (not users) to be able to override the display of tabs. The primary use case is that their plugin may drastically overhaul the sidebar, even introducing their own tabs that would otherwise conflict (see Spectra Blocks). The solution to that, was the proposed filter, but I'm happy to implement a different approach if you have one.
Can you expand on how you see this filter not fitting the philosophy outlined in #30963? I take it that the following quoted section is what you are referring to?
Some quick thoughts on the above include:
I don't think there is an immediate need to land this, so it would be great to get further feedback. Thanks again 👍 |
|
I think @youknowriad summarized some of the cons of using filters for extensibility in this comment #37448 (comment). The alternative approach would be to create an API that hides implementation details. Maybe something like this: // return active inspector tabls for the block type.
getBlockInspectorTabs( blockType )
// Disable tab for the block type
disableBlockInspectorTab( blockType, tab ) |
Thanks for the additional link. I'll read up on the discussion there and look to iterate on the approach this week. |
|
Thanks for the ping. This is tricky for me to answer definitively on as I'm not as strong on the dev side, so I'd appreciate additional opinions. In general, I'm always apprehensive about extensibility, mainly erring on the side of adding them when we're sure we want them and other options are exhausted, and then naming them semantically instead of based on a location, so that if we have to move or tweak them in the future, keeping the semantics intact, we can. In the case of the split tabs, it makes sense that supports-based features are automatically are put in the styles tab, and everything else goes in the settings tab. And also that if there are no items in the styles tab, don't show it the tab bar at all (i.e. it makes sense to remove the "Color" support from Query Loop so that when you select it, you see just the Query Loop settings as those are the important ones for that block). In that vein, we can get pretty far with just best practices. But still, I do hear the reason that if a third party implements a slew of carefully curated features in a highly opinionated way, forcing those into either of these tabs might be really problematic. So I wouldn't be opposed to there being a way to entirely opt out of that. How that looks, I'm not sure, and I'm happy to refer to developers on the details. Is that helpful? |
|
Closing this PR to explore an alternative approach avoiding the use of a filter. |
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.
Depends on:
Related:
What?
Adds a filter allowing plugins to disable the display of tabs in the sidebar.
Why?
Plugins may drastically change the controls in the sidebar or even introduce their own tabs. Making the display of the sidebar tabs filterable will help provide plugins with the escape hatch they need.
How?
editor.BlockInspector.showTabsTesting Instructions