Skip to content
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

Styles: show a hierarchy of styles clearly (theme, user, global unspecific, global specific, local, etc) #49278

Open
4 tasks
Tracked by #41232
annezazu opened this issue Mar 22, 2023 · 25 comments
Assignees
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.

Comments

@annezazu
Copy link
Contributor

annezazu commented Mar 22, 2023

When no styles are detected

No styles detected

When styles cannot be detected (from a style provider like global styles / theme.json) no controls are displayed. This eliminates the confusion between undetectable inherited styles (e.g. custom css, browser styles) and empty/unset controls. See #43082 for additional context.

Working globally

Working globally

Respecting the convention above, panels are empty until a value is set (either in the UI, or via theme.json).

Working locally

When working locally there are three states to consider:

Inherited values
Working locally

If a value is inherited (e.g. from global styles) the control displays that value, and the label has a purple accent denoting the global context. Hovering/focussing the label reveals the source in a tooltip. The control cannot be removed because if there's no local setting, the inherited value will be used.

Local overrides
Local overrides

If an inherited value is changed then a blue dot is appended to the label. Clicking the dot reveals a menu where the user can reset to the global default, or push the value upstream. Some controls (like background image, see below) may need an additional action to 'unset' the value locally.

Local styles
Styles can be added to a block in the local context that have no inherited value. The manifestation of this in the UI is essentially the same as the "Working globally" case above:

Working globally

With one caveat: If a global style for this property is later added, the local control would then exhibit the blue dot.

Examples

In addition to the block-spacing example above, here are a couple others that have come up in other issues recently. In each case the edit context is local to a document:

Lightbox
Lightbox

Background image
Background image

Breakdown

This doesn't all need to be implemented at once. The work could be broken down into the following steps:

Important note

The purple accents used to indicate inherited styles in the mockups above could be a bit heavy-handed in practise. We should continue to explore other design solutions for this.

Original issueKnowing what style is affecting something on a theme is difficult (is it a global element? is it coming from a block type? is it local to the template?). This has shown up in a few areas already, from Custom CSS https://github.com//issues/47946 to the general experience of conflicting styles https://github.com//issues/45086 https://github.com//issues/44931. To improve this, we should find a way to show a hierarchy of styles clearly (theme, user, global unspecific, global specific, local, etc). While building this, we should also consider ways to push or transfer something to the right level when working through a design, which relates to https://github.com//issues/44931

Image

In most cases understanding this style hierarchy is not crucial, as you can accomplish most of what you need to without the visualization. You go to global styles to style site wide, you edit pages to style locally. Nevertheless for more complex sessions, it can be useful to surface, but due to the rarity of needing to surface this, the footprint and UI weight of any visualization feature needs to be small.

Here's a take that makes the design tool label clickable to reveal the inheritance:

Image

  • Click a label to see the inheritance
  • If the popover has a link, that link should serve as a deep-link to the origin of the inheritance

For this concept to work, the Color flyout has received its own label.

Thoughts?

@annezazu annezazu added Needs Design Needs design efforts. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") labels Mar 22, 2023
@jasmussen
Copy link
Contributor

As an example of style inheritance reflected in a few ways:

  • If no font is set in global styles, it inherits Times New Roman from the browser stylesheet (though I think we may have overridden that to system fonts)
  • If "Crimson Pro" is set as a global font, then a group inherits that. But a group can also further set its own font inside, such as DM Sans.
  • Subsequent nested groups could keep at this, creating a deep inheritance hierarchy, i.e. Group ← Group ← Group ← Global Styles ← Browse styles.

The added nuance here is that in the block editor, we are talking not just about CSS inheritance, but also about global styles inheritance, such as inherited content/wide widths. And even further, you might set a block default in Global Styles > Blocks, and then override it locally in a post. In the future you might even set ref'ed colors (Set the background color of this button to the same color as the global text color).

