Global Styles: Show a local-override diamond indicator for inherited values and ship them to Core - #80649
Global Styles: Show a local-override diamond indicator for inherited values and ship them to Core#80649annezazu wants to merge 20 commits into
Conversation
|
The only annoyance I have found thus far in this approach comes down to how the tools panel reset option works. When I set a typography option like Font or Appearance, I go looking to reset it and there's no obvious "Reset" button. Meanwhile, if you set Color, it shows me a clear "RESET" label right there. This seems to be by design where some options have a reset option via the checkmark in the ⋮ menu. It's not clear though that clicking the checkmark is what clears it. These two different reset patterns in the same menu is an annoyance but not a blocker to landing something at this point. Here's a video explaining: annoyance.mov |
|
@jasmussen can you give a final 👍🏼 design/UX wise? Thank you all. I know this has been a lot over what might seem like a small detail but it will permeate so many aspects of the editor if/when it lands. |
| */ | ||
| export function InheritanceOverrideIndicator( { className } ) { | ||
| return ( | ||
| <Tooltip.Root> |
There was a problem hiding this comment.
We're starting to differentiate between tooltips (which are just for showing the accessible name of icon buttons), and infotips (which are a dedicated mechanism to show arbitrary content).
So here we'll want to use the Popover component from @wordpress/ui instead of Tooltip (the basic implementation pattern is documented here).
There was a problem hiding this comment.
Got it! Just updated the PR to use that. Will update the description and video too for good measure in a moment.
The The "shown by default" controls are more "show all the time". So in the menu they are always checked, because they can't be toggled off, they display the Earlier iterations of the I hope that provides a little context |
I won't get time this week sorry. |
Agree. Commented over here #80506 (comment), but I think maybe it's enough to show the inherited value for now. Before the confusion arose that a block background was red for example, but the control didn't reflect the value. At least now the control matches the value in the block attributes. 😄 Anyway, thanks for iterating here and trying out things! If I get time next week I can take a look, but I might have to wait until @aaronrobertshaw is back for support. |
@joedolson For context, we were aligning with upstream guidance to separate labeling popups and informational popups. And the infotip pattern will indeed be necessary when the popup wants to contain anything more than plain text. It just happens that this instance still doesn't. The strings in this branch are currently a bit off (very redundant), but the intended implementation is such that the trigger's aria-label is something like "More information about X", where it tells you that the button is going to expose additional information about X. In this particular case, the aria-label could be something like "Where is the X value coming from?" or "See X inheritance" or whatever, and then the popover can show more information about this. As we continue enhancing this inheritance indicator feature, the infotip popover can contain richer layouts, and maybe even links or buttons to edit the parent control its inheriting from. Let me know if this general infotip pattern needs to be changed in some way, since we need to cover this kind of pattern in the design system in some way or another. |
…values Iterates on how block-inspector controls signal that a value locally overrides one inherited from Global Styles (follow-up to WordPress#77894 / WordPress#80506). - Nothing at rest: an inheriting control shows the inherited value with no marker (removes the at-rest dotted-underline label treatment). - On a local override, a small filled diamond appears next to the control with an "Overrides inherited styles" tooltip. - The diamond is a status indicator only and performs no action; clearing a local override relies on the panel's existing reset options. Replaces the interactive InheritanceResetButton with a shared InheritanceOverrideIndicator (a Button hosting a filled-diamond SVG wrapped in a @wordpress/ui Tooltip), used by the standard InheritanceToolsPanelItem controls and the custom color, gradient, shadow, duotone, and background image controls. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review feedback: the diamond's purpose is to reveal the "Overrides inherited styles" explanation, which is an infotip, not a tooltip (a tooltip is only for the accessible name of an icon button). Uses @wordpress/ui Popover with an openOnHover trigger so the content is reachable by hover, keyboard focus, and tap — a tooltip is unavailable to touch and screen-reader users. Also fills in the PR number in the CHANGELOG entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reverts the infotip in favour of the @wordpress/ui Tooltip, per the accessibility review on WordPress#80649: the entire message is already the trigger's accessible name, so the infotip's Popover.Title and Popover.Description only duplicated it and screen readers announced the same string three times. A tooltip popup is visual-only, which keeps the aria-label as the single source of truth. The trigger goes back to an enabled Button whose only job is to host the tooltip, so tapping it still performs no action. Also inlines the Diamond SVG into the indicator and trims the JSDoc, which had grown to restate the review discussion. Co-Authored-By: Claude <noreply@anthropic.com>
`InheritanceOverrideIndicator` hand-rolled a Tooltip.Root/Trigger/Popup wrapper
around a Button, which is what Button already does for itself: with a `label`
and no children it renders its own tooltip and mirrors the string into
`aria-label`. Collapsing to a single `<Button label icon />` drops the wrapper,
the duplicated label constant and the `@wordpress/ui` import.
Passing the diamond through the `icon` prop instead of as children lets `Icon`
size the SVG and adds the `has-icon` class, so most of the CSS that existed to
undo the default Button chrome (sizing, padding, background, box-shadow,
cursor, text-transform) is no longer needed. `size="small"` replaces the
`__next40pxDefaultSize={ false }` opt-out and its lint exemption.
Co-Authored-By: Claude <noreply@anthropic.com>
a6fee9d to
4ddea4a
Compare
The WordPress#77894 entry had been rewritten in place to describe the final diamond indicator, which erased the fact that the at-rest treatment shipped as a dotted underline plus inline reset and only later changed. Restore that entry to its original wording and add a separate WordPress#80649 entry for removing the dotted underline and reset affordance in favour of the local-override diamond. Co-Authored-By: Claude <noreply@anthropic.com>
Drop the redundant `carries no reset action or menu` case (the indicator's lack of a reset is already covered where it matters) and the explanatory comments that merely restated what the assertions already show. Also tighten the background-image `hasLocalOverride` comment. Co-Authored-By: Claude <noreply@anthropic.com>
Both tests asserted that no "Reset to inherited value" button renders, but the override indicator no longer has any reset control — that label exists nowhere in the source, so the assertions were always trivially true. Remove them and simplify the indicator test to just check the diamond renders. Co-Authored-By: Claude <noreply@anthropic.com>
Condense the JSDoc and SCSS comments in the inheritance module — they had grown to restate design rationale and CSS mechanics at length — to the point each one needs. Also drop the "Reset to inherited value" absence checks from the border-panel shadow tests; that label no longer exists in the source, so the assertions were always trivially true. The real "remove" button assertions, which cover actual behavior, stay. Co-Authored-By: Claude <noreply@anthropic.com>
|
I believe it's quite important that we surface this information to the user, and this looks like a great solution 👍 Thanks to everyone for working hard to find the best path forward here! |
Let's not forget that tooltips are not available on touch devices though. |
|
Thanks for the feedback!
That's right. It probably depends on what we want to prioritize, whether we should use a tooltip or an infotip.
Regarding the size, it should have been fixed.
Let's try to deal with these two. |
The reset button, override indicator and contrast warning each claimed the same absolutely-positioned slot at the toggle's inline end, so they could only be shown one at a time and the label's reserved space was hardcoded per combination. Group them in a single positioned row instead, and derive the label's max-width from how many buttons that row actually holds, so all three can coexist and the label always keeps clear of them. Also make the label a FlexBlock so a long name shrinks itself rather than squashing the color indicator next to it. Co-Authored-By: Claude <noreply@anthropic.com>
The reset button and the override indicator shared one absolutely-positioned slot at the toggle's inline end, so they could only be shown one at a time, and the toggle reserved a fixed inline-end padding whether or not anything was rendered there. Group them in a single positioned row instead, and derive the toggle's padding from how many buttons that row actually holds, so both can coexist and the label reserves no space when there is no action at all. This mirrors the layout the color panel now uses. Co-Authored-By: Claude <noreply@anthropic.com>
The remove button and the override indicator shared one absolutely-positioned slot at the toggle's inline end, so they could only be shown one at a time. Group them in a single positioned row instead, and derive the toggle's inline-end padding from how many buttons that row actually holds, so both can coexist and no space is reserved when there is no action at all. The toggle label also had no truncation, so a long translation ran under the actions row. Render it as a FlexBlock and clip it with an ellipsis, which keeps the shadow icon from being squashed as well. This mirrors the layout the color and background image panels now use. Co-Authored-By: Claude <noreply@anthropic.com>
ramonjd
left a comment
There was a problem hiding this comment.
I had a look with a view to helping out code-wise, but I think I was a bit premature. Happy to help see this through once the tooltip/infotip call is made.
| right: 0; | ||
| top: $grid-unit; | ||
| margin: auto $grid-unit auto; |
| // The button exists only to host the tooltip, so a click does | ||
| // nothing. |
There was a problem hiding this comment.
Sorry, if I'm way too early and you're still working on this. 😄
So this is intentional? I can focus on the button, and it has hover states but in most of the controls (color aside) it has no affect when triggered.
The pointer cursor/hover styles seem contradictory to me. Is there a way to remove interactivity here, or does it rely on the tooltip/infotip decision?
There was a problem hiding this comment.
To indicate to screen readers that inherited styles are being overridden, I believe at least this element needs to be keyboard focusable. However, it doesn't necessarily need to be actually clickable by mouse. I've opted to use a span with an img role instead of a Button component. This should avoid redundant cursor and hover styles.
inherited-indicator.mp4
Each toggle repeated a per-count comment explaining which buttons the reserved space was for. The button combinations differ per control and drift as buttons are added, so state the shared rule once instead: the actions are absolutely positioned, so the toggle reserves room on the right for as many of them as are rendered. Co-Authored-By: Claude <noreply@anthropic.com>
The reset button was the toggle's only trailing control, but the label's reserved right-hand space is now keyed off an `__actions` element and its child count, as in the background, shadow and duotone panels. Wrap the button in a `Stack` carrying that class so the sizing rule applies and further actions can join the same row. Co-Authored-By: Claude <noreply@anthropic.com>
The indicator exists only to host its tooltip, so rendering it as a Button meant suppressing the click it advertised. Render a `role="img"` span with `tabIndex` through `Tooltip.Trigger` instead: the tooltip stays reachable by keyboard, but the element carries no interactive semantics. Styles that came from Button (size, focus ring) move to the span. Co-Authored-By: Claude <noreply@anthropic.com>
|
This PR is ready for code review. Let's discuss again whether we can ship this as 7.1 Beta 4. |
|
At this point, I would like to land this for the next beta, assuming the code is on track. I think adding the reset option back in per this also greatly helps with the reset being so far away in some cases. Right now, this feels like it strikes the right balance of usefulness while leaving open progressive enhancements for 7.2 where more design thinking can be done.
Yes! This is a great fallback to have for this release if we need to scale back further to still land something of value, without pulling out the entire feature. Thank you to @t-hamano for driving this forward, code wise, and for everyone else for chiming in to refine it further, experience wise. |
|
Thanks a lot for working on this. Great exploration. Due to the use of dot indicator for unseen messages and unsaved changes, my first reaction was that each control will remove the indicator after saving them. It’s not fully clear to me where the value comes from in the default state, and perhaps we could continue exploring that idea based on the latest comments shared in #80506. Showing where the value comes from by default is also in line with how the UI communicates the sync nature of template parts, sync patterns, and block bindings. I see potential in pushing that direction.
+1 to this idea. Showing the inherited value is a great improvement and feels safe to go with this version. |
|
I'm a little out of the loop on these efforts now but @fcoveram there is additional work around displaying where these styles come from in:
That work is only waiting for something that can carry that information as discussed in these recent PRs. I'd also like to state that simply filling a control with a value, that we cannot 100% guarantee matches what is rendered on the canvas, without saying where it comes from seems like a poor experience to me. Imagine a user selecting a block and noticing the background color control has a color value in it. I decide I want to clear it. I've learnt previously that anything in these controls can be clear by resetting the control. I now do that but nothing happens as resetting the control now will reset to the inherited value state. Yes, that is closer to what is rendered in the canvas and the reality of things but as a user I'm confused as to where this comes from, why I can't get rid of it without having to define another color. Additionally, not all controls or styles can easily be "cleared" or unset. So not showing there's a local override, to me, means there's further confusion around why resetting a control or style value magically appears as a different value that I still don't know where it comes from. |
|
With a highlighted kebab menu, we are back in the position of having some trouble exposing the information visually. We shouldn't change the accessible name of the control, as it still does the same thing, so we can't use the existing tooltip. We could add text inside the kebab menu that gives the context - that way it can be more detailed, but is at a click remove to discover. It may also run into some challenges from a design front making sure it works with all the existing button states, but it's possible, if we're willing to accept that you'd have to expand the kebab menu to understand the reason for the highlight. |
|
@joedolson That's fair, I haven't seen the full scope of the previous convos so this was just a quick thought. I would defer to those resolved conversations. Happy to dive in deeper, but my designer and long-time-block-editor-user spidey senses tell me these diamonds scattered across settings aren't quite there yet. We could make the case that they're iterative, toward a more comprehensive solution, but in that case, I don't know if it's worth the temporary step. If there was user research done on this problem, it might be helpful to see how they describe the issue or know how these overrides are currently confusing to them. Personally, I've never had a user mention this issue to me, though they have plenty to say about many other block editor quirks! That doesn't mean it hasn't confused them, except that I think it becomes pretty clear once they interact with it. |
|
Assuming this PR lands, I also want to put forth this draft PR to bring back breadcrumbs to the tooltip. It's one thing to say the styles are inherited (and still useful!). It's another thing to provide direction around where it's inherited from so you can better understand if you want to keep the override in place. Once more, this is AI generated but intentionally pulls from prior art from @aaronrobertshaw PRs. It feels safe to add back into this release, considering this approach. Let's discuss there. |
|
I understand the intent here—to indicate when a value has been changed from the default—but I don’t think the current treatment is particularly helpful. Do we know this is a problem people face? Quick notes:
More broadly, this system of controls has evolved into something fairly complicated, and this feels like it adds another layer of UI without making the behavior easier to understand.
+1
At least with this you can interact with the object that's different.
@joedolson what do you mean here? |
|
@richtabor re:
If we use a highlight on the kebab menu, then we can't have a tooltip that indicates anything new. The kebab menu represents the feature group's options label, e.g. "Typography options", and that needs to stay constant - the tooltip is a representation of the accessible name of the control. The accessible name needs to inform users what the control does, so the tooltip can't change. So the information about what that highlight meant would have to be accessed somewhere else; my expectation would be as information inside the options menu that the kebab expands. |
I feel like hiding this information away within the ToolsPanel menu will detach it even further from the affordance and control it relates to. Additionally, the items within the menu are already sort of overloaded managing both display of controls and resetting them. |
This is being punted from 7.1.Hey folks. Thanks again to everyone for the hard work here to iterate on this, debate details, and try it out. Per this slack thread, this is being punted from the release. At the root of the reason why are a few things: lack of a solid design approach, lack of robust exposure and feedback from users due to it not landing soon enough, and lack of agreement on a way forward. What we did not lack is a willingness to try and experiment. Going forward, I think we can land an experiment in Gutenberg to be extra safe that includes a version of this PR/potentially this PR, along with breadcrumbs. As the work in Properly support Global Styles inheritance indicators UI evolves, that can then be added in to solidify the design approach. |
What do folks think here, could we replace is the |
I think that's the way to go, it'll give more flexibility for experimenting and playing around with the UI. |
Thanks for confirming! Getting a quick PR up now. |
|
Experimental toggle PR: |










Important note
This is meant to keep the conversation going, design and solution wise. The code is completely AI generated though and I welcome devs to close out this PR or take it over to see this through.
What?
Follow-up to #77894 / #80506. Iterates on how block-inspector controls signal that a value locally overrides one inherited from Global Styles.
IS_GUTENBERG_PLUGIN. That gate is removed, so inherited values and the override indicator ship in WordPress Core builds too.Here's a video:
infotip.display.inherited.styles.mp4
Why?
Per design/accessibility feedback in #80506:
@wordpress/uiPopoverwith anopenOnHovertrigger, which keeps the content reachable by hover, keyboard focus, tap.How?
InheritanceResetButtonwithInheritanceOverrideIndicator— aPopover.Trigger(openOnHover) hosting a filled-diamond SVG, with the "Overrides inherited styles" text in the popover.InheritanceToolsPanelItemcontrols and the custom controls (color, gradient, shadow, duotone, background image).Testing Instructions
🤖 Generated with Claude Code