Add image size between Medium and Large to better take advantage of Responsive Image support

[kirasong]

Responsive Image support for core landed in [34855].

As part of the discussion before it was approved for merge, it was brought up that it would benefit heavily from having a new image size between medium and large.

Current recommendations from the responsive images team include either:

• A static size between 760 and 800 width (we'll need a a name for it)
• A size called "content-width" that is the size of the $content-width of the theme that generates it.

The second would be the most accurate and effective, but has the downside of a potential expectation from core between theme changes.

[moonomo]

Replying to DH-Shredder:

Responsive Image support for core landed in [34855].

As part of the discussion before it was approved for merge, it was brought up that it would benefit heavily from having a new image size between medium and large.

Current recommendations from the responsive images team include either:

• A static size between 760 and 800 width (we'll need a a name for it)
• A size called "content-width" that is the size of the $content-width of the theme that generates it.

The second would be the most accurate and effective, but has the downside of a potential expectation from core between theme changes.

Earlier this year I've spent couple of month setting up responsive images for my theme (the same thing that the feature plugin is doing)- during the process I found it's best to use image size similar to the browser breakdown.

From that realization, I would suggest to go with first approach- it could be 768px width, more or less.

[kirasong]

Very rough addition of "moderate" size of 768x768

[kirasong]

Not entirely sure what possessed me, but went ahead and did a first rough of a "moderate" image size in moderate-size.diff​.

Uncertain if this is the direction we want to go, but figured that the patch being posted wouldn't hurt.

This uses an image size of 768x768 for "moderate".

Known caveats:

• It's extremely possible I missed cases; this is a rough first run.
• The tests pass, but there are several that still need to be updated to test the "moderate" size properly.
• The addition of the db_version change is almost certainly wrong, but is there to get the schema to update and add the new size. I need to dig into how to do this properly.

Comments:

• I don't like adding another size to the settings page :/ Maybe it's old-fashioned of me, but it seems backwards.
• It does seem to make the responsive images experience better, so it accomplishes the purpose of the ticket.
• There are a lot of places where we reference the default image sizes. Wow.

[travisnorthcutt]

I'm not very familiar with image handling in core, so this could be a ridiculous question. But, to your point about "I don't like adding another size to the settings page" (which I agree with), is there possibly some merit to doing this "behind the scenes", and not exposing a ui for the new size?

[krogsgard]

Moderate is confusing to me. I'd vote for content-width, set to match the $content_width variable that themes set. I would also advocate for not setting a height at all.

[melchoyce]

Moderate is confusing to me.

+1. I like the idea of using $content-width, but if we need a name for this specific "moderate" size, I think even medium-large would work.

I would also advocate for not setting a height at all.

+1

[boogah]

Moderate is confusing to me.

+1. I like the idea of using $content-width, but if we need a name for this specific "moderate" size, I think even medium-large would work.

+1 for medium-large.

As a life-long fat kid, would it be out of line for me to suggest husky? ;)

[kirasong]

Replying to krogsgard:

Moderate is confusing to me. I'd vote for content-width, set to match the $content_width variable that themes set. I would also advocate for not setting a height at all.

I agree that the name is horrible. It's just hard to find one, if we're not going with the approach you mention.

My concern with doing content-width is that at the moment, our default sizes are static, and can be expected to be consistent across themes.

If you are a user, and you change theme, does having a size named content-width suggest that WordPress is (or should be) regenerating their content-width images for the new theme?

[moonomo]

Replying to melchoyce:

Moderate is confusing to me.

+1. I like the idea of using $content-width, but if we need a name for this specific "moderate" size, I think even medium-large would work.

+1 for "medium-large" (that's what I'd used).

I would also advocate for not setting a height at all.

+1

+1 for no limited height, too. A filter to set height would be nice though.

[kirasong]

Cool to see some consensus building on a different name -- I also like medium-large much better than moderate, if we decide to create a new static size.

[melchoyce]

Replying to DH-Shredder:

If you are a user, and you change theme, does having a size named content-width suggest that WordPress is (or should be) regenerating their content-width images for the new theme?

Yeah, that's what I'd assume. Would be cool if we could do it. :)

