Document the difference between File, Image, and Media fields

I will try an initial version of this.

The attached patch adds one item to the definition list:

Listing media items
Media items are listed at Content > Media.

It also adds an H3 and paragraph at the bottom:

When to use Media fields

Users with appropriate permissions can attach a media reference field to any content type, block type, etc. A media reference field can refer to any configured media type. There are two reasons to use media reference fields instead of File or Image reference fields. First, media items can be reused on any other content with a media reference field. Second, media types can provide additional metadata. When in doubt, use a media reference.

One technical problem is the link ("Content > Media") in the definition item: the route is views.media.media_page_list, which does not work as an argument to Url::fromRoute(). So I used the constant string <a href="/admin/content/media">. Is there a better way?

Once #2934424: Media has no collection route is commited, I think we can fix the technical point I raised in my previous comment.

+++ b/core/modules/media/media.module
@@ -32,6 +34,8 @@ function media_help($route_name, RouteMatchInterface $route_match) {
+      $output .= '<h3>' . t('When to use Media fields') . '</h3>';

In the end, I think we want to replace File/Image fields, so media would then be the only available option. We could slightly change the title. Besides that, this partly explains the uses of the module. There is already a separate heading for that. We might want to focus the extra chapter on the differences between media, file and image and move the usage explanation to the existing 'uses' section top.

Here is an updated proposal:
For the uses section:

Listing media items
Media items are listed at Content > Media.

and

Creating references to media items
Users with appropriate permissions can attach a media reference field to any content type, block type, etc. A media reference field can refer to any configured media type. It is possible to allow multiple media types in the same field.

New chapter:

Differences between media, file and image reference fields

There are several differences between the media, file and image reference fields. When in doubt, it is recommended to use a media reference.

• The media reference fields can contain references to multiple media types in the same field. This makes it possible to reference an image and video in the same field. The file and image reference fields only allow a single type per field.
• Besides local file uploads, the media reference fields allow remote media types to be configured. This can be used to reference a Youtube video for example. The file and image reference field can only contain local file uploads.
• Since media types are fieldable, the media reference fields can allow additional metadata to be added. File and image reference fields have a fixed set of allowed metadata. The file and image media types are configured to use file and image reference fields. Media provides an extra layer to store and add metadata related to the files and images.
• Existing media items can be reused on any other content items with a media reference field. The file and image reference fields only allow creating new files and images.

These are good points. I kept the mention of additional fields short, since it is already mentioned in the "Uses" section. Maybe the repetition is useful here, since people will not always read the entire page.

How about putting the item on reusability before the item on fieldability?

Nice, these docs are helpful.

Some initial review:

1. +++ b/core/modules/media/media.module
   @@ -25,13 +25,25 @@ function media_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<dt>' . t('Creating references to media items') . '</dt>';
   

   I would say "Adding media to other content".

2. +++ b/core/modules/media/media.module
   @@ -25,13 +25,25 @@ function media_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<dd>' . t('Users with appropriate permissions can attach a media reference field to any content type, block type, etc. A media reference field can refer to any configured media type. It is possible to allow multiple media types in the same field.') . '</dd>';
   

   Let's specifically say which the "appropriate permissions" are here. Maybe:

   Users with permission to administer content types can add media support by adding a media reference field to the content type on the content type administration page. (The same is true of block types, taxonomy terms, user profiles, and other content that supports fields.)
   A media reference field...

3. +++ b/core/modules/media/media.module
   @@ -25,13 +25,25 @@ function media_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<h3>' . t('Differences between media, file and image reference fields') . '</h3>';
   +      $output .= '<p>' . t('There are several differences between the media, file and image reference fields. When in doubt, it is recommended to use a media reference.') . '</p>';
   

   Minor nitpicks: "Media", "File", and "Image" should each by capitalized, and there should be a comma after "File".

4. +++ b/core/modules/media/media.module
   @@ -25,13 +25,25 @@ function media_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<ul>';
   +      $output .= '<li>' . t('The media reference fields can contain references to multiple media types in the same field. This makes it possible to reference an image and video in the same field. The file and image reference fields only allow a single type per field.') . '</li>';
   +      $output .= '<li>' . t('Besides local file uploads, the media reference fields allow remote media types to be configured. This can be used to reference a Youtube video for example. The file and image reference field can only contain local file uploads.') . '</li>';
   +      $output .= '<li>' . t('Since media types are fieldable, the media reference fields can allow additional metadata to be added. File and image reference fields have a fixed set of allowed metadata. The file and image media types are configured to use file and image reference fields. Media provides an extra layer to store and add metadata related to the files and images.') . '</li>';
   +      $output .= '<li>' . t('Existing media items can be reused on any other content items with a media reference field. The file and image reference fields only allow creating new files and images.') . '</li>';
   +      $output .= '</ul>';
   

   I would use a definition list (<dl>) instead of a <ul>. (I think nested dl are valid markup; if not, disregard this point.) :)

