Button: Adopt support for border color, style, and width #44574
Conversation
aaronrobertshaw
added
[Type] Enhancement
| @@ -79,3 +79,40 @@ div[data-type="core/button"] { | |||
| .editor-styles-wrapper .wp-block-button[style*="text-decoration"] .wp-block-button__link { | |||
| text-decoration: inherit; | |||
| } | |||
|
|
|||
| .editor-styles-wrapper .wp-block-button .wp-block-button__link { | |||
There was a problem hiding this comment.
I'm not a huge fan of having to add this much CSS to provide defaults that can overcome styles in core. If anyone has any ideas on a better approach, I'd be happy to evolve this one.
Without these tweaks the user has to set multiple fields before a border shows up. Even now the default border color is currentColor so when the background is white and the button text is white, the border isn't really visible.
There was a problem hiding this comment.
It doesn't seem like we need to replicate this in editor.scss and styles.scss - if I removed this section from here the CSS from styles.scss seemed to be applied in the editor and the frontend? I did a fresh build and a hard reload to make sure it wasn't just a case of the editor styles being cached.
There was a problem hiding this comment.
We do need to add this CSS to overcome core's theme.json button element styles which reset border-width to 0.
The theme.json styles get the .editor-styles-wrapper class prefixed to their selectors in the editor. As a result, those styles override the styles in the frontend style.scss.
Without these styles, a user needs to adjust the border width before they can see any border.
To witness this behaviour:
- Delete or disable the styles here in the editor.scss file
- Make sure you don't have any
core/buttonstyles in your theme.json - Edit a post and add a button
- Select the button and select only a border color
- Note that there is no visible border due to the core theme.json border width reset for button elements.
Screen.Recording.2023-02-07.at.3.13.43.pm.mp4
There was a problem hiding this comment.
Thanks for the extra detail - I was adding the border width and then the color so wasn't picking this up - if I had read back through the old comments I would have picked all that up - sorry for wasting your time here!
I have modified the comment to specify this detail like the one in style.scss does.
|
Size Change: +1.83 kB (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
|
This is testing pretty well for me. Will have a think about alternatives to all those styles. Do you think we need to worry about hover styles overriding the borders? button-hover.mp4the above is on the Blockbase theme |
Good catch, thanks. I'd say we do. Regarding theme.json and Global Styles, we might be able to leverage feature-level selectors to enforce the user's border selection. I don't think we have a means of styling the hover state of individual buttons at this point, so the block might be restricted to at least maintaining the same border. We could extend the new styles further to enforce the border on hover when a border color, style or width are set. If we come up with a solution to more elegantly override the Element API provided button styles that might also provide a clue to handling pseudo selectors and hover/focus states set by theme.json. |
For me with all those styles removed it seems to work as expected as long as a border width is set, which makes sense to me, ie. I would not expect border color and border radius to be visible at all until a width is specified, ie. this flow makes sense to me: border-color.mp4 |
I'll have to dig it up from the original border support PRs but this flow wasn't desired. Too many clicks and users expect to see visual feedback when they change a control. |
In this instance this might be a |
|
There's a lot of history around the border support and controls but here's a couple of links to where the application of a style and width when the user initially selects a color were raised.
Another factor to consider here, if we remove the styles as suggested, is that this block now behaves differently from others with border support. |
yeh, if it has already been well discussed and decided on then this probably isn't the PR to be unwinding it all on |
|
Thanks for the sanity check though. It is good to challenge past decisions and see how strongly they hold. I agree this might not be the PR to wind it back. I'll revisit this PR next week to see what we might be able to do about the pseudo-selector styles. |
aaronrobertshaw
force-pushed
the
add/button-border-support
branch
from
a2564e7
to
0b3c58c
Compare
|
What is the status of this PR - is it ready for another test after you additional border supports? |
The border-width: 0; set by the button element through the Elements API interferes with the UX of adding a border. It means we are back to the situation needing to set multiple fields before a border shows if we don't override it.
aaronrobertshaw
force-pushed
the
add/button-border-support
branch
from
0b3c58c
to
58e488c
Compare
Thanks for checking in on this one @glendaviesnz 👍 I've rebased it and tested applying borders through theme.json or at the individual block level. It's all testing pretty well for me. If you could give it another once over, then I think we can land this pending your approval. |
|
Flaky tests detected in 4ca13f0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4118026664
|
|
It looks some of the core defaults (hover, focus) are still taking priority over the button-defaults.mp4this is with the theme.json settings suggested in the description. I can take a closer look at this tomorrow if you don't have chance to look at it. |
|
Thanks for reviewing @glendaviesnz 👍
I'm having trouble replicating the behaviour in your demo above. I'll continue to dig into it though. Using TwentyTwentyThree, here is what I see: Screen.Recording.2023-02-07.at.2.43.38.pm.mp4In the meantime, it might also be worth noting that we can probably use the elements pseudo selectors within theme.json to configure the "core/button": {
"border": {
"radius": "0",
"color": "fuchsia",
"style": "solid",
"width": "1em"
},
"elements": {
"link": {
":hover": {
"border": {
"color": "green"
}
}
}
}
}, |
@glendaviesnz If you can take a further look into the behaviour you were seeing that would be great thanks. I can't replicate any core styles overriding the button theme.json styles. At most, I noticed some themes that add their own styles with higher specificity than theme.json, overriding styles e.g. blockbase. Apologies if I'm missing something obvious here 🤔 |
On further testing it was actually some higher specificity theme overrides causing this, which needs to be sorted at the theme end rather than in the block supports I think. |
What?
Adopts border color, style, and width, supports for the Button block.
Why?
How?
block.jsonTesting Instructions
blockssection. (See snippet below)Buttonsblock and a few buttons within it. They should reflect the styles in your theme.jsonExample theme.json snippet:
Screenshots or screencast
Screen.Recording.2022-09-29.at.8.52.17.pm.mp4