-
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
Try: Post content: Add clearfix #63690
Conversation
Add CSS to clear floated items inside the post content block.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +181 B (+0.01%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
Pinging @WordPress/block-themers for review. I don't want to break any legit use cases, but I can't think of any where adding a clear to the post content block will break existing, deliberate designs. |
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.
I'm giving my approval here because I can't think of cases where we would be breaking people's sites. I tagged @jasmussen in case he can think of something we couldn't. In any case I think this is a good one to merge early and let it in the open so it can be tested, and it's an easy revert anyway
This is working fine in my testing and no odd side effects that I can infer. ✅ One small nit-pick for me is the testing instructions:
I don't know why we're emphatically clarifying it be a page. We may be aiming to simplify the testing instructions, but posts will also be impacted. I tested a custom single post template in the Frost theme, and the fix still addresses the issue. 👍 I do not want to mislead any new tester/contributor to only focus on pages. Also, I encourage us to test the header/footer (template parts in general) to come up with a plan, too, assuming we want to continue with the proactive float defense strategy. Of course, this would be a follow-up issue and should not hinder merging the work proposed here. Thanks for the wonderful work here @carolinan |
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! 🫶
Because these two example themes already clears the post content block in the template for the single post. So testing it on the single post would not have shown that the problem was solved by the PR. |
Yes, this is what I found in my testing of Twenty-Something themes. However, let’s not encourage folks to test solely against Twenty-Something themes and allow for robust testing against as many scenarios as possible for the best coverage. A user could create a new post template in the Site Editor and remove everything after the Post Content. It would likely be a “power” user, but it is possible nonetheless. |
That the existing templates in these themes work differently is also explained in the issue. I am not going to choose to instruct a tester to create their own custom theme, or install a third party theme, if the themes that are already included in WordPress can be used to replicate the issue and the solution. |
The use case here seems valid enough, and the fix is bog standard. It's also a good time in the cycle to try this: early, so we can get plugin feedback. I wish there was a better fix than the pseudo selector one, though: while we aim to reduce the use of pseudo selectors in the editor, they are still occasionally used for showing some UI, like focus styles or selected states. But I think that's the "before" selector, but something to check. |
Yeah neither alternatives, including the one used, are perfect. :( |
This may conflict with the block selection ring in the editor, which uses the same
Although, the gutenberg/packages/base-styles/_mixins.scss Lines 597 to 606 in 98c65ea
So if the |
@wongjn Thanks for the report. Certainly, this PR conflicts with the block's selection ring. Clearfix enabled: Clearfix disabled: |
Hey @carolinan 👋 Would you be able to help write a dev note for this for the 6.7 release? We are planning to have this as part of a larger Miscellaneous Editor Updates note. We are hoping to get all drafts in by October 13th to leave some time for reviews before the RC1. All Dev Notes get tracked in #65784 so feel free to leave a note there or ping me directly :) Please let us know if you can assist with that. Thanks in advance :) |
A clearfix has been added to the post content block. Elements at the end of the content that are left- or right-aligned will no longer overflow and be positioned outside the content area. The clearfix was implemented using an
Developers who have already implemented their own clearfix may need to test for duplicate or conflicting code. |
The pseudo-element approach had issues with focus in the editor, so I fixed it in #65364. Instead of the So maybe a dev note isn't necessary. |
@t-hamano Oh OK, thank you! |
What?
This PR adds a clearfix to the post content block, so that left and right aligned blocks in the content do not overflow the content area.
Why?
If the last item in the post content is left or right aligned, it can be incorrectly positioned left or right of the next element, even if it is outside the designated content area. For example to the left of the site footer instead of above it. You can see examples of this in the issue:
#63336
How?
The PR adds a new CSS file to the post content block and adds a clearfix.
Testing Instructions
Activate Twenty Twenty-Four or Twenty Twenty-Three.
Create a new page. (It has to be the page template, not a post)
Add an image and select the left alignment.
Save and view the front of the page.
Notice that the image remains in the "content area".
Screenshots or screencast
Before:
After: