Media icons are not retina friendly

[iseulde]

See screenshot and [27726].

[wonderboymusic]

@melchoyce - can we get your comments here, please?

[melchoyce]

Replying to wonderboymusic:

@melchoyce - can we get your comments here, please?

Sorry, what's the question? Double checked and yeah, seeing 1x images instead of 2x. I can re-attach the 2x ones here, but we should really try to figure out a way to use something vector here, either the dashicons font directly or svgs.

[wonderboymusic]

@melchoyce I will defer to you on the best path forward - I need to go to retina camp and remind myself how these work

[melchoyce]

Do you remember why we couldn't use dashicons in the first place? I remember there was a good reason, but I don't actually remember what that was anymore. Do you know if it's still an issue?

[ocean90]

Replying to melchoyce:

Do you remember why we couldn't use dashicons in the first place?

See #26650 and [27726]. The issue is that wp_get_attachment_image_src() can only return images.

[melchoyce]

Replying to ocean90:

Replying to melchoyce:

Do you remember why we couldn't use dashicons in the first place?

See #26650 and [27726]. The issue is that wp_get_attachment_image_src() can only return images.

Do you know if it considers svgs images?

[joemcgill]

Optimized dashicon .png files.

[joemcgill]

31352.diff​ gets the ball rolling on this by adding @melchoyce's dashicon files (optimized in dashicon-media-2x-optimized.zip​) to the /wp-includes/images/media/ directory and referencing them in srcset attributes in media templates by adding an option to views.js when an icon is present.

Concerns:

• At the moment, the new icon files do not match the 1x versions already in use: ​https://cloudup.com/cAs9F7XWMAQ. We either need to update both or change the new 2x files to relate to the ones already in core. I would defer to Mel here.
• Using srcset with x descriptors has pretty good, but not complete browser support: ​http://caniuse.com/#feat=srcset. If we wanted to provide support for all browsers, we would need to include a polyfill. Not sure we want to add a js dependency here, so I opted not to.
• There's probably a more robust way to take the name of the 1x icon to create the 2x file name than what I've done in the inital patch, but it's a decent proof of concept.

Alternate solutions considered:

• Simply replace the current icon files with the 2x versions and serve oversized images to those who don't need them. Not ideal, but less markup. I wouldn't recommend this, but it's an option.
• Using SVGs. I couldn't find any prior art in the admin for using inline SVGs in the UI. There are several places where we use SVGs as background images via CSS, but not inline. Ultimately, this would be an ideal case for using SVGs but there have been concerns expressed with doing so.

[melchoyce]

Attached the icons as svgs. Let's use these.

[joemcgill]

There is a desire to update dashicons so that they could be used as inline SVGs instead as background images via CSS. Once the dashicon SVGs are updated (see previously linked Slack log). If we want to wait for an SVG solution, those updates would need to happen before this issue could be resolved.

Otherwise, we can add 2x png files for now and look at updating them to SVG once the dashicon improvements are merged.

[melchoyce]

Let's do PNGs for now. (See: ​https://wordpress.slack.com/archives/design-dashicons/p1426183971000112)

1x and 2x files here, if needed: https://core.trac.wordpress.org/attachment/ticket/26650/dashicons-media.zip

[joemcgill]

New media files optimized

[joemcgill]

I've compressed the dashicon files from @melchoyce and colored them to match what was already there (hex: 888888) in media-files-40px-optimized.zip. These files will need to overwrite (and add to) the existing files in src/wp-includes/images/media/ as shown in 31352.2.diff​.

By the way, If there's a better way to deal with media files in diffs, let me know.

[joemcgill]

Replaces bad diff in 31352.3.diff

[joemcgill]

I found a view that I missed when updating these media icons. 31352.4.diff​ adds support in the media list view (ignore 31352.3.diff). Just a reminder, you will still need to manually add the images from media-files-40px-optimized.zip​.

This still isn't a complete patch, because we need to account for times where the icon directory has been filtered and the 2x versions do not exist. Not sure the best way to check if the there is a way to see if a filter has been set on the icon directory within the media/views.js, so it may be better to extend wp_get_attachment_image_src() to return a valid src and src-2x. On the other hand, if we were to just use SVGs instead of PNGs here, we could avoid adding a srcset at all and simplify the whole process.

Would be interested in feedback on the preferred way forward.

[helen]

Not yet on final approach here, too late for 4.2.

[chriscct7]

Punting per discussion in Slack.

[melchoyce]

Would really love to see this get revisited soon.

[melchoyce]

@liljimmi, @empireoflight — This would be a good candidate for using SVGs.

[boemedia]

Totally agree this would be better in svg. Maybe @cathibosco or someone else from the dashicons team can take a peak? I'll ping them on Slack.

[boemedia]

I mentioned this in the #design-dashicons channel on Slack yesterday, and according to @melchoyce , this now needs a developer to make a patch using the Dashicons svgs.

[karmatosed]

As this needs a developer to make a patch, for now removing the keyword as think that's the right step. I can add it back in if this does still need design.