Fix center aligned images styles #39422
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] Bug
youknowriad
requested review from
TimothyBJacobs and
spacedmonkey
as code owners
youknowriad
commented
Mar 14, 2022
| $style .= "$selector .alignleft { float: left; margin-inline-start: 0; margin-inline-end: 2em; }"; | ||
| $style .= "$selector .alignright { float: right; margin-inline-start: 2em; margin-inline-end: 0; }"; | ||
| $style .= "$selector > .alignleft { float: left; margin-inline-start: 0; margin-inline-end: 2em; }"; | ||
| $style .= "$selector > .alignright { float: right; margin-inline-start: 2em; margin-inline-end: 0; }"; |
There was a problem hiding this comment.
I updated the the two selectors above and added the > there to match the editor. For me that's the right thing to do, it was just something that has been missed originally. We don't intended to define styles for any grand children here, only direct children. (Technically it's not related to this PR but I thought I'd just fix it here)
|
Size Change: +2.83 kB (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
|
This is testing well for me. Here's a fresh block theme: Here's an older block theme that hasn't been updated in a while: Here's TwentyFourteen and TwentyTwenty classic themes:
|
ocean90
requested changes
Mar 15, 2022
|
@ocean90 I think both of these issues are the same. The column block doesn't output "layout" styles. I think the ideal solution would be for the column block to opt-in into the layout but there might be some side effects to that change. |
|
The last commit solves that. It adds "layout" support to the column block. It also means this panel is visible on the sidebar of the column block
I think that's fine but wanted to confirm with @jasmussen here |
|
I think it's fine as Columns is a container block like group. I do think there's an opportunity to further refine and clarify the layout panel, and also to generally give a lot of love and attention to the Columns block. But those are separate issues. |
|
With the layout setting it's now working for me too. It may can get a bit confusing when seeing three inputs for setting a width though.
Saying that, the editor seems not to respect the width set for the image. Is this a known issue?
|
I'm not sure I understand the issue here, would you be able to send me some content with that? Those widths define the max-width for child blocks but I know that image blocks can grow beyond their container in Gutenberg (on purpose) |
I guess you mean when they are resized by the user? Looks like I had mine resized at some point which was causing this. Inserting an image without resizing respects the max-width. Example content<!-- wp:image {"align":"center","id":14,"width":512,"height":342,"sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image aligncenter size-full is-resized"><img src="https://user-images.githubusercontent.com/617637/158354719-55822d17-9449-46f8-a84f-20df1d5b8de3.jpeg" alt="" class="wp-image-14" width="512" height="342"/><figcaption>r3e3ed3</figcaption></figure>
<!-- /wp:image -->
<!-- wp:group {"backgroundColor":"pale-cyan-blue"} -->
<div class="wp-block-group has-pale-cyan-blue-background-color has-background"><!-- wp:image {"align":"center","id":14,"width":512,"height":342,"sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image aligncenter size-full is-resized"><img src="https://user-images.githubusercontent.com/617637/158354719-55822d17-9449-46f8-a84f-20df1d5b8de3.jpeg" alt="" class="wp-image-14" width="512" height="342"/><figcaption>r3e3ed3</figcaption></figure>
<!-- /wp:image --></div>
<!-- /wp:group -->
<!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column -->
<div class="wp-block-column"><!-- wp:paragraph -->
<p>Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet. </p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->
<!-- wp:column {"width":"200px","backgroundColor":"pale-cyan-blue","layout":{"inherit":false,"contentSize":"100px"}} -->
<div class="wp-block-column has-pale-cyan-blue-background-color has-background" style="flex-basis:200px"><!-- wp:image {"align":"center","id":14,"width":205,"height":137,"sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image aligncenter size-full is-resized"><img src="https://user-images.githubusercontent.com/617637/158354719-55822d17-9449-46f8-a84f-20df1d5b8de3.jpeg" alt="" class="wp-image-14" width="205" height="137"/></figure>
<!-- /wp:image -->
<!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button -->
<div class="wp-block-button"><a class="wp-block-button__link">ffffff</a></div>
<!-- /wp:button --></div>
<!-- /wp:buttons -->
<!-- wp:image {"align":"center","id":14,"sizeSlug":"full","linkDestination":"none"} -->
<figure class="wp-block-image aligncenter size-full"><img src="https://user-images.githubusercontent.com/617637/158354719-55822d17-9449-46f8-a84f-20df1d5b8de3.jpeg" alt="" class="wp-image-14"/></figure>
<!-- /wp:image --></div>
<!-- /wp:column --></div>
<!-- /wp:columns --> |
ocean90
approved these changes
Mar 15, 2022
This was referenced Mar 15, 2022
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.
closes #39291
What?
Centered image blocks inside containers (group blocks) in a block theme were not centered properly. This PR fixes that.
How?
The "flow" (default) layout didn't have explicit styles for centered blocks that said, the default rule targeting all children implicitly targeted the centered blocks and added margin autos. That default rule is mainly there for wide/full aligned blocks. That said, that default rule is conditionally rendered, it's only added if the layout of the current container defines "widths", meaning it supports wide and full widths and has a centered content area.
So the PR defines an explicit rule adding the margins for centered aligned blocks.
Testing Instructions
1- Insert a group block
2- Insert an image block inside that group block
3- Resize the image so it's small
4- Center the image block
5- It should appear centered on both frontend and editor.
Ideally we should test on different themes (classic and block themes)