Finally, there's theme inheritance. You might activate a block theme that has a default root padding. You might then create your own style variation of that, which has its own root padding. It needs here to be visualized that your style variation is replacing root padding values that exist in theme.json.

Sharing these examples as context for a design that tackles this, that ideally such a design can tackle these, while still being minimalistic, and perhap ideally with a way to navigate to each origin.

Related:

@jasmussen
Copy link
Contributor

I ended up creating #50659 instead of updating this one. I'm happy to close the other one and use this one? Let me know.

@annezazu
Copy link
Contributor Author

It might seem silly but I've linked to this one a fair amount. Mind closing yours and sharing the designs here?

@jasmussen jasmussen added Needs Design Feedback Needs general design feedback. and removed Needs Design Needs design efforts. labels May 22, 2023
@jasmussen
Copy link
Contributor

Not at all, the other one was created by mistake! Updated this one. Thanks Anne!

@annezazu
Copy link
Contributor Author

annezazu commented Jun 5, 2023

Passing along feedback from the FSE Outreach Program's Rapid Revamp call for testing that underscores how painful this currently is:

when I changed the size of the header type h2, I hoped that the titles in the Query Block changed too, but it didn’t change for me. Then, I think that the Post Title block into the Query Block had a custom size, but it had the default size. I reviewed the Blocks in Styles section, but it had the default size too. Same issue with the excerpt, when I change the Text size, don’t change in the excerpt. Also occur with Appearance and Line Height. The Font Family works fine, previewing the changes. Inspecting the front end, I view that get defaults values in the Query Block. For example, in the title have .wp-block-query h2 style. It’s confused for me. I think that should be got the default h2 styles, except if it is customized.

@jasmussen jasmussen added Needs Dev Ready for, and needs developer efforts and removed Needs Design Feedback Needs general design feedback. labels Jun 6, 2023
@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Jul 26, 2023
@jameskoster
Copy link
Contributor

One thing that has been historically challenging to communicate is the presence of default values. The label treatment suggested here effectively handles this in a way that can be applied consistently across all controls. With that said there may be an argument to increase the prominence slightly by making use of the purple 'sync' color to help indicate there's a connection beyond the local scope.

Here's a quick mockup:

Inherited + local styles

@jasmussen
Copy link
Contributor

With that said there may be an argument to increase the prominence slightly by making use of the purple 'sync' color to help indicate there's a connection beyond the local scope.

I see where you're coming from, but I worry that when you see a full editor with a canvas, a top toolbar, and a taller inspector, and end up with a bunch of purple splotches, it'll inevitably add some weight to what is, truly, a power-user feature. In that light, I would not highlight items inside an itemgroup — I'd still keep it label-specific (you could still click a color item, and we could add a label you can hover in the flyout).

