Repository navigation
Conversation
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: +206 B (0%) Total Size: 8.29 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
|
ramonjd
left a comment
There was a problem hiding this comment.
This is working as described. I'll lean on design review.
I think splitting advanced tools where there are separate tabs makes for a logical split.
|
Generally, I think this is a good instinct. I do think there may be some nuance. I believe the Styles tab has largely been a collection of block supports, and with that as the definition of what counts as "style", I guess adding an "Advanced" panel may be an exception. I think there's also a small risk that since the panel is called the same, and that you now have two identical panels, one in Settings, one in Styles, you might not realise they are different panels with different contents. This is not a strong opinion, wouldn't want to block the effort, it's mostly some thoughts, and I can defer to others. One other detail: Francisco (already pinged) has been looking into some block-card dropdowns relating to style inheritance. And that this dropdown could also be where we eventually move at least the "Apply globally" button. That would still leave CSS classes and custom CSS in the grey zone, but nevertheless. |
aaronrobertshaw
left a comment
There was a problem hiding this comment.
This PR makes sense to me.
"Apply globally" sits much better in the same tab as the styles it pushes. @jasmussen makes a good point though that it could also belong in the upcoming popover in the block card that highlights which styles have local overrides. I'm not sure that popover will be the final evolution of that feature though so I'd probably vote to keep it in a somewhat familiar location for users i.e. an Advanced panel in the block inspector.
On Joen's point about two "Advanced" panels, I can see the potential confusion. These are more power user tools though, so I suspect it's smaller in practice than it looks. Someone reaching for Additional CSS, a custom class name, or to apply styles globally, usually knows what they're after.
Would a simple rename for the Advanced panel under the Styles tab be enough to offset some of that confusion?
Keep "Advanced" under the Settings tab, and rename the panel under Styles to something like "Advanced styles" or "Additional styles"? Under the hood, the inspector controls group is already named advanced-styles.
I like the suggestion. I think the decision is what to do when there's one tab - if we stick with a single Advanced panel, it might be confusing that the tools are sometimes in 'Additional styles' and sometimes in 'Advanced'. Or we could keep the split and still have separate 'Additional styles' / 'Advanced' panels when there's one tab. Maybe I should try out the latter. |
f624e31 to
81b5cad
Compare
9169a3c to
0ca64d7
Compare
Yes! |
|
Would it make sense for that additional styles group to be a toolspanel? I'm looking at additional CSS for style states in #83539 and it's looking a bit weird to have that one random control between toolspanels:
|
|
I think @jameskoster had a design for it being a toolspanel. Importantly though, that would mean we hide everything inside ellipsis items by default, similar to the collapsible being collapsed by default. I'd also think we'd want to move "Apply globally" out from there and elsewhere if we did this. Also important than to connect a dot from @annezazu whose feedback here suggests that "Advanced" could be where we show style-inheritance information, rather than in a block card. If that is indeed the direction we go, it pairs naturally with "Apply globally" already sitting there, but if it became a toolspanel it would suddenly be two clicks to see style inheritance information, rather than just a single one to expand the panel. |
|
CC: @fcoveram relating to this comment, on block cards, advanced panels, and the journey it is to surface style inheritance information. |
0ca64d7 to
a63060f
Compare
Yep, potentially. Though worth mentioning your screenshot doesn't show a The presence of 'Settings' like that also feels a bit off. |
|
Pushed some changes that switch 'Additional styles' to a tools panel. It's ok I think, though 'Apply globally' might be unusual as a tools panel item. Not sure if there are examples of other controls like this that don't have a value and are a tools panel item. Kapture.2026-10-05.at.15.06.30.mp4 |
|
Good experiment. I feel like it might be a panel too many. Not strongly felt. |
|
I don't have a strong feeling about toolspanel vs collapsible panel; both can work. But one thing the toolspanel experiment highlights for me is that Apply globally does not conceptually fit in this panel. I don't think it has anything to do with 'Additional styles'. At a push I'd be tempted to consider introducing the same ellipsis menu we find in the document inspector header to the block inspector header, and putting this action there. Then you can push styles regardless of the active tab. Other actions like 'Hide', 'Lock', 'Rename' could theoretically live in that menu too. |
This is essentially what @fcoveram designed for a block inheritance card. |
That part is already in trunk, in the Gallery block responsive controls, which is using the state-specific inspector slot. We could rename it but not sure what to 😅 |
| panelId={ clientId } | ||
| hasValue={ () => !! className } | ||
| onDeselect={ () => setAttributes( { className: undefined } ) } | ||
| resetAllFilter={ () => ( { className: undefined } ) } |
There was a problem hiding this comment.
Not a blocker, this resets the current style variation too.
In trunk, the only way to remove the style is to delete it by hand.
I think it's okay, since it is an "advanced" panel after all, just flagging
|
|
Given the issues, I think perhaps it's better to use a regular It also gives third party extenders a place to add style tools that don't neatly fit into a tools panel. Any strong feelings against that? |
No, sounds good! |
…panels The Advanced panel has always lived in the Settings tab, so styling tools such as Additional CSS end up under Settings even when a Styles tab is present. Introduce an 'advanced-styles' inspector controls group alongside 'advanced', and teach the Advanced panel to render either group. The Styles tab now renders its own Advanced panel for the new group, while the Settings tab keeps the existing one. When the inspector has no Styles tab, or renders no tabs at all, a single panel holds both groups so that no control is dropped. No controls are moved to the new group yet. Block Editor: Move Additional CSS and CSS classes to the styles Advanced panel Additional CSS and Additional CSS class(es) are styling tools, but the Advanced panel that held them lives in the Settings tab. Fill the 'advanced-styles' group instead so that they appear under Styles when a Styles tab is present, and fall back to the combined Advanced panel in Settings when it is not. Editor: Move Apply globally to the styles Advanced panel Pushing a block's typography, spacing, dimensions, and color to global styles is a styling action, so fill the 'advanced-styles' group rather than the settings Advanced panel. Also replaces the deprecated InspectorAdvancedControls wrapper with InspectorControls. Block Inspector: Restore the gap between the two Advanced groups Controls in the inspector take a bottom margin unless they are the last child of their container. Each inspector controls slot renders its own container, so when the Advanced panel holds both the settings and the styling group, the last control of the first group sits flush against the first control of the second. Space the two containers apart. Block Inspector: Cover the split Advanced panel with tests Adds unit coverage for the settings-only, styles-only, and combined renderings of the Advanced panel, and records the change in the block-editor and editor changelogs. The test imports a component whose relative imports reach 484 of the package's 566 source files, none of which the build allowlist types yet. A composite project has to resolve every relative import within its own program, so the test joins the two stories already excluded from the dev project rather than being written as JavaScript with an ESLint suppression. Block Inspector: Fold the changelog into a single entry per package Drops the separate New Features entry for the `advanced-styles` group, which the Enhancements entry now covers, and adds the pull request links. Block Inspector: Mark the Advanced panel's props as optional Every prop has a default, but the JSDoc declared each one required, so TypeScript inferred a props type that demanded all three. The routed TypeScript test graph checks test files against that inferred type and rejected the test for omitting props it does not need. Block Inspector: Split Advanced into Advanced and Additional styles panels Rename the advanced-styles group to additional-styles and give it a dedicated Additional styles panel that mirrors Advanced, rather than merging both groups into a single Advanced panel. - Advanced keeps its settings tools (HTML anchor, allowed blocks) and is unchanged apart from the style fills leaving it. - Additional styles holds the styling tools (Additional CSS, Additional CSS class(es), Apply globally) in its own panel. - When the inspector has no Styles tab, or renders a single tab, the two panels appear side by side instead of merging. - Count the private allowed-blocks fills when deciding whether to show a Settings tab, so a block whose only advanced tool is Allowed blocks keeps that tab. Block Inspector: Stop counting allowed-blocks fills toward the Settings tab The allowed-blocks control renders in the Advanced panel via a private slot that the tab-counting logic never accounted for. Counting it was added as a fix for blocks whose only advanced tool is Allowed blocks, but it over-fires for section blocks (unsynced patterns): they always have a Content tab and run in the default editing mode, so the fill registers and forces a Settings tab that only shows Manage allowed blocks. That underlying gap is a pre-existing trunk issue, so revert the count here and address it in a separate PR. The additional-styles split is unaffected. E2E: Update Push to Global Styles for the Additional styles panel The Apply globally control now lives in the Additional styles panel rather than the Advanced panel, so open that panel in the shared setup helper. Rename global styles Advanced panel to Additional styles Remove the word 'additional' from CSS and CSS Class(es) control labels. Now that the containing panel is labelled 'Additional styles', also using 'additional' in the control labels is duplicative. fix command center test
Hide Apply globally by default Ensure correct a11y text for a non-resettable hideable tools panel item
cf91030 to
f129e89
Compare
|
Ok, I switched back to using the PanelBody. The one thing on my mind is whether this change is going to mesh well with the upcoming style inheritance changes. Let me know if this is likely to cause any issues. |
ciampo
left a comment
There was a problem hiding this comment.
Seeing that ToolsPanel changes have been undone, I'll let the rest of the folks involved carry on the conversation / review.
| const groups = { | ||
| default: InspectorControlsDefault, | ||
| advanced: InspectorControlsAdvanced, | ||
| 'additional-styles': InspectorControlsAdditionalStyles, |
There was a problem hiding this comment.
Should we document group="additional-styles" and update the inspector tab mapping?
Looks like the current docs still place custom CSS classes under Advanced and don't mention the new group.
Oh dear 😅 Using |
|
I like the idea of having an “Additional styles” section. It’s better located and more explicit about what you can find there. But I’m not sure about locating the “Apply globally” action there. I agree with @jameskoster approach. The area should gather all actions belonging to the same category and this one falls short there. Regarding the other fields, “CSS Class(es)” is more clear on what users can type there compared to “CSS”, perhaps “CSS properties” is a better label. From the inherited styles conversation (#82546), I’d say that “Styles” toolsPanel is a good place for the global action, but it seems better to do it when the action to push local changes (i.e., change the text color and add it as a custom option from the popover) is close to the control that modifies it. That consistency would make the style-related actions clearer. |




What?
Proposes moving 'Additional CSS Class(es)', 'Additional CSS' and 'Apply Globally' to an Additional styles panel in the Styles tab of the block inspector.
If a block has no settings or styles tabs (for example, paragraph only has style controls) then there are now Additional styles and Advanced panels that group the controls, both appear in the one inspector.
Why?
I've noticed for a while that the block inspector shows 'Additional CSS' and 'Apply Globally' (Push to Global Styles) in the Settings tab, and I this this could be improved. These are more 'Styles' tools than 'Settings'.
'Apply Globally' is one that's problematic to me, as users are likely to use this when having just modified styles. Users have to switch to the settings tab, scroll down, expand the 'Advanced' panel and then click the button, and then probably switch back to the Styles tab. It's a lot of context switching.
How?
Introduces a new 'additional-styles' inspector group specifically for styles controls.
Renames 'Additional CSS Class(es)', 'Additional CSS' to 'CSS Class(es)' and 'CSS'.
Renames the global styles 'Advanced' panel to 'Additional styles'.
Testing Instructions
Screenshots or screencast
Use of AI Tools
OpenCode / Claude, Kimi