Try: Post content: Add clearfix #63690
Merged
+7
−0
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
Add CSS to clear floated items inside the post content block.
carolinan
added
[Type] Enhancement
|
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. |
MaggieCabrera
approved these changes
Jul 25, 2024
There was a problem hiding this comment.
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
|
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:
|
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?
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:
