WordPress.org

Making WordPress.org

Opened 13 months ago

Last modified 7 days ago

#4214 reviewing defect

Theme preview images are loaded inefficiently.

Reported by: jonoaldersonwp Owned by: 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 diddledan 3 months ago.
Replace theme preview img tag with picture element with appropriate intermediate sizes for differing screens
4214.1.diff (3.1 KB) - added by diddledan 3 months ago.
Re-roll to fix a mistake and ensure the fallback url is correct

Download all attachments as: .zip

Change History (9)

@diddledan
3 months ago

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

#1 @diddledan
3 months ago

  • Keywords has-patch added

#2 @jonoaldersonwp
3 months 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>
`

Version 0, edited 3 months ago by jonoaldersonwp (next)

#3 @diddledan
3 months 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...

@diddledan
3 months ago

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

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


7 weeks ago

#6 @dingo_d
7 weeks ago

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

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

#7 @coffee2code
7 days ago

  • Owner set to coffee2code
  • Status changed from new to reviewing
Note: See TracTickets for help on using tickets.