Making WordPress.org

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#4214 closed defect (bug) (fixed)

Theme preview images are loaded inefficiently.

Reported by: jonoaldersonwp's profile jonoaldersonwp Owned by: coffee2code's profile coffee2code
Milestone: Priority: normal
Component: Theme Directory Keywords: seo speed/performance has-patch needs-testing
Cc:

Description

See, e.g., https://wordpress.org/themes/twentynineteen/.

The largest size the theme preview image is displayed at is ~700px, but is loaded via a query parameter (?w=) which loads at 1142px.

This causes unnecessary load, and, browser jank.

My suspicion is that this particular value has been chosen to account for retina displays. Regardless, we should adapt the code as follows to solve for all cases (using the w param to generate/request each size):

<picture>
  <source media="(min-width: 782px)" srcset="{{image}}, {{image-2x}} 2x">
  <source media="(min-width: 481px) and (max-width: 782px)" srcset="{{image-large}}, {{image-large-2x}} 2x">
  <source media="(min-width: 401px) and (max-width: 480px)" srcset="{{image-medium}}, {{image-medium-2x}} 2x">
  <source media="(max-width: 400px)" srcset="{{image-small}}, {{image-small-2x}} 2x">
  <img src="{{image-full}}" srcset="{{image-full-2x}} 2x" alt="{{ALT}}">
</picture>

Note on size choices
The image scales fluidly from min to max size, so I've chosen some somewhat-arbitrary breakpoints which align with existing breaking behaviour on the page (on the assumption that these are sensible transition points), and selected the largest size of the image in that range.

With that in mind:

  • The base w value of {{image}} should be 572.
  • {{image-large}} should be 700.
  • {{image-medium}} should be 420.
  • {{image-small}} should be 344.
  • {{image-full}} should be 1200.

All 2x versions should set twice the w value as their equivalent image.

Note on retina images
1200 appears to be the maximum valid value, so the size of {{image-full}} and {{image-full-2x}} will be the same. At some point in the future, we should review this limitation, but this isn't necessary for V1.

Attachments (2)

4214.diff (2.9 KB) - added by diddledani 4 years ago.
Replace theme preview img tag with picture element with appropriate intermediate sizes for differing screens
4214.1.diff (3.1 KB) - added by diddledani 4 years ago.
Re-roll to fix a mistake and ensure the fallback url is correct

Download all attachments as: .zip

Change History (11)

@diddledani
4 years ago

Replace theme preview img tag with picture element with appropriate intermediate sizes for differing screens

#1 @diddledani
4 years ago

  • Keywords has-patch added

#2 @jonoaldersonwp
4 years ago

Great patching!
Apologies, though, I made a mistake in the sizes markup. I've corrected it below (removing the full size on the fallback)!

<picture>
  <source media="(min-width: 782px)" srcset="{{image}}, {{image-2x}} 2x">
  <source media="(min-width: 481px) and (max-width: 782px)" srcset="{{image-large}}, {{image-large-2x}} 2x">
  <source media="(min-width: 401px) and (max-width: 480px)" srcset="{{image-medium}}, {{image-medium-2x}} 2x">
  <source media="(max-width: 400px)" srcset="{{image-small}}, {{image-small-2x}} 2x">
  <img src="{{image}}" srcset="{{image-2x}} 2x" alt="{{ALT}}">
</picture>

Last edited 4 years ago by jonoaldersonwp (previous) (diff)

#3 @diddledani
4 years ago

It looks like I made a mistake in the diff, anyway, so I'm rolling both your fix and repairing my mistake into the next attachment...

@diddledani
4 years ago

Re-roll to fix a mistake and ensure the fallback url is correct

#4 @jonoaldersonwp
4 years ago

Nice one!

This ticket was mentioned in Slack in #themereview by dingo_d. View the logs.


4 years ago

#6 @dingo_d
4 years ago

  • Keywords needs-testing added
  • Priority changed from lowest to normal

It would be great if somebody could test this patch :)

#7 @coffee2code
4 years ago

  • Owner set to coffee2code
  • Status changed from new to reviewing

#8 @coffee2code
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 9706:

Theme Directory: Load a more efficiently sized theme screenshot on theme permalink pages based on viewport size.

Props diddledan, jonoaldersonwp.
Fixes #4214.

#9 @coffee2code
4 years ago

FYI, committed differences from 4214.1.diff:

  • Patch had the base image size as 576px (and 1152px for 2x). These seem like they should have been 572px and 1144px respectively.
  • Incorporated loading="lazy" since added for #5062.
Note: See TracTickets for help on using tickets.