Border Panel: Combine width and style in panel menu #36942
Conversation
aaronrobertshaw
added
[Type] Enhancement
|
Size Change: +156 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
|
Thanks for the PR and the quick followup. Before we move much further on this, I think we need to align on what it is we mean to accomplish here. Bringing CSS features 1:1 to the editor isn't necessarily the goal, because we can add smarts and systems that helps abstract the complexity of CSS in favor of a better user experience. In this case, it really doesn't feel like we should support the use case of adding only border width, or border style, either in the editor or the site editor. I'm personally curious what the value there would ever be. CC: @apeatling @jameskoster @mtias The way I picture this working is that if you opt into border support, a single control, called "Border", becomes available which inextricably ties the width and the style together. |
There has been some previous discussion around potentially removing border-style entirely as a visible control. It has also been raised in internal discussions that it could be valid for a theme author to wish to prevent a user selecting dashed/dotted borders. The thinking being that it can help avoid bad choices from the user.
When there is a design or clear direction on how that should look and function, I'll be happy to implement it. |
That definitely feels valid 👍 👍 The use case I think we should avoid is a user seeing border style but no border width. If you have style, you should always have width, at the very least. Designwise I don't think it needs to look any different from what it looks like today, so to me it feels more like a technical implementation. |
I think there's probably more value in the panels behaving consistently. |
|
I'm not sure I totally understand the issue. The original concern with users needing to add a non-default control from the ToolsPanel menu just to get a functional border (in the case of Style) makes total sense to me -- that's certainly a bad workflow. Similarly, just making Style and Width both default controls but still requiring users to set two values before they get a functional border also seems problematic. Especially because as others have pointed out, Style seems like a very rarely desired control in the first place; it's generally safe to assume you want a solid border. With Aaron's changes in #33743, the
I agree this would be an odd choice of controls to enable. Testing this scenario on trunk though, if you only select a border style, you still get a functional border with the default I can't seem to find a link, but I believe there's been discussion before about making certain options required for block supports. I suppose that could be an angle we take? |
aaronrobertshaw
force-pushed
the
update/border-support-combine-width-style
branch
from
e121644
to
15d4fd9
Compare
aaronrobertshaw
requested review from
spacedmonkey and
TimothyBJacobs
as code owners
|
This PR has been updated to:
I know it has been raised a few times that being able to conditionally opt into a block support feature would be handy. Is that what you mean? One example of this could be supporting margins on the group block so long as it isn't a top-level block where it's left/right margins would be overwritten by the layouts block support. |
|
I’ve given this a test and it works well according to the PR description, but I'm still not sure I understand why it's necessary in addition to the CSS defaults solution that was merged with #33743. Configuring a block to support style but not width seems unlikely, but even in the event that someone does so those defaults already ensure that you don’t need to update multiple controls to get a working border. For example if you select a ‘dashed’ style but don’t select a width, the Given that, it’s not clear to me what problem linking the controls fixes. It also raises a couple of new concerns:
|
Both of the points you make are valid. Personally, I prefer the status quo with individual controls and no surprises for theme authors/users. My aim with this PR was to make good on a promise to follow up on these controls. If others agree, I'd be happy to close this PR in favour of the way it is on trunk or new ideas if they emerge. Any thoughts @apeatling, @jasmussen, @mtias? |
|
The border control has been difficult, both technically and from an UX point of view. I appreciate your patience on this. The challenge has always been entirely from a UX point of view, that atomizing a border into three distinct pieces brought with it very few upsides over the previous collapsible panel: the ability to clear each individually from the tools panel, and to disable the "style". The downside is a lot of clicking (the next GIF shows what's shipping in trunk):
I want to applaud the effort to make the border appear even without also choosing a style and color. That was a big win even for the classic collapsible panel. To me, this branch is a step in the right direction:
"Style" doesn't feel like an easily understood term. While it maps to the CSS property, it doesn't necessarily translate well to UI controls. While we definitely want to keep theme.json support and the implementation as simple as we can, at the moment it feels like we are optimizing for an edgecase at the cost of splitting up what feels like symbiotic controls. Would the following rename plus behavior tweak be possible?
I know I've been the main voice of concern with borders, and just to boost my own confidence, I really would love to hear from others: @jameskoster @shaunandrews @javierarce @critterverse @mtias. |
|
I don't think surfacing the style control along with the width is a problem. Though one change I would suggest is to set it to solid by default (that seems to be the case on the canvas, but isn't reflected on the control). On occasions where you want a different style, having to add that control manually, in addition to width, is tedious. Especially as – like Joen pointed out – "style" is not an intuitive term, whereas the icons for dotted / dashed are immediately obvious.
This doesn't feel like a situation we should be compromising the UI for. I've said this in other issues, but I think we're allowing the design tools gamut to complicate what could be a much simpler experience. Each of width, style, and color must to be set for the border to apply, and it is easy to set defaults for each, so why not reveal those controls whenever a border is added? I'd love to work towards something like: border.mp4Which I think the PR does. |
|
I've read through this and it's amazing how complicated things can get when you can enable/disable controls, and they are split up by their CSS properties. :) I do agree with @jasmussen that this is ideal assuming default values are set for things that don't appear in the UI:
However, @jasmussen what happens when radius is enabled? You have to have the dropdown menu rather than immediately adding those controls. In this case I assume menu names would be [Width and Style] or [Width] for first two points above? |
@jameskoster I think the issue with removing the dropdown entirely is how does this work when you add radius? Or if a third party comes along and adds an option for border image? It doesn't really scale unless you're going to add everything with one click. |
|
I'm not sure it makes much sense to combine width and style. Border style is mostly irrelevant, and shouldn't be so prominent. At the bare-minimum a border requires a color and a width. As such, I think it makes sense to combine these two options. Now, a block might only support one of these, so the options would have to adjust as needed. I outline most of these thoughts over here: #35602 |
|
@apeatling I'm not totally convinced that radius should be a part of the border panel at all. I think we maybe got a bit caught up with the css property when adding it there. It seems unlikely to me that the average user would look in the "border" section to round the corners of a block. |
This is unfortunately a recurring problem in the block editor, which is that we cannot set default values in the controls because we do not have access to global styles -- so we cannot know that we are not overriding a value that's set in |
|
Can this be solved in the short term by simply updating the configuration of default controls? #34061 adds defaults. We could just update all the blocks that support border to make width and color default controls. If the consensus is that having to click to add style is more of a problem than giving it prominence, we can make it default as well. In the longer term the designs @shaunandrews linked above at #35602 could address the bigger problem of disconnected controls. |
Jay beat me to it: border radius, despite its name seeming to suggest otherwise, affects the shape of the block element instead, with the curve more or less being a side effect. I wish I could've been a fly on the wall when it the property was named, and I wonder if anyone suggested The border panel seems a fine enough place to put the radius, at least until (and if) we find a better place to put it. But simply because of its behavior, it also makes sense to keep it separate. |
Is @jorgefilipecosta's PR still relevant? Add: API to allow blocks to access global styles. It hasn't seen much love recently, but last time I tested it, it was working well. |
It is relevant (to the issue with adding placeholder/default values to the controls)! That's the PR that changes to add the placeholders in #34465 relied upon. It was working quite well, but recent changes to global styles broke the API. I do think it would add great value to be able to populate controls in the block-editor with existing values coming from global styles, in a future iteration. |
|
Thank you everyone for your thought and feedback on these issues. It's appreciated 👍 My plan to move forward is:
The second step above needs implementing as soon as we can. The third will allow us to polish things further keeping the control selections in sync with what is visible. Lastly, implementing the new control designs will require supporting changes in addition to the new control and will take some time. Once 5.9 is released, I can start work on this if the opportunity hasn't presented itself sooner. |
Yes, it is still relevant our priorities changed because of 5.9 but I will soon rebase and try to push the PR forward. |
Related:
Description
This PR:
The desire for combing controls stems from the switch to the
ToolsPanelfor the border block support: #33743 (comment)Notes
How has this been tested?
Block Editor
__experimentalBorder.__experimentalDefaultControlsin block.json"widthAndstyle": "true"--> controls displayed by default"width": "true"--> controls displayed by default if width supported"style": "true"--> controls displayed by default if style supportedGlobal Styles
Screenshots
Types of changes
Enhancement.
Checklist:
*.native.jsfiles for terms that need renaming or removal).