At least inform content authors where they can list and add media

Use target="_blank" to prevent users from losing work by clicking on the link. This isn't nearly as helpful as #1510544: Allow to preview content in an actual live environment, but it will at least prevent disasters.

Why not open this link in the off-canvas dialog instead? The dialog itself is no longer experimental.

The primary point of concern will be the links, since they take the user out of the context of their form. Is the target="_blank" a reasonable tradeoff for providing users a direct link to the pages?

They would remain in context.

Tagging for UX team review.

The before/after screenshots only show the single form case, not the multivalue form case. Would be nice to see that. Comments on the text/behavior:

1. +++ b/core/modules/media/media.module
   @@ -172,3 +173,99 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    $element['target_id']['#title'] = t('<h4>Use existing %title media</h4>', ['%title' => $label]);
   ...
   +    $elements['#title'] = t('<h4>Use existing %title media</h4>', ['%title' => $label]);
   

   I would not add "media" on the end of this. We should look at which media type names would need that tacked on to be meaningful. The one screenshot definitely does not need it IMHO, it looks distracting and computer generated.

2. +++ b/core/modules/media/media.module
   @@ -172,3 +173,99 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    $reuse_message = t('Type part of the media name. See the <a href=":list" target="_blank">media list</a> (opens a new window) to help locate media.', [':list' => Url::fromRoute('entity.media.collection')->toString()]);
   

   Can we filter for the right media type(s)? Otherwise people need to figure out there how to do it.

I love this issue!

IMHO, the target="_blank" is a very reasonable tradeoff, and justifiable in the context of the hotfix this is intended to address.

In this patch I have:
- Found that the new hook was only being invoked for cardinality -1, so other multiple fields were not getting the text. Tried to fix it, it appears to work now for me.
- Tweaked a bit the hook documentation in field.api.php
- Fixed some minor CS issues
- Found that the new text can get be confusing if the previous field also has a multiple-line description. Maybe we could wrap everything inside a fieldset container to make more obvious that all this text is from a single field?
- Addressed #15.1

- About #15.2:

Can we filter for the right media type(s)? Otherwise people need to figure out there how to do it.

Strictly speaking, the site could have disabled views, and in that case the collection route doesn't have filters. We could add some logic that sends to different links depending if Views is installed or not, but maybe that would be overkill? IMHO I believe it is safe to assume at this point that most users know how to use exposed filters? (or I may have completely misunderstood your point, please correct me if so).

Some screenshots:

Patch in #9:

Patch in #9 but invoking the hook also for non-unlimited cardinality fields:

Adding wrappers and addressing #15.1:

Still needs tests, I can work on that if we all agree on how it should look like.

This seems like a really good idea! Have a bunch of nits and suggestions to change some interface texts.

Besides that it at least needs tests for the new hook and making sure the description is shown for media entity reference field (and not shown for other entity reference auto complete fields), but I see it's already tagged accordingly.

Leaving to "needs review" for now to get more opinions.

