Media: Indicate if image is used as a site option

[melchoyce]

When viewing images in the Media Library's list view, it displays whether or not the image is attached to a site option, like favicon, logo, or header image. However, this isn't indicated in the grid view.

I was thinking we could add this information to the image details, so it's both available in the grid view, and also available in other media selection contexts.

See attached screenshots.

[melchoyce]

We could also do the same thing for images/video/audio used in media widgets

[kirasong]

Hi Mel!

Saw this ticket while we were triaging media tickets without replies, so figured I'd reply with a note of support --
I like this as a feature/enhancement, and think it'd be cool for it to be more clear to users what their images are being used for.

[Mista-Flo]

All right, so I was working on this ticket. A bit more complicated than I thought, here's why:

• List view display cropped images as well
• If I upload a new image, and crop it, and use it as the logo for example, the media state will be displayed along the cropped image attachment in list view. But in grid view, the cropped images are not displayed, meaning I won't know which image is used as a logo. If I don't crop the original image, I will see the media state (with my patch applied).

So I will need some feedback about this inconsistency.

However I have uploaded a patch that:

• Create a new get_media_states in wp-admin/includes/template.php, it just basically mimic the same function as get_post_states, meaning I could use it without printing the same HTML output.
• It will display the media state as "Current state" in the image detail modal, feel free to give feedback about the wording here.

I'll upload two screenshots. One with the current behaviour in the list mode, another with the output of the patch.

[Mista-Flo]

Current output of List view

[Mista-Flo]

Output with the 1st patch applied

[justinahinon]

Hi @Mista-Flo , thanks for your patch. It correctly adds the information about if the image is used somewhere when I open its modal.

[justinahinon]

@desrosj do you know the reason behind (if there is one) not showing cropped images (like those cropped for site icons/logos) in the media library grid view?

[justinahinon]

I added the needs-design-feedback keyword so that we can have feedback about how this will be displayed in grid view.

[pbiron]

Replying to justinahinon:

@desrosj do you know the reason behind (if there is one) not showing cropped images (like those cropped for site icons/logos) in the media library grid view?

Some of the history behind that can be found in #50410 (and the other tickets/commits) referenced there. That ticket also discusses some of the other problems causes by not showing cropped images in grid view.

[Mista-Flo]

Note that for reference, the cropped images are now displayed in the grid library, so the current patch will also be able to display the current state for these cropped images.

[hellofromTonya]

Per Media scrub today, this ticket is win for 5.6. Moving it into the milestone. @karmatosed confirmed adding it on the design feedback list.

[hedgefield]

The work done so far looks good! And do I understand correctly that this info will also be visible in the list view of the media library? That's great. For the grid view, since there is no text there, marking an image with this info is a little harder. It might need some kind of icon overlay or something, at least something for which there is no precedent currently in WordPress, and thus would need design exploration. So I discussed it with Tammie and we agreed that for 5.6 it's fine to ship it without grid view 'support'. The info is available in the list and in the details, we can iterate on that after 5.6 with a good grid option too. Nice work!

[hellofromTonya]

Per Media meeting, with design feedback, this ticket is now ready for commit.

Importing https://core.trac.wordpress.org/attachment/ticket/42063/42063.1.patch and iterating on it.

Trac ticket: https://core.trac.wordpress.org/ticket/42063

[helen]

I moved this over to ​https://github.com/WordPress/wordpress-develop/pull/611 for tests to run and to get used to the PR workflow :)

I have two thoughts here before committing:

1. I would like the string Current state: to say Used as: instead. Both are a little misleading but the latter is somewhat less so, as you have both Header Image and Current Header Image (and the same for backgrounds). This is reflected in the GitHub pull request.
2. I'm not sure (or rather @markjaquith is not sure) that we should be adding the $echo param to _media_states() in this patch because it is not going to be used anywhere in core, and it's an internal function so it doesn't need to appeal to a wider use case. This is not yet reflected in the pull request.

[Mista-Flo]

Hi @helen thanks for your review!

I definitely agree with the wording change, "Used as" better reflects the meaning of this row.

For your second point, I'm not sure, I tried to keep consistency between _post_states, but as you said, if it's not used anywhere else in Core, it might be useless to do so.

[SergeyBiryukov]

Replying to helen:

I'm not sure (or rather @markjaquith is not sure) that we should be adding the $echo param to _media_states() in this patch because it is not going to be used anywhere in core, and it's an internal function so it doesn't need to appeal to a wider use case. This is not yet reflected in the pull request.

Just noting that would be consistent with a similar change for _post_states() in [46309].

[garrett-eclipse]

Replying to helen:

2. I'm not sure (or rather @markjaquith is not sure) that we should be adding the $echo param to _media_states() in this patch because it is not going to be used anywhere in core, and it's an internal function so it doesn't need to appeal to a wider use case. This is not yet reflected in the pull request.

