Fix image alignment

[pento]

Description

After #4898, there are some bugs in image alignment, because the <figure> element defaults to width: 100%, so WordPress' align* classes have no effect on it.

How Has This Been Tested?

Upload an image smaller than the post width, and change its alignment. View the results on the front end.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code has proper inline documentation.

[pento]

This fix requires testing in IE/Edge. These browsers don't support intrinsic values for width, but I don't know what their current behaviour is.

[youknowriad]

If I understand properly, this fix PR is meant for the frontend, is this really necessary? we just merged #5142 that refactors how float work where the idea was to make floats work in a generic way in the editor not requiring specific handling per block.

Can you explain why do we need this specific handling? right now the block nodes (wp-block-image) are not floated in master but their container is in a generic way to avoid having to do it in each block.

[pento]

On further repo reading, I ran into the same bug as #3945, but it may require more attention to properly address the behaviour that causes #3944.

[jasmussen]

I know Ella and I discussed this also. The biggest challenge with adding an inline style is that it becomes impossible for a theme to override it without employing the !important modifier, which would be nice to avoid.

I've recently been on a refactoring spree to simplify how we use floats. Previously we'd float the block itself, not the image inside the block. To do this we had to use crazy margin hacks to make images appear to be floating when in fact they weren't.

As of #5142, however, we now use vanilla floats, using a simpler trick that allows simple wide and fullwide tricks, as detailed in this codepen https://codepen.io/joen/pen/oEYVXB?editors=1100

Aside from the simplification of markup, this can help address a number of issues we have around floats, which I'll take a look at today.

The reason I mention is that if we are going to add an explicit width to the figure element, perhaps we should add that also to the figcaption element? Reason being, if you are going to use that float trick as detailed in the codepen above, you need to have the same width applied to both the figure and the figcaption, because it's not the figure that's floated, it's the image inside it. See also this for gory details.

But did you rebase properly? I could swear I added a clear: both; to both wide and fullwide images, but I'm seeing this:

But not in master. Edit: it's a bit hard to see what goes on in the above picture, but essentially the wide images is not clearing the floated gallery.

[youknowriad]

I know there have been discussions about whether to add this or not, not sure about the conclusion here though @jasmussen

[jasmussen]

Yep — mostly I'm in the camp of the fewer inline styles, the better, because themes can't override them without using !important. So we should be very careful before we add inline styles like this.

[pento]

@jasmussen: I'm note entirely sure what the issue is you're describing, but this branch definitely includes the changes from #5142.

An alternative option may be to add the align* class to the children of the <figure>, but that seems kind of janky.

[jasmussen]

I'm note entirely sure what the issue is you're describing, but this branch definitely includes the changes from #5142.

Sorry, took a deeper dive and yes, this does not appear to be a float clearing issue at all. But it does appear to be an issue with fit-content — which seems a cool property though I've never heard of it before. Specifically, full-wide images are not fullwide, and the placeholderes aren't either. If the property is necessary, we may need to scope it to floats. See GIF:

An alternative option may be to add the align* class to the children of the

, but that seems kind of janky.

I was also unclear here, sorry. The issue I'm describing is not directly a result of this particular PR, and doesn't need to be addressed as part of this PR. Perhaps there's even a better way.

Here's the markup of a resized, floated image, with a caption:

<figure class="wp-block-image alignleft" style="width:355px">
    <img src="http://gutenberg/wp-content/uploads/2017/10/pic6.jpg" alt="" class="wp-image-313" width="355" height="237" />
    <figcaption>Lovely mountain<br/></figcaption>
</figure>

What I'm asking for, is for the markup to be this instead:

<figure class="wp-block-image alignleft" style="width:355px">
    <img src="http://gutenberg/wp-content/uploads/2017/10/pic6.jpg" alt="" class="wp-image-313" width="355" height="237" />
    <figcaption style="width:355px">Lovely mountain<br/></figcaption>
</figure>

or alternately this:

<figure class="wp-block-image alignleft">
    <img src="http://gutenberg/wp-content/uploads/2017/10/pic6.jpg" alt="" class="wp-image-313" width="355" height="237" />
    <figcaption style="width:355px">Lovely mountain<br/></figcaption>
</figure>

Only for resized images. The key difference being we add the explicit width to the caption as well.

I do understand how that seems sub-optimal — why can't the figcaption just be 100% wide, and inherit its constraints from the parent figure? Well, because it's difficult to create a clean and optimal WordPress layout where both floats and wide images work together. We've spent the longest time looking at good solutions to this, we've been past JavaScript, vw units, and many other things. The solution that has presented itself as the most elegant, is this one: https://codepen.io/joen/pen/oEYVXB?editors=1100

What it does is to not float the figure, at all. The figure retains the width of the main column of content. The contents inside, however, is floated, in this case both the img and the figcaption. When these two child elements are floated, the figure itself collapses into a zero height element, which means any adjecent blocks float around the img and the figcaption. Like this:

But notice how the caption width is now unrelated to the figure, it has to have the same width as theimg in order for this float system to work as intended.

Yet another alternative would be to add still more markup to the Image block. A la this:

