Components: Add extra specific CSS #30205
Conversation
| width: revert; | ||
| -webkit-appearance: none; | ||
| word-break: revert; | ||
| word-spacing: revert; |
There was a problem hiding this comment.
can we separate the "revert" styles from the styles that are actually useful to style the button and add a comment explaining why all the reverts are necessary?
There was a problem hiding this comment.
I actually I feel we should remove the "revert" styles entirely from here and have them in the "reset.css" (I added them here #30176 ). I think if you combine these two PRs, these reverts are not needed anymore here.
There was a problem hiding this comment.
(still hesitant on this, after landing #30176 we'll get more clarity)
|
The classic block looks pretty funky :D
|
|
@MaggieCabrera This PR doesn't address the classic block at all because these buttons are not |
|
Size Change: +7.65 kB (+1%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
|
The search block buttons seem to be affected too when they have icons (this happens with just the gutenberg changes, not the PR on empty theme):
|
Yeah, I know, but I'm wondering if we could include them on this PR too (we considered them for #29229 by using the |
|
Really appreciate the PR, it's been a headache for too long, and I'm excited to see a fix land. I also think that in absence of (or awaiting) Shadow DOM, it is the right solution to increase specificity of these rules so they are harder to override. In that vein, a selector like Riad left some good comments, he's been deeper in this for the past few weeks than I have, so I'll defer to him. But thanks for the work! |
This appears to be because the search block uses the button component rather than a generic button, when the icon is used:
I've seen a few other blocks do this, presumably because the components are easier to use than to use the vanilla elements. But it still seems like the wrong thing to do, becasue it diverges the markup in the editor, from that of the frontend (where the markup is simpler, just What's the best way to discourage using the components in this way? |
That selector is a bit too broad and does mean "editor canvas", should we replace it with |
Funny thing is that if you use directly |
youknowriad
force-pushed
the
add/component-button-styles
branch
from
29545ea to
e642c0d
Compare
|
I think this PR is ready. The search block is still affected but shouldn't be once #30176 is applied on top of this one. |
|
Can we get some testing here? |
|
This is really nice, and a good holdover until better things. When I just style
My rules are overridden as they should:
However, if I add extra specificity by targetting
Specifically:
that is, my padding, appearance, font family, font size. Background color does not win out, even though it looks as if it does, in the above, because a color is explicitly defined above. I would think both font size, family and padding are all defined as generic The easiest things we can do to increase specificity further (because I would think the |
|
I increased the specificity, how this does look? |
|
The problem I noted looks solved, but I'm sorry I failed to test the Cover image when I suggested that specificity, those now look off:
So it appears they need higher specificity as well. In testing all the placeholders, most looked okay, but there were a few issues in addition to cover. Group is now sans border and not centered:
Columns has an off padding on the buttons.
|
|
This PR could probably also use a rebase, as I know that both the Navigation and Site Logo blocks have recently received updated placeholder states, and those changes weren't visible in this branch. |
|
I'm starting to think that this PR might not be the right approach :( because it forces us to add higher specificity to basically all the buttons that we use in the canvas, (all the appenders, all the buttons inside placeholders) and there's a lot of selectors to upgrade in our codebase. |
|
Yeah, it's tricky :( |
Is there a reason this is a problem besides the amount of work? |
|
my main concern is the fragility of the PR. It something that could break very easily as we iterate on the UI... |
|
What would be needed to unblock fixing the issue — iteration on this solution, or completely different approach? |
|
A thought: IE11 no longer has to be supported. Should we actually look into wrapping these items in Shadow DOM containers? |
|
Would converting button CSS-in-JS (#26340) solve this problem, or at least allow for a less hack-y approach to solving it? |
annezazu
added
[Feature] UI Components
|
Closing in favour of #33494 |
closes #10178
Description
Adds extra specific CSS to component buttons so that themes don't override it.
How has this been tested?
Screenshots
Checklist: