Try: Polish code styles to properly apply border properties. #37818
Conversation
jasmussen
added
[Type] Bug
| .wp-block-code { | ||
| border: 1px solid #ccc; | ||
| border-radius: 4px; | ||
| box-sizing: border-box; |
There was a problem hiding this comment.
This property keeps the border on the inside, pushing content inwards.
There was a problem hiding this comment.
I would expect this to be the default for all blocks.. Do you know why we have it in block editor but not in front end? Check the difference in a Heading with margin between the editor/frontend.
Screen.Recording.2022-01-10.at.2.08.14.PM.mov
There was a problem hiding this comment.
Good thought, yes, it makes sense for this to apply to all blocks 🤔
I partially recall conversations around box-sizing being something for the theme to apply if necessary. But if, as is the case here, there's a discrepancy between frontend and editor, that crosses a line for me. CCs at random for thoughts: @jameskoster @MaggieCabrera — could we/should we apply box-sizing: border-box; across all blocks? What might be a good way to do this?
There was a problem hiding this comment.
Generally many themes apply a blanket * { box-sizing: border-box; }. We do so on Blockbase + children and has generally worked well (I don't recall any inconsistencies in blocks due to the rule from the top of my head). I think we tried to add a similar general rule directly in the editor but the consensus was that it was better done on a per-block basis? I'd love it if we could aim for this, but it would probably require some testing. If I can help with this, let me know!
There was a problem hiding this comment.
Good thoughts. I wonder if maybe this is a good time to revisit a consensus. Essentially any block that opts into the border control will have a probably-not-expected effect on the frontend only, meaning if it might start to make sense at a blanket level instead. CC: @ramonjd @aaronrobertshaw since I know you worked on border support. What do you think?
There was a problem hiding this comment.
any block that opts into the border control
Same goes for blocks with padding/margin support.
There was a problem hiding this comment.
We'll have to be careful with things like images, and other blocks that involve wrappers. It might be strange if you set a specific width on an image, then add some padding (which gets applied to the wrapper), and find that the image then renders at a different size to the setting. I ran in to a similar issue with the border controls and the site logo block here.
There was a problem hiding this comment.
Essentially any block that opts into the border control will have a probably-not-expected effect on the frontend only, meaning if it might start to make sense at a blanket level instead. CC: @ramonjd @aaronrobertshaw since I know you worked on border support. What do you think?
I added a bunch of blocks to a post using the Blank Canvas theme and removed the theme's box-sizing rules. Here's the frontend.
Heading and List stand out as blocks that don't take care of their own box-sizing in the absence of any theme rules.
I don't have much context on the background, but I had a poke around and, in the editor, there's a blanket rule for box sizing based on a reset mixin.
Here's how the editor looks with and without this reset rule:
So it makes me think: if we have a default blanket box-sizing rule for blocks in the editor, then it does make sense that they exist on the frontend as well.
On the back of the napkin in a seedy bar after a few 🍺 I'd have written:
- add a low-specificity
box-sizing: border-box;rule, or carry over the reset rule, for blocks on the frontend - see if
box-sizing: border-box;core blocks still need their specific rules - watch things break 🍿
There was a problem hiding this comment.
Excellent thoughts, Jay, I knew there was a good reason it wasn't necessarily simple.
And thanks so much Ramon for testing so thoroughly. Your feedback definitely suggests that either we output the blanket statement on the frontend, or we remove .edit-post-visual-editor from the list to receive the reset, so at least editor and frontend are the same.
The following could potentially provide a soft reset across the frontend:
:where(*) {
box-sizing: border-box;
}
That would give it low specificity, making it trivial to override.
The thing we want to avoid, though, is what Jay is suggesting, the use case where you want the border to be on the outside, but can't do it because of the rule. I suppose that's technically already the case in the editor if not the frontend, but is that a concern?
There was a problem hiding this comment.
I suppose that's technically already the case in the editor if not the frontend
I think is the the crux: we're forcing box-sizing: border-box; in the editor so, if I understand the concern correctly, should we add the reset, we'd be removing the ability to "trick" the editor into showing the border on the outside on the frontend. (?)
Another concern might be that the editor styles rules are rather specific:
Rather than a blanket rule across all elements, would we need to target blocks in a post wrapper element instead?
Anyway, I've thrown up a draft PR if we want to move the discussion over to there.
Thanks for the CSS suggestion @jasmussen !
|
Size Change: +15 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
Thanks for the green check! In light of #37818 (comment), I think I'll remove the box-sizing rule from this PR, then we can look at re-applying it in a different manner separately. |
|
@jasmussen I know I am hopping in here very late, but this change seems to cause conflicts with border styles in theme.json. If you set border styles in theme.json, they are applied to I am happy to continue working on this in a separate PR, but wasn't sure if this issue was already surfaced in your testing. I am also seeing some bizarre extra CSS being added by WordPress that Gutenberg does not seem to be overwriting. 🤔
|
|
Thank you for the ping! I tested in TT2 and things seemed to work well, but it's entirely possible I missed bits and pieces here. I'd be very happy to test and help approve any PRs you can whip up. But I'm confused, though, the purpose of this PR specifically was to wire up so border controls applied. Notice the before image here: Ideally we have consistency between what styles theme.json and the border panel apply, so you could set a default in one and tune it in the other. Can you share a snippet of theme.json styles that are causing conflicts here? Could it be that you are styling the |
|
To add: styling the code element in theme.json is fine to do, and could be an argument to add CSS to the code block |
|
Yeah, theme.json is applying all settings to the This is a little hard to see, but the red border is placed on
I think the root issue is that the theme.json styles are targeting |
Description
Followup to #36282 (comment) and #37816.
A recent change moved the code block border definition to the
codeelement inside thepre. This meant that the border and radius properties you could apply using the inspector did not replace the predefined border, but added another:This PR tweaks and polishes things:
preelement, so they are affected by global styles.currentColorfor the border, so the color would be fully inherited by theme colors, but this was a bigger departure from the visual style that's been shipping for a while, so I held off.How has this been tested?
Insert a code block. Try it in light and dark themes. Try changing the border radius, color, and width. Try styling it using theme.json, for example using this snippet.
Checklist:
*.native.jsfiles for terms that need renaming or removal).