I wonder, can we add some purple on hover only? I.e can we still have a more muted treatment of the resting state? (Perhaps it's still my recent AFK that's in my blood but coming back to this I'm even thinking that the dashed underline is too prominent here 🤔 )

@jameskoster
Copy link
Contributor

I wonder, can we add some purple on hover only? I.e can we still have a more muted treatment of the resting state?

Yup, definitely something to explore, if we think the general heuristic makes sense. In that regard, we may consider the opposite mechanic: instead of highlighting controls that are set to their default values, highlight ones that are not, perhaps with a blue dot or something similar. This could provide a direct pathway to resetting individual values too:

Screenshot 2023-07-31 at 10 59 13

@jasmussen
Copy link
Contributor

highlight ones that are not, perhaps with a blue dot or something similar. This could provide a direct pathway to resetting individual values too:

Oh that's interesting. So anything that's inherited, regardless of origin, shows programmatically detected values, but anything else have a dot. How would the slider in your example look, in either state?

  • Inherited from global styles
  • Locally overridden

@jameskoster
Copy link
Contributor

These are the states I think we need to account for:

  • Inherited styles that cannot be detected programatically, e.g. custom css or browser styles (this is basically Design tools: Better handling of unset / inherited values #43082).
    • Local styles that override the above. An example of this would be styling a block in the global styles panel. Another would be adding padding to a Group while editing a post when there is no padding assigned to Group blocks in global styles.
  • Inherited styles that can be detected.
    • Local styles that overrides the above. E.g. locally styling a block while editing a post.

The second point(s) is the one that is most important to communicate to users.

Here's a comparison of the approaches above:

Highlighting local overrides

blue-dot

Highlighting inherited defaults from style provider

purple

@jasmussen
Copy link
Contributor

I think this might just work. @SaxonF @richtabor any thoughts, any nuance we're missing here?

@SaxonF
Copy link
Contributor

SaxonF commented Aug 3, 2023

I like the dot to indicate which properties have been overridden. We can use a similar pattern when making changes to a style variation in a similar way to keynote. When someone pushes local styles to global those dots just disappear which is another good bit of feedback.

@youknowriad
Copy link
Contributor

One downside of the design proposed above is that if the default is value "3" and I want to use the value "3" for my block style/setting/whatever explicitly. I mean I don't want that value to change if I switch themes or styles variations. How do I do that? I just "focus" the input, I change back and forth. It's a bit weird.

@jameskoster
Copy link
Contributor

It's a bit weird, but I don't think it's too bad. You have to perform a similar action if you want to replace a global value in Figma and it works okay:

radius

As a secondary affordance we might offer an action to detach/unlink from the inherited value in a single click. To connect a dot, such an action has already been suggested for the box shadow control for customising presets: #44651.

@youknowriad
Copy link
Contributor

Works for me, just wanted to mention it so it's something we're aware of and making a conscious decision about it.

@jameskoster
Copy link
Contributor

Testing the proposed design against the lightbox control, and trying a single-click detach affordance:

lightbox

I'm not entirely sure we need it — in most cases I suspect it's probably easier to just tweak the control. Still, it's a good one to have up the sleeve, but we'd probably need more intuitive language than 'detach'.

@youknowriad
Copy link
Contributor

@jameskoster I also suspect that we have to keep the button separate from the label of the control for a11y reasons.

@jameskoster
Copy link
Contributor

I updated the OP to include detailed versions of the designs as discussed throughout the issue. We can (and should) continue to discuss, but hopefully it's ready to be picked up for development.

@michalczaplinski
Copy link
Contributor

I've created a very first and very small attempt to tackle the issue of showing whether a particular value is inherited or not and to validate the approach. It only works for the text color at the moment:

#55952

@annezazu annezazu removed the Needs Dev Ready for, and needs developer efforts label Nov 15, 2023
@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Nov 16, 2023
@afercia
Copy link
Contributor

afercia commented Nov 16, 2023

@SergeyBiryukov, @carolinan and myself quickly looked into this issue this morning. Chimin in to quickly note that any informration about the 'inherited' style/setting must be provided in an accessible way.

Looking at this mockup, it seems less than ideal:

inherited feedback
  • Purple color: it would be only a color change, which is not sufficient to provide the information in an accessible way.
  • The icon is a little better, at least it's a clear visual indication. Not sure the 'unlink' icon is appropriate though. Still not ideal though, because icon-only controls come with inherent usability and accessibility issues and should be avoided.
  • Tooltips should not be used for descriptions. The original intent of tooltips is to visually expose the accessible name of an icon-only control. Tooltips usage should be limited to that purpose.
  • The blue dot would be an insufficient indication. We already tried a blue dot in the Templates list a while ago and removed it in favor of visible text.

Ideally, for better clarity for all users and better accessibility, visible text is always preferable. It could be something along these lines:

Inherited from global styles/settings + an UI control to disable inheritance.

@jasmussen
Copy link
Contributor

As an update, #37752 is in a place where it can serve as a good first step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Status] In Progress Tracking issues with work in progress [Type] Enhancement A suggestion for improvement.
Projects
Status: Later
Development

Successfully merging a pull request may close this issue.

8 participants