Updated help text documentation with suggestions made by @xjm in #8. Items 1, 2, and 3 have been updated, but I'm holding off on item 4 as a <ul> may make more sense in this context.

@xjm, thanks for the review in #8.

#8.3 The existing text on admin/content/media capitalizes "Media module" but not "media item(s)", "media type(s)", nor "media source". The existing text does not refer to "[Mm]edia field(s)" nor "[Mm]edia reference(s)".

#8.4 If you want a definition list (<dl>) instead of an unordered list, what are you suggesting for the definition terms (<dt> elements)? I do not think this list is nested, so that seems like a non-issue.

+++ b/core/modules/media/media.module
@@ -20,18 +20,30 @@ function media_help($route_name, RouteMatchInterface $route_match) {
+      $output .= '<p>' . t('There are several differences between the Media, File, and Image reference fields. When in doubt, it is recommended to use a media reference.') . '</p>';
+      $output .= '<ul>';
+      $output .= '<li>' . t('The media reference fields can contain references to multiple media types in the same field. This makes it possible to reference an image and video in the same field. The file and image reference fields only allow a single type per field.') . '</li>';
+      $output .= '<li>' . t('Besides local file uploads, the media reference fields allow remote media types to be configured. This can be used to reference a Youtube video for example. The file and image reference field can only contain local file uploads.') . '</li>';
+      $output .= '<li>' . t('Since media types are fieldable, the media reference fields can allow additional metadata to be added. File and image reference fields have a fixed set of allowed metadata. The file and image media types are configured to use file and image reference fields. Media provides an extra layer to store and add metadata related to the files and images.') . '</li>';
+      $output .= '<li>' . t('Existing media items can be reused on any other content items with a media reference field. The file and image reference fields only allow creating new files and images.') . '</li>';

Re-read this; I think we can actually simplify it a lot:

Media reference fields offer several advantages over basic File and Image references:

• Media reference fields can reference multiple media types in the same field.
• Fields can also be added to media types themselves, which means that custom metadata like descriptions and taxonomy tags can be added for the referenced media. (Basic file and image fields do not support this.)
• Media types for audio and video files are provided by default, so there is no need for additional configuration to upload these media.
• Contributed or custom projects can provide additional media sources (such as third-party websites, Twitter, etc.).
• Existing media items can be reused on any other content items with a media reference field.

Note that I've made the bullet about remote media more generic since core doesn't support remote media yet.

We should also add what File and Image references are useful for in a paragraph after the list:

The File and Image reference field types are used within Media types. In most cases, you only need to use them when you add your own custom media types (or for legacy data created prior to enabling the Media module module).

Re: #10:

#8.3 The existing text on admin/content/media capitalizes "Media module" but not "media item(s)", "media type(s)", nor "media source". The existing text does not refer to "[Mm]edia field(s)" nor "[Mm]edia reference(s)".

For this one, I've capitalized them only where they refer to the actual user interface labels for the field types (vs. the generic plural nouns). So just in that one place. (We should also add a couple<em> tags as in the rewrite above for that reason.)

Agreed on not adding the nested <dl>; when I re-read it became clear that a <ul> is totally correct here.

Thanks!

Based on this, I think we can potentially add a case entity.whatsit.add.field.route to media_help() (if we can figure out a way to only display it when the user has selected file or image or media...) that says:

Use Media reference fields for most files, images, audio, videos, and remote media. Use File or Image reference fields when creating your own media types, or for legacy files and images created before enabling the Media module.

That can be a followup, since it might require some tricky JS or whatnot for revealing the help conditionally. Said followup might be a simpler interim fix than #2934860: Show field type descriptions when creating a new field.

Finally, bumping this issue to major since it's probably a blocker expose Media in the UI.

Thanks!

Actually, the paragraph in #12 might be a better replacement for the other final paragraph I propose in #11, since it is more succinct and definitive.

How about a good ol' form alter (in the media module, so only active if Media is enabled), that adds the help text indicated in #12 as a states-enabled description only to these 3 fields? If that is acceptable, I believe we can do that here, so no need for a follow-up.

I have also addressed the feedback from #11.

Screenshots are always good:

Hook help:

Other field types are not affected:

Help text added only when a user selects File, Image or Media references:

Found two minors:

1. +++ b/core/modules/media/media.module
   @@ -115,3 +134,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +      '@help_url' => '/admin/help/media',
   

   We shouldn't use hardcoded URLs. Better call Url::fromRoute

2. +++ b/core/modules/media/media.module
   @@ -115,3 +134,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +    $description_text .= '<br />' . t('To learn more about the difference, please install the <em>Help</em> module and visit the media help page (<em>/admin/help/media</em>).');
   

   Not sure if we could use Url::fromRoute here as well because the route might not exist.

Thanks @chr.fritsch for reviewing!

This fixes #15.1.
For #15.2, I believe this can be an acceptable exception, because we are mentioning an URL that will only be available after the Help module is installed, so I'd rather not have a clickable link here.

The patch from #16 still contains the hard-coded path that I apologized for in #4. I think that will cause problems on multilingual sites that use a language code at the start of the URL path. So here is a version that uses Url::fromUri() with the use-only-as-a-last-resort scheme internal:.

We can still update once #2934424: Media has no collection route lands. Is that worth a code comment?

@marcoscano, great idea on the form alter hook; I think that's a completely acceptable implementation and it's no risk at all if we decide to change the approach later.

Good catch @benjifisher on the multilingual implications. Hopefully someone will take a look at #2934424: Media has no collection route. A @todo to remove the internal: is needed, I think.

Some review:

1. +++ b/core/modules/media/media.module
   @@ -127,3 +134,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +function media_form_field_ui_field_storage_add_form_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {
   ...
   +  else {
   +    $description_text .= '<br />' . t('To learn more about the difference, please install the <em>Help</em> module and visit the media help page (<em>/admin/help/media</em>).');
   +  }
   

   If Media isn't installed, this hook won't be run, so I don't think we need this else. :)

2. +++ b/core/modules/media/media.module
   @@ -20,18 +21,39 @@ function media_help($route_name, RouteMatchInterface $route_match) {
   +        ':media-new' => Url::fromRoute('entity.media_type.add_form')->toString(),
   +        ':field' => Url::fromRoute('help.page', ['name' => 'field'])->toString(),
   

   Same question here about the toString().

3. +++ b/core/modules/media/media.module
   @@ -115,3 +137,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +    $description_text .= '<br />' . t('You can find more information about the difference between them on the <a href="@help_url">help page</a>.', [
   ...
   +  else {
   +    $description_text .= '<br />' . t('To learn more about the difference, please install the <em>Help</em> module and visit the media help page (<em>/admin/help/media</em>).');
   +  }
   

   I think we can simplify this sentence to:

   For more information, see the Media help page.

   The else can be:

   For more Help module and see the Media help page.

4. +++ b/core/modules/media/media.module
   @@ -115,3 +137,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +      '@help_url' => Url::fromRoute('help.page', ['name' => 'media'])->toString(),
   

   I also think we shouldn't need the toString(); it should be stringcast automatically now (at least I thought we addressed that in the last few months of 8.0.0's beta). Please test and see if that works. :)

+++ b/core/modules/media/media.module
@@ -20,18 +21,39 @@ function media_help($route_name, RouteMatchInterface $route_match) {
+      $output .= '<dd>' . t('Media items are listed at <a href=":media-collection">Content > Media</a>.', [

I don't think we should use the format "Content > Media" because that implies a menu link (there's not), the format might not be understandable, and site owners also might have put the page somewhere else. "The media administration page" might be a good replacement.

Finally, we probably want a test asserting that the text shows for the file, image, and media references, but nothing else.

Thanks @xjm for reviewing!

1.

If Media isn't installed, this hook won't be run, so I don't think we need this else. :)

This is supposed to deal with scenarios where the Help module isn't installed, is this a confusion or am I missing something?

2. The ::toString() seems to be necessary, if it's not there we have:

Warning: strpos() expects parameter 1 to be string, object given in Drupal\Component\Utility\UrlHelper::stripDangerousProtocols() (line 352 of core/lib/Drupal/Component/Utility/UrlHelper.php).

3. Fixed, although with a slight variation (maybe a copy-paste typo in the proposed text)?

4. Same as 2

Also, added a test!

Thanks!

Once again, I do the easy stuff: this time, adding the @todo in case #2934424: Media has no collection route does not get fixed before this issue.

If Media isn't installed, this hook won't be run, so I don't think we need this else. :)

This is supposed to deal with scenarios where the Help module isn't installed, is this a confusion or am I missing something?

Oops sorry, I thought I had deleted that bullet from my review. I had realized halfway through typing it that I was confused. :)

+++ b/core/modules/media/media.module
@@ -20,18 +21,41 @@ function media_help($route_name, RouteMatchInterface $route_match) {
+      $output .= '<dd>' . t('Media items are listed at <a href=":media-collection">The media administration page</a>.', [

Nitpick "The" should not be ccapitalized inside this link since it's in the middle of a sentencec.

Ops, sorry!

Yay! Now I can remove the @todo that I added in #22 ... and todo it.

On a widescreen, the description jumps to the right. Not sure if it is intended.

Just realize the magic tag was missing.

Adding the SS to the summary for the part that needs usability review.

Maybe the form help text is easier to read in bullets?

• Use Media reference fields for most files, images, audio, videos, and remote media.
• Use File or Image reference fields when creating your own media types, or for legacy files and images created before enabling the Media module.

Edit: another note for the usability reviewers: We currently have the same text on the selection of both Media fields and File/Image (because you need to know what your choice implies in either case). It's also possible for us to make the text different depending on the selection, if that would work better, but this is the best so far I think. :)

I think having the same description for all three is fine. It is a bit long, but we need to explain the different choices somehow. The dropdown does not allow that, so we need a way to explain it. I have some comments on the text itself:

1. +++ b/core/modules/media/media.module
   @@ -20,18 +21,39 @@ function media_help($route_name, RouteMatchInterface $route_match) {
   +      $output .= '<p>' . t('Use <em>Media</em> reference fields for most files, images, audio, videos, and remote media. Use <em>File</em> or <em>Image</em> reference fields when creating your own media types, or for legacy files and images created before enabling the Media module.') . '</p>';
   
   @@ -115,3 +137,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +  $description_text = t('Use <em>Media</em> reference fields for most files, images, audio, videos, and remote media. Use <em>File</em> or <em>Image</em> reference fields when creating your own media types, or for legacy files and images created before enabling the Media module.');
   

   What about user profile pictures, let's say? :)

2. +++ b/core/modules/media/media.module
   @@ -115,3 +137,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +  if (\Drupal::moduleHandler()->moduleExists('help')) {
   +    $description_text .= '<br />' . t('For more information, see the <a href="@help_url">Media help page</a>.', [
   +      '@help_url' => Url::fromRoute('help.page', ['name' => 'media'])->toString(),
   +    ]);
   

   It looks strange to me to have this on a separate line. It makes it appear like a list (we don't do paragraphs in descriptions, so given its not running-on, it made me think of a list immediately). I would just keep the text running as one without the br.

3. +++ b/core/modules/media/media.module
   @@ -115,3 +137,41 @@ function media_field_ui_preconfigured_options_alter(array &$options, $field_type
   +  else {
   +    $description_text .= '<br />' . t('For more information, install the <em>Help</em> module and see the Media help page (<em>/admin/help/media</em>).');
   +  }
   

   Hm, I would *personally* not add this, it looks like overly fool-proof. Do we tell people elsewhere to enable the help module for help, or is that too obvious? :) No very strong feelings against it, but it looks very odd to me.

What about user profile pictures, let's say? :)

Those should still eventually be stored as image media and just have access restrictions (and good defaults and etc.). Edit: which brings us back to the per-media type permissions issue which I forgot to add to the UX discussion aside from it being tagged in the RTBC queue. Will attach it in a moment.

It looks strange to me to have this on a separate line. It makes it appear like a list (we don't do paragraphs in descriptions, so given its not running-on, it made me think of a list immediately). I would just keep the text running as one without the br.

Agreed. Or if anything the bit above it should be two bullets and this should be a separate paragraph.

Hm, I would *personally* not add this, it looks like overly fool-proof. Do we tell people elsewhere to enable the help module for help, or is that too obvious? :) No very strong feelings against it, but it looks very odd to me.

How about the sentence is just added conditionally when Help is enabled, without the "enable the module" bit? Like remove the else entirely.

NW for #31 and #32.

Here we go!

Hm, I did not expect profile pictures to be ideally media. Otherwise looks great. Thanks all!