One thing to note is this is due to the implementation here is slightly off, instead of using get_post_states in the template we should be using _media_states( $post, false ) to get them as a string that's already separated properly. Also the span should use media-state and not post-state for a class. So with this change the echo would then be used in core and is there for back-compat. Although these are meant to be internal functions when we looked at _post_states and just changing behaviour to echo by default we found it was used by plugins so that change wouldn't work well. Instead the new param is so we can change the core use without issues. Hope that helps make sense of it there.

[garrett-eclipse]

Revisiting after I attempted to make a refresh...
Attempting to use _media_states( $attachement, false ); I found the media template doesn't support HTML as it encodes it and viewing in the modal the label is bold so we don't so much care to do the same for the list of states. And looking at the class post-state is appropriate as the existing usage of _media_states is in the list table so it's just adopting that styling.
In short... @helen's previous PR there looks great and I would almost suggest keeping the echo as with the post_states method it was applied in multiple locations through follow-up tickets after the initial change so I would expect similar here.

[helen]

I'm not strongly opinionated on the $echo addition, just feels weird to add it before core usage actually happens. Will commit.

[helen]

In 49223:

Media: Indicate if item is or was used as a site option in the details modal.

Props Mista-Flo, melchoyce.
Fixes #42063.

[mukesh27]

We should update the doc for _media_states() function because 5.6 introduce return value as well

* @since 5.6.0 Added the `$echo` parameter.

Replace to

* @since 5.6.0 Added the `$echo` parameter and a return value.

[mukesh27]

Patch that added missing doc

[SergeyBiryukov]

In 49231:

Docs: Update the @since note for _media_states().

Follow-up to [49223].

Props mukesh27.
Fixes #42063.

Merged into WP Core in https://core.trac.wordpress.org/changeset/49223

[emrikol]

It looks like this will cause a PHP fatal if wp_prepare_attachment_for_js() is called early in a theme file.

A simple test like this should fatal:

diff --git a/themes/twentytwenty/header.php b/themes/twentytwenty/header.php
index dab6b32..f3e25d7 100644
--- a/themes/twentytwenty/header.php
+++ b/themes/twentytwenty/header.php
@@ -26,6 +26,7 @@

        <body <?php body_class(); ?>>

+               <?php var_dump( wp_prepare_attachment_for_js( '28' ) ); ?>
                <?php
                wp_body_open();
                ?>

[16-Nov-2020 17:49:40 UTC] PHP Fatal error:  Uncaught Error: Call to undefined function get_media_states() in /var/www/wp-includes/media.php:4032
Stack trace:
#0 /var/www/wp-content/themes/twentytwenty/header.php(29): wp_prepare_attachment_for_js(Object(WP_Post))
#1 /var/www/wp-includes/template.php(730): require_once('/var/www/wp-con...')
#2 /var/www/wp-includes/template.php(676): load_template('/var/www/wp-con...', true, Array)
#3 /var/www/wp-includes/general-template.php(48): locate_template(Array, true, true, Array)
#4 /var/www/wp-content/themes/twentytwenty/index.php(17): get_header()
#5 /var/www/wp-includes/template-loader.php(106): include('/var/www/wp-con...')
#6 /var/www/wp-blog-header.php(19): require_once('/var/www/wp-inc...')
#7 /var/www/index.php(17): require('/var/www/wp-blo...')
#8 {main}
  thrown in /var/www/wp-includes/media.php on line 4032
[16-Nov-2020 17:49:40 UTC] Fatal error: Uncaught Error: Call to undefined function get_media_states() in /var/www/wp-includes/media.php:4032
Stack trace:
#0 /var/www/wp-content/themes/twentytwenty/header.php(29): wp_prepare_attachment_for_js(Object(WP_Post))
#1 /var/www/wp-includes/template.php(730): require_once('/var/www/wp-con...')
#2 /var/www/wp-includes/template.php(676): load_template('/var/www/wp-con...', true, Array)
#3 /var/www/wp-includes/general-template.php(48): locate_template(Array, true, true, Array)
#4 /var/www/wp-content/themes/twentytwenty/index.php(17): get_header()
#5 /var/www/wp-includes/template-loader.php(106): include('/var/www/wp-con...')
#6 /var/www/wp-blog-header.php(19): require_once('/var/www/wp-inc...')
#7 /var/www/index.php(17): require('/var/www/wp-blo...')
#8 {main}
  thrown in /var/www/wp-includes/media.php on line 4032 [www.example.com/] []

Version: 5.6-beta4-49608

[smithjw1]

Replying to emrikol:

It looks like this will cause a PHP fatal if wp_prepare_attachment_for_js() is called early in a theme file.

A possible solution would be to wrap the call in a conditional:

<?php
if ( function_exists( 'get_media_states' ) ) {
  $media_states = get_media_states( $attachment );
  if ( ! empty( $media_states ) ) {
    $response['mediaStates'] = implode( ', ', $media_states );
  }
}