Revisit image floats.

[jasmussen]

This adds a wrapping aside element to any image block that's floated left or right. From the spec, https://www.w3.org/TR/html52/grouping-content.html#the-figure-element:

For content that is only tangentially related, or that serves a separate purpose than the surrounding flow, the aside element should be used (and can itself wrap a figure). For example, a pull quote that repeats content from an article would be more appropriate in an aside than in a figure, because it isn’t part of the content, it’s a repetition of the content for the purposes of enticing readers or highlighting key topics.

The above is the reasoning for using the aside element to wrap the figure. Per subsequent discussion, we're going with a div.

But why wrap the figure at all?

Because due to issues surfaced in #7624 (comment), it seems our current implementation isn't responsive.

The challenge is — what if you float a very small image to the left, and write a giant caption. Even if we apply width: fit-content; on the figure, the caption will expand the figure to accommodate as much text as the parent wrapping element will allow. min-content doesn't work either, because this will make the figure only as wide as the smallest word.

What we have in master works in most cases, through dark magic, but it also only works because we remove the max-width from the nested image. This means the image won't resize with the viewport, and is therefore not responsive.

I have explored so many many options for fixing this, and after all this time, what it boils down to is this:

• We can either set a fixed size on floated captions, say 33%, and hope it works for the image that's floated. Not ideal, and you can see the end result here: https://codepen.io/joen/pen/wXbqwN
• We can add a wrapping element around the figure, so that it and the figcaption can be sized using table rules, as in this PR. See also https://codepen.io/joen/pen/qKGYwy?editors=1100

Why not just set the width on the figure element and float that? Because then we can't accommodate wide images, which rely on an unbounded main column.

It's not ideal that we have to add an extra wrapping element, but it can be semantic, and it feels like the simplest to work with for themers implementing wide images coexisting with floats.

[gziolo]

We will need a deprecation handler for this change. This will make all old blocks having align set to right or left to be recognized as externally modified.

[gziolo]

To avoid code duplication, we can assign figure to its own variable, similar to what is done for img above.

[tofumatt]

I think this can be refactored to not repeat the <figure> code and needs a deprecation handler, but the idea seems fine to me. I think the extra tag is okay; the explanation is solid, cheers! 👍

[tofumatt]

I know it's the style in core and it's unrelated to your patch, but constant-first does my head in… it feels unlike how anyone would speak logically about the conditional. </rant>

This looks like it'll need a deprecated handler because this changes block markup.

[tofumatt]

This looks like it could be a variable defined above, so it's not duplicated.

[tofumatt]

@gziolo and I are twins! 😆

[jasmussen]

Thanks so much for the reviews and examples and thoughts. I think this is a 3.3 milestone thing, in any case it's sort of a sensitive change, which is why I'm going to let it sit for a bit and gather thoughts and opinions. It solves an issue, and I believe it's the best way to solve that issue. But nonetheless I expect there to be valid opinions.

[tofumatt]

I don't follow why it would be sensitive 🤷‍♂️. It fixes a legit problem and doesn't remove any markup, just adds a (valid in this case) wrapper tag which, hey, sometimes you need for flexibility in HTML 😄

[jasmussen]

I took a stab at addressing your comments in 41b3db4, please have a look and see if I did it right.

Also, I realized that this markup change only applies on the frontend. But ideally is reflected in the backend as well. The markup is a bit more verbose in the admin, which hides some of the real world issues themers have to deal with, which is why I didn't catch this the first time round.

In any case, if anyone has bandwidth for helping me with adding the wrapping aside element when an alignment is applied, I'd appreciate any input. It seems I can't output just the <aside> in the beginning, and then the </aside> at the end.

[tofumatt]

I'm having a look now 👀

EDIT: Sorry I got distracted and failed. I'll try looking at this again tomorrow (July 6); feel free to ping me on Slack if you want help or if you've got a handle on it and don't need my help anymore. 😄

[ZebulanStanphill]

I am not certain that <aside> makes sense for all floated images. <aside> should only be used for things that are only tangentially related to the main content and are not actually part of the main content. In many cases, images are a main part of the content even when they are floated, e.g. a diagram used to explain something in a scientific article. I think just <div> should be used; no semantics are better than wrong semantics, in my opinion.

[jasmussen]