[mor10]

How about "full width" or "theme width"?

[mor10]

Replying to melchoyce:

Replying to DH-Shredder:

If you are a user, and you change theme, does having a size named content-width suggest that WordPress is (or should be) regenerating their content-width images for the new theme?

Yeah, that's what I'd assume. Would be cool if we could do it. :)

IMO that is one of the requirements for Responsive Images to make sense in WordPress. We must supply the theme with an image sized to the theme, and when the user swaps themes, that size must be regenerated.

[krogsgard]

Funny enough, I was just doing some testing, and the media modal actually acts like the image size is the content width already, for image sizes that are in actuality larger. Twenty Sixteen sets content width to 840, so you see this:

When the actual sizes (I changed the content width to account) are this:

It was super disorienting when testing. The resulting markup is then this:

<a href="http://local.wordpress-trunk.dev/wp-content/uploads/2015/10/cape-town1.jpg" rel="attachment wp-att-82"><img src="http://local.wordpress-trunk.dev/wp-content/uploads/2015/10/cape-town1-1024x356.jpg" alt="cape-town" width="840" height="292" class="alignleft size-large-alt wp-image-82" /></a>

And the frontend markup (with srcset) this:

<img src="http://local.wordpress-trunk.dev/wp-content/uploads/2015/10/cape-town1-1024x356.jpg" alt="cape-town" width="840" height="292" class="alignleft size-large wp-image-82" srcset="http://local.wordpress-trunk.dev/wp-content/uploads/2015/10/cape-town1-300x104.jpg 300w, http://local.wordpress-trunk.dev/wp-content/uploads/2015/10/cape-town1-1024x356.jpg 1024w, http://local.wordpress-trunk.dev/wp-content/uploads/2015/10/cape-town1-1200x417.jpg 1200w, http://local.wordpress-trunk.dev/wp-content/uploads/2015/10/cape-town1.jpg 1200w" sizes="(max-width: 840px) 100vw, 840px">

So the actual image is the large size but the image attributes make it seem as if it's smaller.

I really dislike the defined with and height parameters being set to content width. If we have both a content width image and a large image (typically larger than content width), I think we'd need to adjust this to account for that attachment dropdown in the media insert modal, if not also the image markup itself.

[kirasong]

Replying to melchoyce:

Yeah, that's what I'd assume. Would be cool if we could do it. :)

I agree, and I think we can! I don't, however, think that it's doable for 4.4.

Do you think it's better for users to:

• Create another static size (medium-large) and have a content-width regenerating size later
• Create a content-width size now that does not regenerate, but can later
• Wait until we can regenerate a size to create any size
• Something else entirely?

[kirasong]

Replying to krogsgard:

Funny enough, I was just doing some testing, and the media modal actually acts like the image size is the content width already, for image sizes that are in actuality larger. Twenty Sixteen sets content width to 840, so you see this:
<snip>
I really dislike the defined with and height parameters being set to content width. If we have both a content width image and a large image (typically larger than content width), I think we'd need to adjust this to account for that attachment dropdown in the media insert modal, if not also the image markup itself.

I noticed the same thing when playing around with the new size. This strikes me as strange user behavior, and likely warrants its own ticket. Guessing it has to do with TinyMCE integration, but I don't recall these conversations.

[krogsgard]

@DH-Shredder Ha, I thought the same regarding image regeneration, so I made a separate ticket for it on #34223. Perhaps we should make another yet for the modal dropdown.

[Shapeshifter 3]

Replying to mor10:

Replying to melchoyce:

Replying to DH-Shredder:

If you are a user, and you change theme, does having a size named content-width suggest that WordPress is (or should be) regenerating their content-width images for the new theme?

Yeah, that's what I'd assume. Would be cool if we could do it. :)

