Make WordPress Core

Opened 9 years ago

Last modified 3 years ago

#36191 assigned defect (bug)

Support responsive images in WP_Customize_Media_Control

Reported by: melchoyce's profile melchoyce Owned by: joemcgill's profile joemcgill
Milestone: Future Release Priority: normal
Severity: normal Version: 4.1
Component: Customize Keywords: needs-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 9 years ago.
36191.patch (2.1 KB) - added by joemcgill 9 years ago.
Add srcset and sizes to WP_Customize_Media_Control::content_template()

Download all attachments as: .zip

Change History (34)

@melchoyce
9 years ago

#1 @obenland
9 years ago

  • Milestone changed from Awaiting Review to 4.5

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


9 years ago

#3 @obenland
9 years 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.


9 years ago

#5 @joemcgill
9 years 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
9 years ago

Add srcset and sizes to WP_Customize_Media_Control::content_template()

#6 @joemcgill
9 years 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.


9 years ago

#8 @celloexpressions
9 years 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.


9 years ago

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


8 years ago

#11 @melchoyce
8 years ago

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

#12 follow-up: @celloexpressions
8 years 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 years 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.


8 years ago

#15 @westonruter
8 years ago

  • Milestone changed from Future Release to 4.8

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


8 years ago

#17 @westonruter
8 years ago

  • Description modified (diff)

#18 @westonruter
8 years ago

  • Description modified (diff)

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


7 years ago

#20 @flixos90
7 years ago

  • Milestone changed from 4.8 to Future Release

#21 @Ninos Ego
7 years ago

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

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


7 years ago

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


7 years ago

#24 @joemcgill
7 years ago

  • Milestone changed from Future Release to 4.9

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


7 years ago

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


7 years ago

#27 @westonruter
7 years ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

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


7 years ago

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


7 years ago

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


7 years ago

#31 @westonruter
7 years ago

  • Milestone changed from 4.9 to Future Release
  • Priority changed from high to normal

Punting since we're too far into beta.

#32 @celloexpressions
3 years ago

  • Keywords needs-patch added; has-patch removed
  • Version set to 4.1

@joemcgill does the media library have a way to send the full srcset markup to the customize control JS now? I've had success with pulling srcset data from frontend post content markup and back into JS templates elsewhere.

Ideally the content_template would only have something like srcset="{{ data.attachment.srcset }}" added to it and the rest of the logic happens in JS.

Note: See TracTickets for help on using tickets.