I'm back from some vacation ready to look at this. But before I rebase and address semantics feedback (suggestions welcome — div fine?), I'd love some idea on whether we're okay with this. I'm personally very okay and believe it's the only rock solid way to achieve the necessary flexibility for layouts. But I also know it's controversial. @pento any thoughts?

[pento]

I find myself quite excited about the prospect of using <div> for the actual HTML spec reason it exists: as a container element. 🙂

[jasmussen]

Thanks Gary — is that also a way of stating that you would be okay with the added wrapping element when that element is floated?

[pento]

It absolutely is my obtuse way of saying I'm okay with adding a wrapping element. 😉

[afercia]

Aside should be used with caution, and mainly to mark an entire region that is "tangentially related" rather than single pieces of content. The reason is that in ARIA terms, the aside element is mapped to a complementary role which is a landmark role. Landmarks should be used to identify the main regions of a page. Having (potentially) dozens of landmarks in a page would defeat their purpose. (aside: 🙂in core there's the same problem with the widgets).

[jasmussen]

This PR has been rebased, squashed, updated, tested, to fix the remaining issues. I believe it's ready for final review. The small things:

• aside > div
• works in admin
• feedback addressed

Note that the editor markup differs from the frontend, due to a plethora of editor necessary containers, such as one for resizing and so on. I'm not sure we can ever get the backend markup to match the frontend, so instead I've made sure to map the containers necessary for getting floats to behave the same in the admin as the frontend. See d36cce2 for details.

The trick is that when an image is floated, the parent container (block container) retains its same max-width which usually allows a centered column to coexist with wide images. But it is zeroed out in height. The child element, the figure itself, is then floated, which allows the caption to share the same width as the figure, using a table-caption trick. See also this codepen: https://codepen.io/joen/pen/zLWvrW

Note that this PR only touches images. If this is approved and merged in, we'll probably want to backport these changes to other blocks that can float, i.e. video, embed, cover image — any others?.

Screenshots:

[tofumatt]

I chatted with @youknowriad and he let me know the div getting those classes is normal: https://wordpress.slack.com/archives/C02QB2JS7/p1534760407000100?thread_ts=1534753637.000100&cid=C02QB2JS7

So this is all good to go by me. I say we 🚢

Thanks for all your work and the constant back and forth on this one!

[jasmussen]

Okay, I'm merging now. While it's a big change, it's been in progress for a while, it's solid, and it's fixing a number of issues. Thanks for the reviews everyone!

[jcklpe]

@jasmussen Glad to have found this! I've been struggling to find the answer to this question and it's popped sporadically and completely threw off a legacy theme I had developed midway through Gutenberg being released and so it's good to now actually get a concrete bit of information about it.

It has recently popped up again as I am now working on developing a theme for the HeadlessWP+React+SSR framework frontity.

My latest issue I posted in response to that problem.
#18122

I defer to your expertise in this stuff but I'd like to ask a quick question and offer some feedback:

First
"Why not just set the width on the figure element and float that? Because then we can't accommodate wide images, which rely on an unbounded main column."

I'm not sure I totally understand this reasoning. Why would people apply a float to a wide image? Aren't wide images full width etc? So they would wouldn't really be floated right?

Second if a wrapper div is needed, I think it should be consistent across all images. I find it kind of confusing that images are div->figure and then wide images etc are just figure. Consistency in the structure would be nice because there is some CSS that applies to all images that I would like to be able to target, and then could have the variations targeted in a nested fashion (via css-in-js or scss etc) like so:

div.wp-image-block. {
opacity: 50%;
background-color: yellow;
   &.alignleft {
     float: left
   }

   &.fullwidth {
      ...
   }
}

Preferable to me though is just having figure elements but I understand that you guys have already given a lot of thought to this stuff. (I'm also curious how much of some of these problems could be addressed in a flexbox fashion? )

[jasmussen]

@jcklpe Sorry for the headaches this has caused you!

Let me try and answer to the best of my ability!

"Why not just set the width on the figure element and float that? Because then we can't accommodate wide images, which rely on an unbounded main column."

I'm not sure I totally understand this reasoning. Why would people apply a float to a wide image? Aren't wide images full width etc? So they would wouldn't really be floated right?

Yes that phrasing was obtuse. You are correct, a wide and fullwide image will never be floated, so that's not the problem.

The problem is: how do you create a layout that accommodates both wide, fullwide, and floated images? This codepen shows how the block editor solves it.

Traditionally in themes you'd have a centered main column, something a la .site-content { max-width: 800px; margin: 0 auto; }. Anything you'd float inside there would be nicely flush with text as well. But this layout makes it hard to add wide and fullwide images and blocks — you can use negative margins, or vw units and make it work. But this does not work inside the editor because there you have to account for the WordPress navigation and the editor sidebar.

Therefore the approach taken for the editor styles, is one where there isn't a max-width on the main column, instead there are max-widths on every element that goes inside, a la this:

.site-content > *:not( .alignwide ):not( .alignfull ) {
  max-width: 50%;   /* Could be a pixel value too: this is the one that decides the width of the text */
  margin-left: auto;
  margin-right: auto;
}

This allows wide and fullwide images to go from edge to edge without having to use vw units or negative margins, and it constrains blocks to a centered main column.

Floats, as you can imagine, become a challenge, because if you simply float a block, it goes right up against the edge of the viewport, where usually you'd want it to go up against the margin of the perceived main column.

We accomplish this by having a wrapping container element, the div, which allows you to assign a max-width to that container, and then float the figure inside.

Second if a wrapper div is needed, I think it should be consistent across all images. I find it kind of confusing that images are div->figure and then wide images etc are just figure. Consistency in the structure would be nice because there is some CSS that applies to all images that I would like to be able to target, and then could have the variations targeted in a nested fashion (via css-in-js or scss etc) like so:

Yes indeed, it was unfortunate the change had to be made as such. The truth is, any image inserted after the change was made, will have the wrapping div and be consistent going forward. But images inserted before that, don't break because they lack the wrapper. You could re-insert the old images, and they will be updated to the new markup.

It's a headache, I understand, but ultimately the wrapping div was added in order to give more control to themers like yourself. Wide images, floats, figures that are only as wide as the caption inside, and combinations of the three present quite the challenge to solve.

Hope this helps!

[jcklpe]

@jasmussen yes thank you that does help.

Quick question though, for the purposes of my developer self education. I've been until now applying my elements to the perceived main column via margin similar to how you do here:

.site-content > *:not( .alignwide ):not( .alignfull ) {
  max-width: 50%;   /* Could be a pixel value too: this is the one that decides the width of the text */
  margin-left: auto;
  margin-right: auto;
}

What I do instead is something more like this though:

.site-content > *:not( .alignwide ):not( .alignfull ) {
margin: 25px 20vw 0;
}

and then when something is floated right I just do this

.alignright {
float: right;
margin: 25px 20vw 25px 25px;
}

Why not do something like this second example? I see you mention vw being an issue for the editor but you seem to be saying that in the context of full width images right? Or were you saying that this is an issue for floated images in the editor because vw is based on viewport width and in the editor that layout won't be taking into account the more limited space due to the editor and wordpress admin bar etc? Why not simply have separate rules output for the admin versus the frontend? isn't that what is already done in wordpress themes? One can just build separate build outputs based on scss imports right?

At the very least would it be possible to add some kind of class to the top level wp-image-block wrappers that contain an alignedright figure? That way when people want to use vw they can just apply a float to the top level container block and write a second set of styles for their admin view.

[jasmussen]

Why not do something like this second example? I see you mention vw being an issue for the editor but you seem to be saying that in the context of full width images right? Or were you saying that this is an issue for floated images in the editor because vw is based on viewport width and in the editor that layout won't be taking into account the more limited space due to the editor and wordpress admin bar etc?

Yes, the last part exactly.

Consider here that 20vw to the right and left of the paragraph below the image would make for a very thin paragraph.

However you can definitely use vw units for the front-end. What we have learned from user test is that if the editor looks like the frontend, usability is drastically increased, which is why editor styles are very much recommended. However they don't have to be 100% accurate to the front-end. So if you use % in the editor and it looks good, that's totally fine.

A personal goal of mine is to make it so you can load the theme CSS directly into the editor without changes. This is a bit difficult, but there are ways to accomplish that, but it's going to take some time to make this possible.

At the very least would it be possible to add some kind of class to the top level wp-image-block wrappers that contain an alignedright figure? That way when people want to use vw they can just apply a float to the top level container block and write a second set of styles for their admin view.

This layout:

brings with it this markup:

<figure class="alignright size-large is-resized"><img src="..." alt="" class="wp-image-149" width="287" height="192">
    <figcaption>Loem ipsum dolor sit amet, ferri vidisse nam eu, ad nec copiosae mnesarchum vituperatoribus. Te brute dicunt sea, ut vis omnium menandri, ut sumo aliquam has. Eum aperiam interpretaris at, sea et recusabo expetenda, omnis tibique mea no. Pri suas partem ea, ius sonet numquam offendit cu, ad simul admodum pri. Eum cu unum choro albucius.</figcaption>
</figure>

Was this helpful?

[jcklpe]

@jasmussen

However you can definitely use vw units for the front-end. What we have learned from user test is that if the editor looks like the frontend, usability is drastically increased, which is why editor styles are very much recommended. However they don't have to be 100% accurate to the front-end. So if you use % in the editor and it looks good, that's totally fine.

A personal goal of mine is to make it so you can load the theme CSS directly into the editor without changes. This is a bit difficult, but there are ways to accomplish that, but it's going to take some time to make this possible.

That makes sense. Personally I think it's so easy to break out variations in editor/admin styles and frontend styles, and it's so difficult to get files matching 100% between both, and inevitably that stuff is going to get changed by themes anyway, that it seems like preserving markup is more important.

That's just my personal perspective though and a lot of it is informed by the fact that I'm working with WP as a headless CMS so the styling difference between front and backend doesn't even really make a difference to me. I understand that ya'll are operating in the context of the organization and what ya'll feel is it's best interests.

At the very least would it be possible to add some kind of class to the top level wp-image-block wrappers that contain an alignedright figure? That way when people want to use vw they can just apply a float to the top level container block and write a second set of styles for their admin view.

This layout:

brings with it this markup:

<figure class="alignright size-large is-resized"><img src="..." alt="" class="wp-image-149" width="287" height="192">
    <figcaption>Loem ipsum dolor sit amet, ferri vidisse nam eu, ad nec copiosae mnesarchum vituperatoribus. Te brute dicunt sea, ut vis omnium menandri, ut sumo aliquam has. Eum aperiam interpretaris at, sea et recusabo expetenda, omnis tibique mea no. Pri suas partem ea, ius sonet numquam offendit cu, ad simul admodum pri. Eum cu unum choro albucius.</figcaption>
</figure>

Was this helpful?

That's weird because I don't get that markup:

That's a fresh default install of WP with twentynineteen installed.

What I'm asking is if instead of this:

<div class="wp-block-image">
	<figure class="alignright">
		<img src="url" alt="iFly Virtual Reality" data-pagespeed-url-hash="1116608801" onload="pagespeed.CriticalImages.checkImageForCriticality(this);" googl="true">
	</figure>
</div>

we could have this:

<div class="wp-block-image block-alignright">
	<figure class="alignright">
		<img src="url" alt="iFly Virtual Reality" data-pagespeed-url-hash="1116608801" onload="pagespeed.CriticalImages.checkImageForCriticality(this);" googl="true">
	</figure>
</div>

That way if a theme dev is using vw then they can more easily target the wp-image-blocks that contain alignright figures.

because as it is now, if the paragraphs etc and images are all laid out using vw, it's kind of hard to cleanly target images that have been aligned right. The framework I'm using uses css-in-js (specifically the emotion framework) which applies nesting rules in such a way that that I can successfully target wp-image-blocks that contain an alignright figure but only by using css unset styling rules. It results in very unclean styling logic. (I might also need to refactor my code some too but I'm pretty sure that I am in fact running into this problem in the way that I think I am. )

[jasmussen]

Indeed, the markup you get is what I get as well. I'm not very confused how I got the other markup to show up. A glitch in the matrix? A case of lack of sleep on my part? My bad, in any case.

I do understand how if the alignment class was on the parent node, there would be more control on the part of themers, yes. I think you should feel free to open a ticket to suggest that additional class.

[jcklpe]

@jasmussen Cool. I've opened a request here: #18276

[feastdesignco]

Just wanted to drop a comment here that randomly deciding to display images like tables (only when the is-resized class is applied) is completely insane and causes a bunch of unexpected headaches.

This is extremely disappointing.

[ZebulanStanphill]

It would be helpful if you could explain why it's bad and what would be better. Maybe open an issue suggesting how to fix it?