IMO that is one of the requirements for Responsive Images to make sense in WordPress. We must supply the theme with an image sized to the theme, and when the user swaps themes, that size must be regenerated.

For what it's worth, I've been through 12-15 Themes (free and commercial) in the past 3 years adding to the same content (Featured Images and Videos). My current method of uploading images is to crop the largest that I find so that it is center-focused regardless of the Theme size that I am using. I have been a very strong believer in Mobile First Web Design which is based on design created from the "bottom up". That said, I'm of the opinion that Images (particularly Featured) should be uploaded from the "top down". Let the end user upload an image 1200 px or larger in the beginning, and then let the RICG Responsive Image Plugin integration take it from there.

Example: I currently have two themes installed on this site: ​https://shapeshifter3.com/. I upload a 1200px x 700px featured Image using the Kelly Theme by Automattic, center focus it by cropping, and then use a Child Theme of Twenty Thirteen for actual display. The second theme uses the Masonry layout where the images are much smaller.

The reason I do it this way is to allow me to switch back to Kelly or any other Full-Width Display Theme whenever I want to WITHOUT HAVING TO RESIZE MY FEATURED IMAGES AGAIN. If, I had chose a smaller image in the beginning and then tried to expand it...it may end up looking crappy.

So...my idea is to start with the largest size possible within coding and server requirements, in order to have the least amount of problems with imagery if I decide to switch themes again.

Hope this didn't sound self serving, but I'll side with mor10 on this issue.

[kirasong]

Initial medium-large patch. No UI. Unit Tests Need Fixing.

[kirasong]

Attached working_bad_tests.diff​ which adds a size named medium_large with a width of 768, and height of 0 (unbounded). Intentionally omitted all UI in Media Library or Settings. It includes the generated images in the responsive image display filter as expected. I suspect the db version will also need to be incremented when this goes in to get the default size into the options table.

I wanted to get a patch out there, but this patch includes broken unit tests, and I'd appreciate eyes (from @azaozz @joemcgill or anyone), since I'm probably missing something minor.

The updated responsive tests pass, but the new ones for the size are currently failing.

[joemcgill]

Fixes tests

[joemcgill]

34196.patch​ Is a refresh of working_bad_tests.diff​ that fixes the failing tests. It may be wise to split the test for inserting the new size into its own test instead of adding it to Tests_Post_Attachments:test_insert_image_medium_sizes. There are already too many assertions happening in that one test IMO.

Also, to make sure the new image size got created for the test, I had to update the value of the medium_large_size_w option so the width would be smaller than the original image, otherwise the intermediate size was getting skipped.

[kirasong]

Thanks, @joemcgill!

34196.patch​ looks good to go.

[kirasong]

For whoever commits this, it will also likely need a db_version change to get the new default options added.

[azaozz]

The patch looks good. Refreshed it a bit after r35464 in 34196.1.patch.

One question: as far as I understand we want the new medium_large size to be constant. There is no UI to change it like the other default sizes, and there is no UI to insert it in the editor. However we still store it in the options table, so any plugin or theme can change it or even remove it (by changing/deleting the options).

Wouldn't it be better to "hard-code" it in some way? Maybe in a (private) function that will return all default image sizes (and could be used to reset the changes to the other default sizes by the users)?

[kirasong]

Replying to azaozz:

The patch looks good. Refreshed it a bit after r35464 in 34196.1.patch.

One question: as far as I understand we want the new medium_large size to be constant. There is no UI to change it like the other default sizes, and there is no UI to insert it in the editor. However we still store it in the options table, so any plugin or theme can change it or even remove it (by changing/deleting the options).

So, I definitely don't think we should have UI, but have no reservations about themes still deciding what's best for their layout.

Since the optimal "mid-size" is different depending on the theme, developers/designers that know what they want should still be able to do that.

[wonderboymusic]

In 35479:

Media: add a new image size, medium_large. Bumps db version to add new options.

Adds unit tests.

Props DH-Shredder, joemcgill, azaozz.
Fixes #34196.