Make WordPress Core

Opened 6 years ago

Last modified 27 hours ago

#45985 new enhancement

Twenty Nineteen: Introduce better theme support for responsive images

Reported by: kjellr's profile kjellr Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.0.3
Component: Bundled Theme Keywords: dev-feedback
Focuses: performance Cc:

Description (last modified by sabernhardt)

As originally reported by @mor10, in the Twenty Nineteen GitHub repository:

https://github.com/WordPress/twentynineteen/issues/50


Currently, for images inserted within posts, the sizes attribute of the responsive images markup does not map to the actual displayed width of images.

According to my tests (using a 1920x1080px non-aligned image, inserted via Gutenberg (and set to Image Size: Full):

The sizes markup is output as:
(max-width: 1920px) 100vw, 1920px

  • At viewports <300px: A 300px image is loaded
  • At viewports >300px: A 768px image is loaded
  • At viewports >768px: A 1024px image is loaded
  • At viewports >1024px: A 1568px image is loaded
  • At viewports >1568px: The original 1980px image is loaded

Since this is not a full-width image, each of those image sizes is actually wider than necessary, resulting in a larger file size.

For a more specific example: at a screen width of 1024px, this non-aligned sample image is loaded in at 1568px wide. As per theme rules, it actually appears only 700px wide in the layout. On a non-retina screen, there's no need to load a 1568px image for a 700px space.

Twenty Nineteen should include some theme-specific markup to properly assign sizes values that map to theme styles. This was done in Twenty Seventeen for instance:

https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentyseventeen/functions.php#L491-L517

At the time of writing, this issue is blocked by Gutenberg: We can currently issue a custom sizes attribute for non-aligned images like the example above, but it is not currently possible to have the theme customize the sizes attribute for full/wide images. See the following GitHub issues for reference:

https://github.com/WordPress/gutenberg/issues/6131

https://github.com/WordPress/gutenberg/issues/6177

https://github.com/WordPress/gutenberg/pull/11973


I'm including patches ported over from two of @mor10's PRs on the original Twenty Nineteen repository:

Modifying the sizes attribute for non-aligned body images (This currently causes negative results for full and wide images)

https://github.com/WordPress/twentynineteen/pull/701

45985-regular-images.patch

Adding custom sizes attributes, including wide and full variants (This is dependent on Gutenberg #11973 being merged):

https://github.com/WordPress/twentynineteen/pull/629

45985-all-images.patch

Attachments (2)

45985-regular-images.patch (1.4 KB) - added by kjellr 6 years ago.
45985-all-images.patch (1.9 KB) - added by kjellr 6 years ago.

Download all attachments as: .zip

Change History (9)

#1 @kjellr
6 years ago

  • Description modified (diff)

There are many more details in the linked threads, but @mor10: feel free to add anything that I may have missed.

Thank you!

#2 @vishalkakadiya
20 months ago

We are already working on the task related to this ticket, so we may need to discuss this task before starting on it. Thanks!

CC @mukesh27

#3 @vishalkakadiya
19 months ago

@kjellr Thank you for opening this ticket!

I checked your patch for the regular images, and it is working fine. But, can you explain the below condition there?

if ( 767 <= $width ) {

Adding custom sizes attributes, including wide and full variants (This is dependent on Gutenberg #11973 being merged):

Also, I think the ticket number(11973) is looking incorrect to me or let me know if I'm understanding it incorrectly.

Thank you!

#4 @kjellr
19 months ago

Hey @vishalkakadiya, thanks for taking a look. The patches on this ticket were ported over from @mor10's GitHub PR, created when Twenty Nineteen was originally built:

https://github.com/WordPress/twentynineteen/pull/701

I'm not sure of the details of this patch personally anymore (it's a few years old now! 😅) but it's possible @mor10 can help provide more info if you're still interested in pushing it forward.

Thanks!

#5 @sabernhardt
19 months ago

  • Description modified (diff)

(I corrected the link to GB11973, which was not merged)

#6 @karmatosed
4 weeks ago

  • Keywords dev-feedback added

#7 @karmatosed
27 hours ago

I am not sure what the current status is on this but due to the time it has sat and the partial work, I would recommend closing or a refresh. For now, going to suggest we get some thoughts to then get the next steps worked out for this issue.

cc @sabernhardt and @vishalkakadiya. When you are considering what to do consider this theme has been live for a while and we are focusing on bugs not enhancements at this point.

Note: See TracTickets for help on using tickets.