WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 5 weeks ago

#36191 assigned defect (bug)

Support responsive images in WP_Customize_Media_Control

Reported by: melchoyce Owned by: joemcgill
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch
Focuses: ui, administration Cc:

Description (last modified by westonruter)

When you add a custom logo, the preview in the Customizer sidebar isn't retina, even if the image file you uploaded is big enough.

Discovered in https://core.trac.wordpress.org/ticket/35942#comment:13. Some additional comments there.

Related:

  • #21455: HiDPI (retina) theme custom backgrounds
  • #36442: Customizer: when setting header image and site logo, also create a 2x image if possible

Attachments (2)

not-retina.png (49.0 KB) - added by melchoyce 16 months ago.
36191.patch (2.1 KB) - added by joemcgill 15 months ago.
Add srcset and sizes to WP_Customize_Media_Control::content_template()

Download all attachments as: .zip

Change History (23)

#1 @obenland
15 months ago

  • Milestone changed from Awaiting Review to 4.5

This ticket was mentioned in Slack in #core-images by mike. View the logs.


15 months ago

#3 @obenland
15 months ago

  • Milestone changed from 4.5 to Future Release
  • Summary changed from Custom Logo: Logo in Customizer sidebar not retina to Support responsive images in WP_Customize_Media_Control
  • Version trunk deleted

This is a bug in the way WP_Customize_Media_Control displays images, we should address it there.

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


15 months ago

#5 @joemcgill
15 months ago

  • Keywords needs-patch added
  • Owner set to joemcgill
  • Status changed from new to assigned

There seem to be a couple of challenges here.

The backbone view for the WP_Customize_Media_Control relies on data modeled from wp_prepare_attachment_for_js(), i.e., the raw attachment data, which isn't necessarily the best model for rendering img elements by itself. We probably need to create a general use model that can be used for img data—much like wp.media.model.PostImage—where we could include methods for setting up attributes like src, srcset, sizes, etc. based on the raw attachment data.

We might be able to fudge it in the mean time by using additional attachment sizes to create a srcset attribute in the WP_Customize_Media_Control::content_template() view, but the template starts to become logic-heavy at a certain point, and maybe not worth it. I also worry about sites that have overridden the default sizes and turned some into hard-crops, which would cause the image to look skewed.

@joemcgill
15 months ago

Add srcset and sizes to WP_Customize_Media_Control::content_template()

#6 @joemcgill
15 months ago

  • Keywords has-patch added; needs-patch removed

36191.patch updates the template in WP_Customize_Media_Control::content_template() to add srcset and sizes attributes when the relevant sizes are available. See my previous comment for caveats.

This ticket was mentioned in Slack in #core-customize by joemcgill. View the logs.


15 months ago

#8 @celloexpressions
15 months ago

I like the idea of building out some more robust functions for the media models here. Especially since this control handles other types of media, adding a lot of image-size logic to the template is a bit messy as noted above. That being said, it could definitely work for a shorter-term solution. It won't make 4.5 anyway though, so maybe worth going for the better solution for 4.6 early.

This ticket was mentioned in Slack in #core-images by joemcgill. View the logs.


15 months ago

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


10 months ago

#11 @melchoyce
8 months ago

Would reeeaaaally love to see this get some love. :) Non-retina logos are embarrassing.

#12 follow-up: @celloexpressions
8 months ago

It looks like this ticket has been focusing on the customizer UI; we could probably spin something off for frontend display separately without the questions around doing srcset in JS. For the output, it gets a bit complex with the way cropping happens, but my thought would be to double the size of the resize that happens after cropping to get a 2x version, then also make the 1x version as it currently does.

#13 in reply to: ↑ 12 @melchoyce
8 months ago

Replying to celloexpressions:

It looks like this ticket has been focusing on the customizer UI; we could probably spin something off for frontend display separately without the questions around doing srcset in JS. For the output, it gets a bit complex with the way cropping happens, but my thought would be to double the size of the resize that happens after cropping to get a 2x version, then also make the 1x version as it currently does.

That sounds good!

This ticket was mentioned in Slack in #core-themes by melchoyce. View the logs.


5 months ago

#15 @westonruter
5 months ago

  • Milestone changed from Future Release to 4.8

This ticket was mentioned in Slack in #core-customize by melchoyce. View the logs.


5 months ago

#17 @westonruter
5 months ago

  • Description modified (diff)

#18 @westonruter
5 months ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core by flixos90. View the logs.


5 weeks ago

#20 @flixos90
5 weeks ago

  • Milestone changed from 4.8 to Future Release

#21 @Ninos Ego
5 weeks ago

Adding svg-support for images would solve this problem, also with higher dpi than 2x (think about the future).

Note: See TracTickets for help on using tickets.