Block Controls: add None option for text decoration #31768
Conversation
creativecoder
added
[Package] Block editor
packages/icons/src/index.js
Outdated
| @@ -123,6 +123,7 @@ export { default as moreHorizontalMobile } from './library/more-horizontal-mobil | |||
| export { default as moreVertical } from './library/more-vertical'; | |||
| export { default as moveTo } from './library/move-to'; | |||
| export { default as navigation } from './library/navigation'; | |||
| export { default as none } from './library/none'; | |||
There was a problem hiding this comment.
Alternatively this could be named for the icon form, rather than it's purpose, e.g. shortDash. Any recommendations @jasmussen @jameskoster?
There was a problem hiding this comment.
Is this the same icon we're using the the solid border style? cc @aaronrobertshaw :D
This could be seen as an antonym to the "Add" icon (+). So perhaps "Remove", "Subtract", "Minus" ?
There was a problem hiding this comment.
I took the remove icon from Icons / Editor (it's called minus in the icons package) and shortened it from 12 px to 8 px. The minus icon seemed a little wide to me, as is.
Here's a figma link.
|
Size Change: -17.9 kB (-1%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
|
Thanks for your work. The key problem, to reiterate, is to allow a design control to untoggle a style applied by a theme. This is different from a default, which it is in Figma. In Figma, "none" is toggled until you choose one of the other options. In this implementation, none of the controls would be enabled by default — the "none" option is one you toggle when you discover that the theme has applied an underline to a heading, title, link or something else, and you want something else. I surface this as it's pretty key to getting the interface, as well as the user interface right. And it adds some followup questions:
The feature itself makes sense to me, even from a usability or accessibility viewpoint, because it gives control to the user over themes that might be inaccessible:
So I'm mostly thinking in terms of a good design for this. |
I'm curious about how all this works under the hood. Obviously there are two ways to "untoggle a style" with css:
Option one is a blunt instrument, but can be required in some cases. Option two is cleaner, but leaves the block open to the css cascade which could have unpredictable results (e.g. removing the text-decoration control, but the text remains underlined). In order to accomodate end users who want to over-ride a theme setting, and themers who want to build clean themes in the site editor, don't we need to allow for both? If that's the case, then having On reflection I think the dash actually works well as an icon because it caters to both scenarios:
|
I understand the use case, it seems valid. I think the primary challenge I have especially with comparing to figma is that due to the cascade we don't necessarily have a default, where Figma does. I say we don't necessarily have a default, because I'm unsure whether the Global Styles system can help there:
2 I like, but don't think we can do (but hope to be corrected). One, especially the "no visible effect" is the one that has me pause. |
Ah, I understand the (my) confusion now... I'm working on the assumption that the buttons behave like a segmented control. IE one of them must be selected, and it is not possible to toggle them all off, or toggle combinations on. The value is either If that's not the case then I agree, having a So I suppose it's a question of whether we want to allow combinations of decorations. Should it be possible to apply both linethrough and underline? 🤔 If so, how far do we take it... there is |
With vanilla CSS (using purely To me the question mostly boils down to: are we able to know from |
I thought so too, but TIL: https://www.w3schools.com/cssref/tryit.asp?filename=trycss_text-decoration2 🙈
I don't think that's quite right. You're actually specifying I made a quick figma prototype to test how this feels as a segmented control. |
Whoah brain explosion! That's ... awesome?
Right, there's nuance. I also agree that the use case seems valid enough, it's nice for a user to be able to remove an underline on a theme they otherwise love. It may be worth bringing up an inconvenient truth: so long as CSS is allowed, themes can write CSS that our controls can't touch. For example this one: That's a deliberately extreme example. But it's an example of a CSS rule that would almost always win, even if you explicitly pressed a "none" button in the UI. And that brings me back to the are we able to know from |
I did some investigation on this yesterday. The theme.json sets global styles. I don't see a way to access global style settings from within the block typography components the way things are structured now. The global styles context and provider are contained within the Note that
Regarding the UI these are really great points: the main confusion seems to be differentiating between the "default" state set by global styles and inherited by the block, from a "none" setting for the block that overrides global styles. Maybe that could be represented like this Default (inherit from global styles)
None (block setting of
I pushed an update to the branch that works this way. This brings the toggles close to how the dropdown settings work, with a "default" setting at the top, but also allows you to unset that default. I'm not sure if it's better than a 4 way segmented control where one option must be selected 🤔. |
Yes. We have |
|
Looks like this was taken care of in #44067 |
Description
text-decoration: nonewhen all toggles are turned offThis option is needed for overriding styles that a theme sets for a block.
Uses a new icon, as suggested here. Figma link
How has this been tested?
To test overriding a theme, insert block settings in the theme.json of your theme, which will default an underline style to the navigation:
{ "styles": { "blocks": { "core/navigation": { "typography": { "textDecoration": "underline" } } } } }Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.jsfiles for terms that need renaming or removal).