Opened 9 years ago
Last modified 3 years 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: | 4.1 |
Component: | Customize | Keywords: | needs-patch |
Focuses: | ui, administration | Cc: |
Description (last modified by )
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:
Attachments (2)
Change History (34)
This ticket was mentioned in Slack in #core-images by mike. View the logs.
9 years ago
#3
@
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 ticket was mentioned in Slack in #core by joemcgill. View the logs.
9 years ago
#5
@
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.
#6
@
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
@
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
@
8 years ago
Would reeeaaaally love to see this get some love. :) Non-retina logos are embarrassing.
#12
follow-up:
↓ 13
@
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
@
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
This ticket was mentioned in Slack in #core-customize by melchoyce. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by flixos90. View the logs.
7 years ago
#21
@
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
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
@
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
@
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
@
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.
This is a bug in the way
WP_Customize_Media_Control
displays images, we should address it there.