1. +++ b/core/modules/field/field.api.php
   @@ -221,6 +223,77 @@ function hook_field_widget_WIDGET_TYPE_form_alter(&$element, \Drupal\Core\Form\F
   + * To adjust the individual widgets for the values themselves, use
   

   I'm kind of surprised about this. We should probably add documentation to hook_field_widget_form_alter() to show that this hook exists as well.

2. +++ b/core/modules/field/field.api.php
   @@ -221,6 +223,77 @@ function hook_field_widget_WIDGET_TYPE_form_alter(&$element, \Drupal\Core\Form\F
   + * To modify the widget for individual itesm, use
   

   Typo 'itesm'

3. +++ b/core/modules/media/media.module
   @@ -172,3 +173,109 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  // On autocomplete widgets, add some help text to aid users know what to do.
   +  // Only act on entity reference fields that point to Media entities.
   

   This sentence feels a little weird to me? Maybe it's because English is not my first language.

   Could we change it to:
   Add a description to inform users where they can find and add new media. Only add the description on autocomplete widgets for entity reference field pointing to media.

   Feel free to ignore this if it doesn't make any sense.

4. +++ b/core/modules/media/media.module
   @@ -172,3 +173,109 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +
   

   We can remove this blank line I think.

5. +++ b/core/modules/media/media.module
   @@ -172,3 +173,109 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    $reuse_message = t('Type part of the media name. See the <a href=":list" target="_blank">media list</a> (opens a new window) to help locate media.', [':list' => Url::fromRoute('entity.media.collection')->toString()]);
   

   Again, English is not my first language but shouldn't "Type part" be "Type a part"

6. +++ b/core/modules/media/media.module
   @@ -172,3 +173,109 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    // Only add the suffix if the user also has permission to create media. Some
   +    // sites might allow content authors to pick approved media but not to
   +    // submit their own.
   

   Can we show the message about the media list only once as well? Repeating that seems just as "spammy". Maybe something like:

   Type a part of the media name. See the media list (opens a new window) to help locate media. Create new media via the media add page (opens a new window), then add it by name to the field below.

7. +++ b/core/modules/media/media.module
   @@ -172,3 +173,109 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +      $new_message = t('<h4>Uploading new %title</h4> Upload your media on the <a href=":add" target="_blank">media upload form</a> (opens a new window), then add it by name to the field below.', ['%title' => $label, ':add' => Url::fromRoute('entity.media.add_page')->toString()]);
   

   I proposed some small changes above. Mainly "upload" is not always applicable (for remote video for example). The link is to the media add page and not a media form, so that could be changed as well to avoid confusion.

Discussed in the UX calls and it looked fine for us as an MVP. The target="_blank" might not be the best solution, but the help text makes it good enough for MVP.

@roland.molnar mentioned that maybe some more text explaining how to extend the basic functionalities could help. One possible location might be Drupal's help.

Also it was suggested to add some more text in the description of the module, right now is: 'Create reusable media.'. But maybe it can be a followup issue.

I think this makes sense for an MVP. I agree that target="_blank" isn't ideal, and that a Settings Tray dialogue would be better, but think this works in the interim.

My one nitpick is that I think "Upload new Photos and videos media" makes more sense than "Uploading new Photos and videos media" to match the wording of "Use existing Photos and videos media" vs "Using existing Photos and videos media"

Adding this new hook is a great idea. I cannot overstate how excited I am about this idea. It is incredibly useful for very many different use-cases and a lot of new room for experimentation in contrib. Let's add this in a separate issue, though, and get a sign-off from one of the Field API maintainers ( === ['@amateescu'] ) please.

We reviewed this on the weekly Drupal Product Management call, and though there was much hand-wringing and gnashing of teeth, agreed with this as a stop-gap for the current behaviour to unblock exposing the module in the UI, since it is easily 3,000% better than what's in HEAD right now. (Though UX-wise, still not remotely comparable to even our most basic competitors out there, so we definitely have more work to do. :\)

Not sure if I covered all feedback, but this patch should address #17, #19, #22 and adds some tests.
Did not investigate the possibility of skipping the new hook (#20) because I feel there is some interest also outside of media to have it available? (based on #23)

Also, I think I would question the perception that the description text below each autocomplete element is spammy... Users tend not to read stuff. However, when they are trying to type something they are not sure what it is, it will be *certainly* read. Actually, I have the feeling that this is probably the most important piece of text that we are adding in this patch, so I would try to leave it there, even at the price of being repetitive if the field is multi-valued.

Please let me know if I missed something!

While testing patch from #25 I found that each time you add new item to the multi-value media field there is another fieldset added.

So I've moved the fieldset creation code to the top of the alter function and now it's working as expected.

Ok looked at the example the system block and what it does in system_theme

'block__system_branding_block' => [
      'render element' => 'elements',
      'base hook' => 'block',
    ],

Ad then it adds the extra stuff it needs in system_preprocess_block()

This seems to be what we need to do.

So I added media_preprocess_media_reference_help and added the are extra info there.

It seems to work.

This should fix the tests. The assertions were quite out-of-date and looking for elements that didn't exist. Also DRYed things up a bit.

And here's a screenshot of the test running in Chrome:

EDIT: I'd been working on this for about an hour before @xjm posted her comment, so hopefully this doesn't step on anyone's toes!

+++ b/core/modules/field/field.api.php
@@ -220,6 +222,77 @@ function hook_field_widget_WIDGET_TYPE_form_alter(&$element, \Drupal\Core\Form\F
+/**

+++ b/core/modules/media/templates/media-reference-help.html.twig
@@ -0,0 +1,60 @@
+
...
+

These spaces don't need to be here.

+++ b/core/modules/media/templates/media-reference-help.html.twig
@@ -0,0 +1,60 @@
+ * @todo I have no idea what I'm doing.

Lies and damn lies.

So, maybe it's just me, but something doesn't smell right. Do we ever put this much text in a template in core?

I haven't applied this and gone through all the comments, but are we trying to make this consistent with the core fieldset template? If so, the fieldset element should have an attributes object and the label should have a wrapping span for the label text.

This seems pretty damn good to me. The tests are quite thorough so I'm removing the tag.

1. +++ b/core/lib/Drupal/Core/Field/WidgetBase.php
   @@ -311,12 +324,14 @@ public static function addMoreAjax(array $form, FormStateInterface $form_state)
      protected function formSingleElement(FieldItemListInterface $items, $delta, array $element, array &$form, FormStateInterface $form_state) {
   +      $cardinality = $this->fieldDefinition->getFieldStorageDefinition()->getCardinality();
        $element += [
   

   Nit: $cardinality should be outdented by two spaces.

2. +++ b/core/modules/media/media.module
   @@ -66,6 +67,10 @@ function media_theme() {
   +      'base hook' => 'field_multiple_value_form',
   

   I don't understand what this does, so I assume it's necessary...

3. +++ b/core/modules/media/media.module
   @@ -172,3 +177,54 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  if (!empty($elements[0]['target_id']['#type']) && ($elements[0]['target_id']['#type'] === 'entity_autocomplete') && ($elements[0]['target_id']['#target_type'] === 'media')) {
   

   Nit: The final two conditions are surrounded by superfluous parentheses.

4. +++ b/core/modules/media/media.module
   @@ -172,3 +177,54 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    $use_existing_label = t('Use existing %title', ['%title' => $label]);
   

   Would it make more sense for $label to be the plural label of the target entity type?

5. +++ b/core/modules/media/media.module
   @@ -172,3 +177,54 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +function media_preprocess_media_reference_help(&$variables) {
   

   $variables should be type hinted.

6. +++ b/core/modules/media/media.module
   @@ -172,3 +177,54 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +      if (\Drupal::entityTypeManager()->getAccessControlHandler('media')->createAccess($bundle, \Drupal::currentUser())) {
   

   We don't need to pass \Drupal::currentUser() to the createAccess() call.

That test is great. I really like it. Super thorough, and clear too!

1. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -192,6 +204,294 @@ public function testRenderedEntityReferencedMedia() {
   +    $this->assertEquals($field->getLabel(), $fieldset->find('css', 'legend')->getText());
   

   Should probably use assertSame() when checking strings; there's no real reason not to. We should also assert the legend exists (rather than courting fatal errors by assuming it does), like this:

   $this->assertSame($field->getLabel(), $this->session->elementExists('css', 'legend', $fieldset)->getText());

2. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -192,6 +204,294 @@ public function testRenderedEntityReferencedMedia() {
   +  /**
   +   * Asserts that the given texts are present exactly once.
   +   *
   +   * @param string[] $texts
   +   *   A list of the help texts to check.
   +   * @param bool $expected
   +   *   Whether or not the text is expected.
   +   */
   +  public function assertHelpTexts(array $texts, $selector = '') {
   

   Doc comment seems to be out of date :)

3. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -192,6 +204,294 @@ public function testRenderedEntityReferencedMedia() {
   +        $this->session->elementsCount('css', $selector . ":contains('$text')", 1);
   

   What if $text contains apostrophes/single quotes? Should we escape it first?

4. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -192,6 +204,294 @@ public function testRenderedEntityReferencedMedia() {
   +  /**
   +   * Asserts that a given link is not present.
   +   *
   +   * @param \Behat\Mink\Element\NodeElement $element
   +   *   The element to search.
   +   * @param string $text
   +   *   The link text.
   +   */
   +  protected function assertNoHelpLink(NodeElement $element, $text) {
   +    $this->assertHelpLink($element, $text, [], FALSE);
   +  }
   

   Not a big deal, but it seems weird for this to be a wrapper around assertHelpLink(). It could just be as simple as:

   $this->session->elementNotExists('named', ['link', $text], $element)

Here is patch that just shows the media changes for review as the field API changes are being worked on in #2940201: hook_field_widget_form_alter() can no longer affect the whole widget for multi-value fields

Look through @phenaproxima review in #57

All fixed.

For 4) \Drupal\Tests\media\Functional\MediaUiFunctionalTest::assertNoHelpLink no has another check so I think it should not be removed now.

1. Re #55.2
   

   I don't understand what this does, so I assume it's necessary...

   and

   Me neither but Ted added that and it made it work so... :D

   Oh no!
   See #41 for where I got this idea from. I think some who knows the theme system better me to confirm that this is the proper way to do this.

2. +++ b/core/modules/media/media.module
   @@ -172,3 +177,54 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    foreach ($bundles as $bundle) {
   +      if (\Drupal::entityTypeManager()->getAccessControlHandler('media')->createAccess($bundle)) {
   +        $create_access = TRUE;
   +      }
   +    }
   

   We should add a break here after $create_access = TRUE;.

   We just need to know if the user has access to create 1 bundle. Check this access could be expensive if there are implementations of hook_entity_create_access

3. +++ b/core/modules/media/media.module
   @@ -172,3 +177,54 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    if ($create_access === TRUE) {
   +      $variables['media_add_page'] = Url::fromRoute('entity.media.add_page')->toString();
   +    }
   

   If the user has access to add only 1 bundle then we should probably make the link to the add page just for that bundle to avoid confusion.

   The user could access only 1 bundle for this field but then other bundles that can't be referenced here.

4. +++ b/core/modules/media/media.module
   @@ -172,3 +177,54 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    if (\Drupal::currentUser()->hasPermission('access media overview')) {
   

   Would it be better to check access against the actual route like
   Url::fromRoute('entity.media.collection')->access()
   Then if access on the route is altered this would be picked up. Not positive on this but it seems more robust.

Addressing #60

1. Not fixed
2. Added the break but it changed to not use $create_access but rather $create_bundles because we need to keep track of whether the user has access to create 0, 1 or more than 1 bundles.

   So breaking on count($create_bundles) > 1

3. Fixed use route entity.media.add_form if the user has access to create just 1 bundle
4. fixed

In #61 I forgot to update the tests. So this patch does:

1. Updates the tests to check for the different route if the user can only create 1 media type
2. Updates \Drupal\Tests\media\Functional\MediaUiFunctionalTest::providerTestMediaReferenceWidget to use keyed indexes because this is easier for debugging and test error results
3. Fix a couple unused variables in \Drupal\Tests\media\Functional\MediaUiFunctionalTest::testMediaReferenceWidget

Just a typo in variable name in #65

This is pretty good, and is an improvement on the standard entity reference field. Nice job 👍

1. One thing I noticed is that the name of the field is used for the "Create new ..." and "Use existing ..." text, which looks OK when the field is named something like "Media", but when you use something more specific (see below), it can look overly repetitive (and may be grammatically incorrect, i.e. "Create new "Vacation pictures" media").

Instead of using the field name, could we use the label of the bundle if only one bundle is targeted, or just "media" if multiple bundles are targeted?

2. The "Create new ..." text uses an h4 tag, which makes it larger than the "Use existing ..." label. Is this intentional? Could we just a strong tag instead?

3. The "Allowed media types ..." text uses the machine name of the Media types, instead of the label. Would look a bit nicer to use the label.

Example for this comment:

+++ b/core/modules/media/templates/media-reference-help.html.twig
@@ -0,0 +1,70 @@
+      {% if media_list_link %}
...
+      {% if description.content %}

These ifs shouldn't be necessary. There is no markup being printed inside them, only Twig variables.

It is needed, for example, for the button and h4.

Also, no text in the template. yay \0/

Just looking at the interdiff,

+++ b/core/modules/media/media.module
@@ -184,7 +184,7 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
 /**
  * Implements hook_field_widget_multiple_WIDGET_TYPE_form_alter().
  */

The docblock should be updated to match the new hook name.

Sorry, I do not have time to do a real review.

This makes the changes requested in #81 and #82. I have no opinion on #81.

All my previous feedback has been addressed, and the patch is looking good now. Marking as RTBC, pending further review.

Content wise this looks ok. For me "then add it by name *to* the form" felt slightly off, was thinking "then add it by name *in* the form below. I'll defer to the native speakers, no biggie I think.

Is there a reason why the texts below the form elements ("Type part of…") are not shown as actual descriptions for those fields? Currently these texts have a larger gap between them and the form field above, again looking slightly off.

+++ b/core/modules/media/templates/media-reference-help.html.twig
@@ -0,0 +1,66 @@
+      </h4><br />

Where did this wild BR come from? It shouldn't be needed for spacing following an H4. Is there some CSS forcing that inline? I didn't notice any in Seven with a quick look.

1. +++ b/core/modules/media/media.module
   @@ -172,3 +180,77 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  Element::setAttributes($element, ['id']);
   +  RenderElement::setAttributes($element);
   +  $variables['attributes'] = isset($element['#attributes']) ? $element['#attributes'] : [];
   

   I don't understand the need for these 3.
   How do they improve upon the attribute handling in template_preprocess that will run for this first?
   Could use some comments.

2. +++ b/core/modules/media/media.module
   @@ -172,3 +180,77 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +      $bundle_labels[] = MediaType::load($bundle)->label();
   

   What about using the EntityTypeBundleInfo service? Or retrieving the labels directly from the element? See \Drupal\Core\Entity\Plugin\EntityReferenceSelection\DefaultSelection::buildConfigurationForm().
   NWing for this

3. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -21,6 +26,13 @@ class MediaUiFunctionalTest extends MediaFunctionalTestBase {
   +  /**
   +   * The assertion session.
   +   *
   +   * @var \Behat\Mink\Session
   +   */
   +  protected $session;
   

   Why does this need to be stored? If it's a helper, I've never seen it done this way, I'd recommend not doing it and having a

   $assert_session = $this->assertSession();
   

   at the top of each method.

   If this is needed for some actual functional reason, that is scary and should be explicitly documented.

4. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -192,6 +204,296 @@ public function testRenderedEntityReferencedMedia() {
   +   * Test the default autocomplete widgets for media reference fields.
   

   Nit: start with "Tests"

5. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -192,6 +204,296 @@ public function testRenderedEntityReferencedMedia() {
   +    entity_get_form_display('node', $non_media_content_type->id(), 'default')
   ...
   +    entity_get_form_display('node', $content_type->id(), 'default')
   

   Nit: This is deprecated.

1. +++ b/core/modules/media/media.module
   @@ -181,25 +181,97 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  if ($field_type !== 'entity_reference' ||  $target_type !== 'media') {
   +    return;
   +  }
   

   Yay early returns!

2. +++ b/core/modules/media/media.module
   @@ -181,25 +181,97 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +    if (count($create_bundles) === 1) {
   +      $add_url = Url::fromRoute('entity.media.add_form')->setRouteParameter('media_type', $create_bundles[0])->toString();
   ...
   +    elseif (count($create_bundles) > 1) {
   +      $add_url = Url::fromRoute('entity.media.add_page')->toString();
   ...
   +    $elements['#media_help']['#media_add_help'] = t('Create your media on the <a href=":add_page" target="_blank">media add page</a> (opens a new window), then add it by name to the field below.', [':add_page' => $add_url]);
   

   Unless I'm missing something, there is an unwritten else case here in which $add_url will be an undefined variable. Aka, when there are no bundles?
   Adding $add_url = NULL; above and wrapping the last line in if ($add_url) would help

3. +++ b/core/modules/media/media.module
   @@ -181,25 +181,97 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
        }
   +    $elements['#media_help']['#media_add_help'] = t('Create your media on the <a href=":add_page" target="_blank">media add page</a> (opens a new window), then add it by name to the field below.', [':add_page' => $add_url]);
   

   The double nested #-prefixed keys look off to me. What's the reason this is needed at the interior level?

4. +++ b/core/modules/media/media.module
   @@ -181,25 +181,97 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  $cardinality = $context['items']->getFieldDefinition()->getFieldStorageDefinition()->getCardinality();
   

   Whenever I see code digging this deep I think of a skier on a slalom course. Not sure why. But, yikes! (also nothing to be done about that here, this is correct AFAIK).

5. +++ b/core/modules/media/media.module
   @@ -220,42 +292,10 @@ function media_preprocess_media_reference_help(&$variables) {
   +    foreach ($element['#media_help'] as $key => $text) {
   +      $variables[substr($key, 1)] = $text;
        }
   

   If the # prefixes from above are removed, this substr call can be dropped. And in fact, the whole thing could be $variables += $element['#media_help'];

6. +++ b/core/modules/media/templates/media-reference-help.html.twig
   @@ -25,7 +25,7 @@
   -    <span{{ legend_span_attributes.addClass(legend_span_classes) }}>{{ label }}</span>
   +    <span{{ legend_span_attributes.addClass(legend_span_classes) }}>{{ original_label }}</span>
   

   The need for this change is not clear to me, but seems okay?

Created follow up issues and update @todo in patch

#2943020: Inform content authors where they can list and add media on all media reference widgets
#2943024: Ensure widget title is correct when adding media reference help text and links

UPDATE: sorry was not on 8.6.x when I created my new issue branch. will upload a new patch

Here is the correct patch for #107

#102.2 created issue #2943035: hook_field_widget_multivalue_form_alter and hook_field_widget_multivalue_WIDGET_TYPE_alter are not invoked for single widget that handle multiple values

1. +++ b/core/modules/media/media.module
   @@ -9,8 +9,11 @@
   +use Drupal\Core\Render\Markup;
   

   Unused

2. +++ b/core/modules/media/media.module
   @@ -172,3 +179,123 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +function media_field_widget_multivalue_form_alter(array &$elements, FormStateInterface $form_state, array $context) {
   ...
   +  $field_type = $context['items']->getFieldDefinition()->getType();
   

   I think this function deserves a nice

   /** @var \Drupal\Core\Field\FieldDefinitionInterface $field_definition */
   $field_definition = $context['items']->getFieldDefinition();

   at the top, so all further calls will typehint correctly.

3. +++ b/core/modules/media/media.module
   @@ -172,3 +179,123 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +      $add_url = Url::fromRoute('entity.media.add_form')->setRouteParameter('media_type', $create_bundles[0])->toString();
   

   Super unimportant nit, but
   $add_url = Url::fromRoute('entity.media.add_form', ['media_type' => $create_bundles[0]])->toString(); is way more common

4. +++ b/core/modules/media/media.module
   @@ -172,3 +179,123 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  $cardinality = $context['items']->getFieldDefinition()->getFieldStorageDefinition()->getCardinality();
   

   This is unused.

5. +++ b/core/modules/media/media.module
   @@ -172,3 +179,123 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  //   Research and resolve this in htttps://www.drupal.org/node/2943020.
   

   Too many t's

6. +++ b/core/modules/media/media.module
   @@ -172,3 +179,123 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  if (!empty($elements[0]['target_id']['#title'])) {
   +    $elements['#media_help']['#original_label'] = $elements[0]['target_id']['#title'];
   ...
   +    $elements['#media_help']['#original_label'] = $elements['#title'];
   ...
   +    $elements['#media_help']['#original_label'] = $elements['target_id']['#title'];
   

   This is what's causing the failures.

   In WidgetBase, if the cardinality is > 1, the " (value $delta)" part seen in the failures is appended to the title.
   Additionally, it is explicitly marked as '#title_display' => 'invisible' with a comment about how it's not expected to be rendered.

   If you want the correct title, $field_definition->getLabel() has it.

7. +++ b/core/modules/media/media.module
   @@ -172,3 +179,123 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +  // Only present the help text about autcomplete on autocomplete widgets.
   

   First one is "autcomplete". Also, out of context that sentence reads a bit snarky :) (but there's totally more text below it and it's fine)

Looks good to me 👍

Screenshots look like a good interim step.

1. +++ b/core/modules/media/media.module
   @@ -172,3 +178,128 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +function media_field_widget_multivalue_form_alter(array &$elements, FormStateInterface $form_state, array $context) {
   ...
   +  if (in_array($widget_plugin_id, array('entity_reference_autocomplete', 'entity_reference_autocomplete_tags'))) {
   +    $is_autocomplete = TRUE;
   

   Was there a reason why we didn't add new purpose built widgets that extend these instead of going down the alter path?

   We could limit their availability to just media ER fields with the isApplicable method.

   If the answer is, because we intend to remove all of this in the future and doing it as an alter hook means we don't have to support it forever but doing it as a stand-alone widget would mean we're supporting it till Drupal 9 - +100.

2. +++ b/core/modules/media/media.module
   @@ -172,3 +178,128 @@ function media_form_field_ui_field_storage_add_form_alter(&$form, FormStateInter
   +      $variables[substr($key, 1)] = $text;
   

   The theme system does this for you, if you declare the variable names in 'variables' in your hook theme.

   Was there a reason we went with 'render element' instead of 'variables'?

3. +++ b/core/modules/media/tests/src/Functional/MediaUiFunctionalTest.php
   @@ -192,6 +197,308 @@ public function testRenderedEntityReferencedMedia() {
   +  public function testMediaReferenceWidget($cardinality, array $media_type_create_access, $list_access, $widget_id = 'entity_reference_autocomplete') {
   

   Each of these provider cases installs a new Drupal site. Are we ok with that?

   If not, we can move to a doTestSomething() style approach where we feed the cases in, but have only one Drupal install for performance sake.

   Given we don't really need isolation here, because we aren't creating any content - we can change the config between each case.

this is just an interim fix

In that case, how about adding @internal to media-reference-help.html.twig with a link to the issue that will remove it? I also wonder if the theme system would allow us to prepend an _ to the theme hook name (- to the .html.twig name) as a further hint to contrib that this will get removed.

I'm ok with #120 being explored/done in a post-beta followup. Therefore, pushed #116 (plus the coding standards fixes in this interdiff) to 8.6.x and 8.5.x.