<div class="wp-block-image">
    <figure class="alignleft" style="width:355px">
        <img src="http://gutenberg/wp-content/uploads/2017/10/pic6.jpg" alt="" class="wp-image-313" width="355" height="237" />
        <figcaption>Lovely mountain<br/></figcaption>
    </figure>
</div>

Also not ideal. Feel free to chat me up if you need more explanations. I've been thinking about floats for more than a year now.

[jasmussen]

This is definitely an important PR, as it addresses directly the addition of the figure to the image block. It seems like all the max-widths that we have applied to floated images are a result of the figure element too, and if we were to find a way to remove the need for the max-width on unresized images, both #5213 and #4000 would be closed as a result.

[karmatosed]

This is still an issue @pento for the user in the support forums here: https://wordpress.org/support/topic/bug-align-center/. Would be great to work on getting towards a resolution.

[jasmussen]

@karmatosed that issue was fixed separately in last weeks 2.3 release :)

[mariushosting]

Hello guys! No, the bug is not solved. The bug is solved only on admin area but no when you publish the post. As you can see here: https://wordpress.org/support/topic/bug-align-center/
and search the last post, you can see the problem persist and never solved, tested on version 2.3.0 and version 2.4.0
Tested to align this image on version (avocado.jpg) 2.4.0 live website: http://artickey.com/testing-align-center-image-gutenberg-2-4-0/

[jasmussen]

Hi @skynever, thanks for the feedback. Here's a new PR to make sure aligncenter classes are output for the theme also: #5706

[joemcgill]

Hi all – jumping in here with an outside perspective. I'm not up to speed on all of the background for why you've landed on the current markup for images in the image block, but it seems to me like we're reinventing the wheel quite a bit here and opening up some backcompat concerns that we need to be aware of.

Currently, themes and plugins can generally make assumptions about the default markup for images and images with captions in post content since our default markup hasn't changed in forever. As #4898 illustrates, even core makes some assumptions about markup.

As this gets wider use, I expect that we'll discover many more cases where people are either doing a preg_match for images on the_content filter, are expecting particular markup for styling in their theme, or are expecting to be able to modify their image markup using the image_send_to_editor hook, etc.

It may be that the markup changes are warranted — and if so, there's no better time to do that than now — but if it's only going to be moderately different, I wonder if we'd be better sticking with the current markup patterns?

[jasmussen]

@pento I made a new version of your pull request, essentially, here: #5973

@joemcgill wanted to loop in @mtias as he has the deeper insights, but to my understanding the image block comes with new markup that isn't filterable like the old is, and this is an opportunity to look at the semantics again, and in that situation figure and figcaption become natural upgrades to the semantics.

Images inserted in the classic block retain their classic markup.

[joemcgill]

Thanks @jasmussen, trying to take this opportunity to improve semantics seems sensible to me. I think I have two main concerns in this regard:

1. My opinion is that we should do our best to preserve backward compatibly where possible for how themes currently expect to style inline images in content. For example, you should be able to author a post using image alignments with and without captions right now and themes in the .org repo should show those alignments as expected without the need for us to load any additional styles on the front end via Gutenberg.

2. We should avoid, as much as possible, adding any inline styles to the HTML markup and instead leave that responsibility to themes. At present, the only place core adds inline styles to image markup is specifically when using the caption shortcode in order to control the width of the caption in relation to the intrinsic width of the image, by applying an inline width style to the containing div. We attempted to use max-width instead recently and ended up breaking some assumptions with existing themes, so we reverted.

[joemcgill]

Looking into this more, and it looks like the issue that @skynever is having is due to the fact that his theme styles did not account for the change from a div based structure to a figure based structure. Since browsers tend to add top and bottom margins to figure elements, it's not uncommon for themes (like the one in question) to use a CSS reset that does something like:

figure {
	margin: 0;
}

Which, due to CSS specificity, will override the styles that are commonly used in themes for applying center alignment to images, i.e.,:

.aligncenter {
	clear: both;
	display: block;
	margin-left: auto;
	margin-right: auto;
}

This is handled in #5706 by loading additional styles to fix image alignments. I'm of the opinion that WordPress should not be loading extra styles for this. I understand the reasoning articulated in this comment from the PR:

However the change to add a figure around an img is somewhat big, and given how basic an element images are, it would be nice if images, floats and centering did not require theme updates.

...but would suggest that a change in the markup that requires either theme updates or extra styles from WordPress should probably be handled through explicit theme supports option. We have precedence for this, in declaring HTML5 support for captions and galleries.

[jasmussen]

Joe, you are correct, the centering issue is fixed.

By the way if you have time, I would love your input and thoughts on #5973, ❤️

[mariushosting]

Hello @jasmussen image align center is not work in excerpt. So if i set featured image this featured image will be always fixed on the left not center, any idea to solve this?

[pento]

@jasmussen: Awesome, this PR has sadly languished.

Closing, because this PR is old, has merged conflicts, and is superseded by #5973.

[jasmussen]

image align center is not work in excerpt. So if i set featured image this featured image will be always fixed on the left not center, any idea to solve this?

In excerpt?

I'm not completely sure what we are doing differently in Gutenberg with the excerpt. Can you try in the classic editor? Otherwise, feel free to ping me in Slack if you can, then we can work through it https://make.wordpress.org/chat/ — my username there is "@joen"