Add a box-sizing border-box to lists #39895
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
[Type] Enhancement
|
Size Change: +72 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
|
Yeah, I think this is fair and I believe when we talked about adding a blanket rule for the whole site we decided against it in favor of adding them block by block should it be needed and this seems clearly the case. I'll test in a few themes tomorrow in any case. |
youknowriad
changed the title
MaggieCabrera
approved these changes
Mar 31, 2022
youknowriad
added
the
Needs Dev Note
37 tasks
|
Cool, just as you acknowledged it would, this broke one of my client's publicly facing website's styling. Why was it necessary to do this? The theme in question uses a very popular reset method: and then uses a section element to scope to box-sizing:content for that whole component. Your change is too global, and very inconsiderate. |
|
Hey @WraithKenny I'm sorry this broke your theme. Unfortunately, this was necessary as it's a bug fix. Without any theme provided styles, the list block by default was not aligned properly (its width grew) if you just apply a random background. |
|
This could've probably been scoped, by using the same inherit technique and setting border-box to an appropriate class. It's almost never necessary to inject a global style. Besides, there's countless "bugs" like that that have been ignored so far; hopefully, you don't resort to this kind of fix for all of them. |
|
Indeed, but the list block doesn't have a dedicated classname, and the bug is not specific to backgrounds, it's a bug that shows up every time you add padding to a list without any scoped selector. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
I noticed that when you apply padding to lists (whether via theme.json or by just picking a background for the list block), the width of the block is impacted and the block becomes unaligned with the rest of the content.
That's because the list block is missing
box-sizing: border-box;CSS rule. This PR adds this PR to the default style of lists.This can be seen as a breaking change for themes as it might impact all of them. That said, I expect that most themes already do
box-sizing: border-boxfor most of their elements. So I think the impact will be small and mostly bug fixes but I'd like to know what you all think.Also, right now the list block doesn't support "padding" block support but if we decide to add at some point, the bug will be even more visible, so might as well fix it now.
Testing Instructions
1- Add a list block
2- Choose a background color
3- Notice that the list block doesn't "grow", it's aligned with the rest of the content.
cc @WordPress/theme-team @WordPress/block-themers