-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Query block: Fix full width children from scrolling horizontally only in the editor #32892
Conversation
Size Change: +71 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions here:
- Why is this only happening in the editor and not the frontend?
- It seems the ul, ol we're targeting here is the
Post Template
block's one right? so why add this bit of CSS to theQuery
block instead?
Yes, the frontend doesn't have that
yeah you are right, I can totally do that |
It's this line that's breaking things in this particular case. |
Do you know which CSS is that? |
So this is what's happening: On the frontend we don't have this or this, we only have the block style adding border-box to the cover and it works as expected. On the editor however, the |
So the issue is in fact in the cover block right? not the post-template? Isn't the right fix here to add |
I totally understand, it's kind of a tricky one! the cover block already has that rule (here) but it's losing the specificity war to this: |
Ok that's clearer now :) I think the ideal fix (which would solve more issues later :P) is to actually apply the same "wrapping" behavior we have for the reset and theme styles to the block library styles. (there's an issue for it somewhere) But obviously, that fix is bigger and more tricky. So I think it's fine to do something like the proposal in this PR in the meantime. One thing I'm wondering though is if it's an issue that is specific to Query block or if it needs to be solved differently. I mean the issue would surface if we insert a Cover inside any other container block that would use the reset rule, isn't it? So do you think maybe a better fix would be to actually add the following css to the
|
I was wondering myself what's the best way to solve this because the "inherit" value that is more specific now is only an issue because of the ul/ol rule that exists that is setting it to "revert". So this is only failing whenever one of this blocks is inside of a list, not just the query block (but I don't know where or if that even happens, but ofc if it doesn't it totally could in the future). It's also the reason why the cover block is perfectly fine when outside the query block. I tried your solution and it solves the specificity issue. Now I'm thinking that we should do this for any other block that can have paddings, such as a group block but I just tried that on my test env and the problem is not happening, so I'm going to debug further. |
Ok, I just applied the fix to the cover block and I think it should be fine for now, unless you can think of any other blocks that could be affected by this. The group block would need this fix too if it were to have a width value applied to it. By its nature it doesn't have it so right now it's working as intended (though the box-sizing value for it is also content-box right now in the same context that is for the cover block) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
We should explore separately, applying the same "wrapping" behavior to the block library stylesheet to have the same relative specificity between stylesheets that we have in the frontend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
01aeb0e
to
b701ce2
Compare
Description
After #30141 was merged, the box-sizing inherit rules for ul and ol are no longer setting them to border-box as they should. This causes the layout to break in the editor if a full width query block with full width children inside when they have horizontal padding inside, causing horizontal scroll. I am hesitating as to where this should be fixed and I'm leaning towards this block fix, but maybe this should be better handled elsewhere.
How has this been tested?
On emptytheme, paste this query loop on a page. Alternatively: create any query loop that is full width and has inside it another block that is full width and has horizontal padding, such as a cover or group block.
The editor will have horizontal scroll while the frontend shows up fine.
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.js
files for terms that need renaming or removal).