[PP-1] Allow media items to be edited in a modal when using the field widget

We needed this, so here's a patch against 8.6.x. There are some aspects of this patch that I don't like very much and it would need tests as well, but maybe it's still useful for someone else who needs this feature now.

@FeyP, thanks for the patch. Seems like a huge improvement already. I suggest adding some sort of indication that the title is the modal-trigger, perhaps and "Edit" label or something similar. But a great start!

We briefly started discussing this problem-space as part of the Barcelona Media sprint.

Since media items are re-usable, several concerns appear as well:
- Should we prevent editing of items already present in the widget, or all items can be edited?
- What is the best way to inform users that the modifications they do in the edit form are going to affect all "usages" of that asset?
- Would this be mitigated by the concept of "per-placement overrides"? (So the edit in the modal would affect only that instance, not the canonical entity fields)
- Etc.

I've seen some client projects specifically lean towards not exposing an (Entity Browser) edit button at all at the widget level. By forcing editors to go to the canonical route and edit the media there, it's likely easier to understand that modifications done to the entity are going to spread across all usages. I understand however that this is a very site-specific simplification that we couldn't generalize in core.

I would ideally suggest a combination, so you could have "fields" / options that would be "per-placement overrides" and global fields too. Something like an alt-field usually should describe the content of the image - this is something I would then mark as a global field.

As per informing the user adding a simple help text somewhere would suffice IMO. I.e.: "This field is global and any changes will affect all usages of this media."

Would be amazing if per-placement overrides could be expanded to allow a media reference field to setup certain requirements such as minimum / maximum image dimensions, so that you can't select a media item that doesnt' match those requirements - basically what the image field currently does, but for media reference fields.

The problem with per-placement overrides was previously considered a nice to have but out of scope feature. I'd suggest we open a new issue for that (my first thought for that would be a separate field widget) and use this issue here to focus on providing a low-tech feature we can use right now.

The straightforward solution: We could easily add an "allow editing" widget setting that's disabled by default. That way, sitebuilders can use whatever their use case demands and existing and future installs would remain simplistic.

The roundabout way: Since the preview is using a separate media_library view mode already, we could alternatively rely on the "link to content" setting on the media entity's "name" field. If the name's linked, add the AJAX overlay editing - but only to left click and their touch equivalent! That way the user could still right-click to open the form regularly.
(Note: The media--media-library.html.twig template always links the entity label, even when the formatter is configured not to!)

As for indicating that this portion of the preview allows editing, how about this?

.media-library-item:not(.js-click-to-select) .media-library-item__name a[href] {
  /* Indicate that title clicking allows editing. */
  background-image: url(/core/themes/stable/images/core/icons/787878/pencil.svg);
  background-color: transparent;
  background-position: top left;
  background-repeat: no-repeat;
  text-indent: 20px;
}

(Note: The .js-click-to-select-trigger CSS class is added to any media_library media entity in media_library_preprocess_media(), instead of only for the view.)

Regarding the patch proposed by @FeyP: I don't think we should react only to .media-library-widget. We can either apply to all .js-media-library-item elements, or just those not being used as triggers (using .js-media-library-item.js-click-to-select to differentiate).

In HEAD, only mouse-triggering of the edit link is disabled, by:

/* @todo Remove or re-work in https://www.drupal.org/node/2985168 */
.media-library-widget .media-library-item__name a,
.media-library-view.view-display-id-widget .media-library-item__name a {
  pointer-events: none;
}

At minimum, we should also prevent keyboard-triggering this link, for consistency sake?

#9 A valid point, but the scope of the issue making the links actually work instead of doing nothing. I suggest we block keyboard input in a different issue.

Attached patch is a straight reroll of #4 against 8.8.x after #3020716: Add vertical tabs style menu to media library was committed.

I was reminded by a contrib issue comment that my concerns in #6 are even more relevant if we consider the scenario where an image media item is used as a default value for the ER ("media) field. That default value would be available as any other value on the entity_reference field, and if the editor isn't aware of what is the repercussions of the operation, they might end up changing the output of all parent entities that used the default media item.

I still feel strongly that the "Edit" operation shouldn't be part of the widget by default in core. Maybe a contrib module could expose that for advanced sites, where it could be made more explicit what the repercussion of editing the media item is?

Discussed this with @xjm and @seanB at Drupal Dev Days in Cluj. We are in agreement that this functionality is probably too risky, from a UX perspective, to do in core. UX confusion could easily lead to data loss unless we were super careful, and we simply don't think it will add the kind of value that would justify the effort do this right. We think it could probably be done in contrib or custom code, so we'd rather leave this to that domain. Sorry!

Closing this one out.

In fact this is a feature *every* client asks.
It's a pita if you need to change a youtube url when you got 10 000 media items.

Let's try the patch...

Patch works. I hope we still can get this feature in the end as a setting. (not enabled by default)
It's not because it can confuse some users that we shouldn't add the major UX improvement for the more experienced webmasters.

In fact this is a feature *every* client asks.

Same case.

IMO I don't think this issue should be closed. I think at minimum there should be a link that allows you to navigate the media item. It takes way to many clicks to navigate to the individual media item when on the node edit form. Also could be difficult for an editor to find said media item with the media list.
I enjoy the media library module and it solves many issues, but not being able to edit a media item within a node or at least a link to the media item is a blocker for me and my team.

@and_daz a link (which sadly cannot be opened in a new tab) is already provided by this contrib module: https://www.drupal.org/project/media_library_edit

But however we used entity browser for a very long time and were happy to switch to core media library, but this is a mayor drawback which prevents us to use the core media library on our customer projects.

I'm sorry if I am not allowed to, but the community voices above lead me to the belief that this should indeed be considered, therefore I'm reopening it.

After playing around with various ideas to make it easier to update images when using Image Widget Crop, including the module mentioned in #18, the clearest solution was also one of the simplest: a link to the media library content list. This shows that the user is editing the media entity and not the referencing entity.

For example, to add a link to the Media Library widget:

use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Url;

/**
 * Implements hook_field_widget_form_alter().
 */
function MY_MODULE_field_widget_form_alter(&$element, FormStateInterface $form_state, $context) {
  $plugin_id = $context['widget']->getPluginId();

  if ($plugin_id == 'media_library_widget') {
    $element['media_library_edit'] = [
      '#type' => 'link',
      '#title' => t('Edit media items'),
      '#url' => Url::fromRoute('entity.media.collection'),
      '#attributes' => ['target' => '_blank'],        
    ];
  }
}

The problem we had with in-place editing using Inline Entity Form was that it looked like the user was editing just that instance or reference rather than the media entity itself.

Attached allows AJAX requests within the modal to work, such as replacing the current file.

I strongly agree that being able to edit the media item from the media field on a node (or whatever) is very important!

I agree with previous commenters that this is a significant drawback from switching from entity_browser/IEF to core media library, and that makes me very sad :(

Anyway, just wanted to chime in. Thank you!

Previous fixes worked fine but last version have css rule "pointer-events: none" that ruin clicability of media item.
Patch works for 8.7.x version, since 8.9.x doesn't have css folder at all.

We should update the rendered media in the field. I'm not sure what is the best way. Something like this?

Oops, the previous patch only works once. This should fix it.

Tested in Drupal 8.9-dev, PHP 7.1 with patch #25. No errors logged, but I don't see any functionality changing.

Tested on 8.8.1 on PHP 7.1.25 with patch #25, same as @Karolus, no errors, but no functionality change either.

@YahyaAlHamad, could you delete media_library.widget.es6.js.orig and media_library.module.orig files from #25 patch?

Kindly review a new patch , rerolling patch.

Adding the path *without* media_library.widget.es6.js.orig and media_library.module.orig files. I believe *.orig files were added by mistake and the files are temporarily andwere created by composer.
This is exactly the same patch as we have #29, just without extra data.

Tested patch #31 from @RuslanP on 8.9-dev, running on PHP 7.3. Could apply the patch cleanly, no errors logged, but no functionality changes were experienced.

Because the Media Library team have committed this: https://www.drupal.org/node/3039829

They removed the <a> wrapper in the template, causing this patch to... have nothing to do :-)

I would suggest installing https://www.drupal.org/project/media_library_edit 'till a better UI / core update is delivered.

greets,
Kim

PS: Wanted to post-back to you guys since it took me nearly 2 hours of sherlock-holms'ing through git to find this.

So, trying to re-add the edit link using the media_library_edit, wasn't working...

I decided to just re-add the hyperlink around the media item's name, using a preprocess hook rather than a tricky template override.

for anyone looking for a simple solution, best added to a module:

/**
 * Prepares variables for media templates.
 *
 * Default template: media.html.twig.
 *
 * @param array $variables
 *   An associative array containing:
 *   - elements: An array of elements to display in view mode.
 *   - media: The media item.
 *   - name: The label for the media item.
 *   - view_mode: View mode; e.g., 'full', 'teaser', etc.
 */
function MODULE_preprocess_media(array &$variables) {
  if ($variables['elements']['#view_mode'] === 'media_library') {
    $media = $variables['elements']['#media'];

    if ($edit_template = $media->getEntityType()->getLinkTemplate('edit-form')) {
      $edit_url = Url::fromUserInput(str_replace('{media}', $media->id(), $edit_template));

      $variables['name'] = [
        '#type' => 'link',
        '#title' => $variables['name'],
        '#url' => $edit_url,
        '#attributes' => [
          'class' => ['media-library-edit__link'],
          'target' => '_blank'
        ],
      ];
    }
  }
}

#32 works perfect, thank you @nightlife2008!

Just a small remark: make sure to add
use Drupal\Core\Url;
on top of the MODULE_preprocess_media function.

@nightlife2008 thnaks for the simple solution! But the media_library_edit does work, when you enable the checkbox on the widget, and then it looks nice/user friendly. Opening in a modal would be even nicer (https://www.drupal.org/project/media_library_edit/issues/3034205)

This patch is for 8.9.x

1- We still need a way to preview the changes when hitting submit.
2- Why don't we just add a new button to the media widget to handle editing?

This patch does the following on top of patch number #31:

• Add a new button "Edit".
• Change the old unreliable way of updating the media item with a new one. JS click event to tag the media we are currently editing.

Images and interdiff attached.

Things needed:

• Some styling. I'm not the best designer, sorry. Maybe we could make the button float above the image with an "Edit" icon
• Editing more than one media item could result on the wrong media item being updated.

Changed a line of code in JS to make sure that any "selected-media" class is deleted.

We also don't want being spammed with status messages due to editing media in ajax.

• Remove status messages on submit.
• Remove useless line.

Hmm, I didn't expect to upload that many patches. Anyway. Remove old lines that don't do anything right now, they were added to help to update the media item as we submit, now we don't need them.

Uploaded the wrong patch, I really need to sleep.

I reapplied the patch to Drupal 9, and I've added a few lines on the display because it would insert a button "Edit" above the media title. I'm using adminimal_admin_theme and I'm not sure if the same applies to you guys, feedback is very welcome.

@fmueller_previon
Need detailed steps to understand issue which u have fixed
I will be testing this on drupal 9.1 x dev branch
Do i need to install/enable any other module after adding the patch

@samiullah I have media and media_library installed, and this is a regular media reference field on a content type which has the Media Library formatter. When applying the patch it gives me the edit button to edit the media in the modal window - pretty basic :-)

We still need to apply some access handling, I think the edit button always shows.

#44 looks good
Needs usability testing as well before moving to next steps

There an issue in Media page "/admin/content/media" when you try to edit the media you need to save the form twice the form alter media_library_form_media_form_alter work in this page and change the action button

          $form['actions'][$key]['#name'] = 'media_library_ajax_submit';
          $form['actions'][$key]['#ajax'] = [
            'callback' => 'media_library_media_form_ajax_submit',
          ];

and cause the issue

@Firass Ziedan I noticed the issue as well.

Patch from #43 works for me with Drupal 8.9.13. I think this is a very important and popular feature that should be in the core media library module.

Hi,

I had applied the patch with Drupal 9.2.x and it worked as designed.

Below is my review:

Before applying the patch
1) I have added media field widget into article content type to test.
2) I have uploaded an image but there was no edit functionality
Below is the screenshot before applying patch.

After applying the patch
1) There is edit button over the image.
2) Below is the screenshot after applying patch.

Thank You

Applied patch #44 and it works fine.Adding screenshots below.

Before patch:

After patch:

Edit window

This is applying #43 but with styles from #44. I was also seeing the same issue of Edit button overlapping the attributes.

Patch from #56 works well.

Re-rolled for 9.2.x and fixed CS issues

I have tested patch #44 on Drupal version 9.1.4 and #58 on Drupal version 9.2.x-dev. Both are working as expected.

Bumping back to needs review because there's still "Needs usability review" tag.

Thanks for manually testing this, everyone!

I think that we still need a good amount of work here before this is ready to be committed. Code, at this point, is less important than getting sign-off from the usability and accessibility teams. So this will probably need to be presented to the usability team on their weekly(?) call for discussion, and potentially implement changes they ask for. Same goes for the accessibility team (except they don't have a call that I'm aware of, so we might have to ping individual accessibility maintainers in Slack).

Besides that, we'll need extensive automated test coverage of this change. We'll need functional tests, not so much unit or kernel tests. If anyone would like to take a crack at writing those, there is documentation/tutorials about writing tests available at https://www.drupal.org/docs/automated-testing/phpunit-in-drupal. That said, I'd recommend we hold off on writing tests until we have UX and accessibility sign-off.

A reroll for 8.9.x.

BTW in #62 there was an existing Drupal.behaviors.MediaLibraryWidgetEditItem so I combined the two together, if there was a better way to do that please let me know.

#62 doesn't work, am working on fixing it.

Fixed #62, sorry about that.

This still needs work per #61.

I just tried the patch in #62 and realized this issue isn't about what I thought it was about so I'm wondering if there's an existing issue for what I deem to a be a bigger problem for me which is that there's no way to "go back" or to edit a media entity that you just added to the media library, a media entity that currently has no usage records.

Example: I've created a remote video media entity for use in a node. I want to allow my client to also upload a preview image to be shown over the video before you start playing it and I thought it would be cool if they could do everything on the remote video entity instead of me creating 2 separate fields on the node, one for the video and one for the preview image.

The problem: My client beings creating a new node, uploads the video and immediately clicks "Save" to add it to the library so that it can be inserted into the node, BUT they then remember they should upload a preview image as well, but now there's no way to go back and change anything about the media entity they just created.

This problem just grows exponentially the more fields you add to a media entity in my opinion. Things like meta information, categories, description etc.

I'm wondering if I'm overlooking something obvious here. For me such a feature has no UX/accessibility implications compared to what this issue is trying to solve because this media entity currently isn't in use anywhere, we just want to ensure it's been created correctly before being used for the very first time. At the moment I'm adding two separate media fields to the node, one for video and one for the preview image because I know that will be so much more intuitive for my clients and lead to much less support for me.

And to actually write something on topic: In my experience the problem this issue is trying to solve is almost impossible to solve in practice in the way that's being approached here because "anyone" would expect that when you edit an image in an image field on a specific node that you're only editing that specific image or usage of that image. The media module would have to create a new instance of the media entity in my opinion for the feature discussed in this issue to be useable by me. It's the only way to solve this in a way that wouldn't cause backlash from my clients. Or at least I feel this should be the default option and the option of actually editing the original media entity should be an alternative available to users who really know what they're doing.

There was a slight problem with the selector, .find('selected-media') should be .find('.selected-media'). Changed the parent selected to be $(this).closest('.media-library-selection') instead of $(this).parent().parent()

Also, I added the changes on the es6 file and build it to make sure everything is good.

Moved the latest changes from #69 patch to #58 for Drupal 9, also, properly patch es6 file instead of the es5.

Re-rolled patch #70. Attached interdiff for the same.

Something is wrong in the AJAX/jQuery. I think we need to re-run something or re-attach something after the submit. Try this:

1. drush site-install standard
2. drush en -y media media_library
3. drush uli
4. https://drupal-9.2.x.ddev.site/admin/structure/types/manage/page/fields - add an image field to the basic page
5. https://drupal-9.2.x.ddev.site/node/add/page - first notice that ckeditor is attached to the body, then add media & submit, notice that ckeditor is still attached to the body, then edit media & submit (which is essentially using this patch) and notice that ckeditor is no longer attached to the body.

The #69 patch works well for me with Drupal 8.9.16.
What @douggreen mentions in #73 is not my experience, but Ckeditor is only used with Paragraphs, not in a bodyfield on the node itself.

Also tested this with Drupal 9.2 using the #71 patch on a content type with only a body field and a Media image field.
The patch works as expected, no problems with ckeditor and the body field. No errors in the console either.

I tested this patch with Drupal 8.9 and in my opinion there might be an usability issue with this approach when a site has more than one language and end users are able to translate both content types and media entities. There is a risk that user accidentally changes the language of the media item when they are working with content translations.

Let's say that the site in question has two languages: English and Finnish.

The user starts to create a Finnish content and uploads a media item. Now they can edit it nicely and change for example the alternative text of the image.

Now in the next step they start to create an English translation for the page. Now when they click on Edit media item, they are shown the media edit modal where the alternative text is in Finnish. It would really simple to do the mistake here and change the media items language into English and change the alternative text and think that now you have created an English translation for the media item. When in fact the user just changed the original language of the media item.

It would be nice if the edit button would edit the media item in the current language and not show the language selector to prevent accidental changes. If there is no current translation for the media item, it could open the translation dialog.

I've tried #71 and have run into the problem that the op key gets renamed - but is required for the correct triggering_element to be set. I'd initially thought this was a Core problem, but was able to trace the origin to this patch (#3167909-8: Media Library widget unintentionally removes items when used from a media item's edit form). We noticed the issue on gallery type media items (that reference other media items), which would cause every AJAX operation to additionally trigger removeItem.

We also need to consider the issue of nested modals #2741877: Nested modals don't work: opening a modal from a modal closes the original, e.g. when users want to edit a media item, within which they want to specify a link (there's a good illustration of this in said issue).

> It would be nice if the edit button would edit the media item in the current language and not show the language selector to prevent accidental changes. If there is no current translation for the media item, it could open the translation dialog.

Simpler, and to avoid having to navigate through multiple forms in the modal, show an 'Add $language translation' button instead of an 'Edit' button when there is no translation for the host entity's current language.

Not sure if this is the "Drupal way", but we configured multilingual sites to in a way, that a translatable content type will only have an untranslatable media field and then we hide untranslatable field when editing a node's translation. Media entities, on the other hand, are still translatable and by default the correct translation of the media item (if available) is shown on the respective translation of the node.

> that a translatable content type will only have an untranslatable media field and then we hide untranslatable field when editing a node's translation

It's correct in general to make reference fields untranslatable, so your setup does sound like it's following the 'Drupal way' :)

The actual referenced entity is translated, and then the correct translation of that is shown by the reference field:

- node 1 in en -> shows media 2 in en
- node 1 in de -> shows media 2 in de

So for media, the media field is set to untranslatable, and the media entity has different translations. (The exception to this is perhaps the actual file field on the media entity, because an image might contain text, or a video might have speech, and you'd want different versions of that for different languages.)

Untranslatable fields normally still show on a translation form, with a note appended to the field label saying 'All translations'. If however the entity is configured to allow pending revisions, then the field is hidden (but this might change in future: see #2940575: Document the scope and purpose of pending revisions).

- In the first case, the media library field says 'All translations' on it. Is it then ambiguous to the user which translation an edit button will open? I suppose we can resolve that by making the button say 'Edit current language' / 'Add translation for current language'.
- In the second case, there's no field at all!

> - In the second case, there's no field at all!

Yes, but that is exactly the problem. No field widget means no opportunity to show something. And references like that are a special case, you still need to edit/add a media translation and right now, that's quite hard to do.

It is possible to bypass that hide-logic, paragraphs.module is doing that and there is a corresponding issue in the entity browser queue: #3031439: Add Translate button for translate-able reference entities in entity browser form widget

Thanks for the info @Berdir.

In case someone else works on this before I do, the bypass is done like this:

    // Signal to content_translation that this field should be treated as
    // multilingual and not be hidden, see
    // \Drupal\content_translation\ContentTranslationHandler::entityFormSharedElements().
    $elements['#multilingual'] = TRUE;

With that fixed, then I think #3221720: Improve workflow for translating Media entities with the Media Library would be covered by this too, as the workflow for translated node + media becomes:

1. create EN node
2. add EN media, upload new file
3. translate node to DE
4. click 'add DE translation' on the media field
5. translate media to DE

I would once again caution folks that we will need, at a minimum, to get this into core:

• Review and sign-off from the usability and accessibility teams
• A fix, or at least a plan, for the translation issue that @joachim brought up
• Many automated tests so we could be sure this functionality works in as many scenarios as possible
• Plans/fixes/testing of whatever other integration bugaboos we don't know about yet

That's a lot of work. I strongly suggest that this issue be closed in favor of Media Library Edit, because it is likely to take a very long time (years) to get into core on its own, given how rigorous the process is, how many variables there are, and the limited availability of people to work on and review it.

If we had a working, battle-tested prototype of this functionality in Media Library Edit, with working (and tested) support for translation, moderation, Layout Builder, etc., then that would certainly be a stronger case for inclusion in core. A lot of the potential problems would have been sussed out and fixed. Doing this in contrib right now is highly preferable do doing it directly in core, and guaranteed to yield fruit sooner.

How can we get https://www.drupal.org/project/media_library_edit to be covered by Drupal’s security advisory policy. and remove the following comment from the media_library_edit module page?

If you'd rather patch core for this functionality, follow issue https://www.drupal.org/project/drupal/issues/2985168 . When merged to core, this module will be obsoleted.

Can we put the unpatched version of media in front of the use-ability and accessibility teams with the following scenario?

Create article node
Add media to field - crop it, fill out all the fields, get it all perfect.
Save media
Add article body text
Realize you misspelled something in your alt text, and go back and edit it.
Save node

> If we had a working, battle-tested prototype of this functionality in Media Library Edit, with working (and tested) support for translation, moderation, Layout Builder, etc., then that would certainly be a stronger case for inclusion in core.

Getting stuff done in contrib has merit, and it has the advantage that developers can install a module rather than have to patch core.

But there is the potential for a lot of effort to be wasted. If we build things in contrib *without* waiting for review and sign-off from the usability and accessibility teams, then a lot of time and energy will go into something that might have to be completely rewritten because of usability and accessibility concerns. Look at Entity Browser in contrib which AFAIK was the starting point for Media Library in core -- it's markedly different, and I'm guessing there was probably a significant rewrite to get it into core.

We should get usability and accessibility review early in the process, regardless of where it happens.

See #3023807: Override media fields from the reference field and https://www.drupal.org/project/media_library_media_modify for a really nice UI for editing media entities in a modal.

It's going to clash with requirements here though :/

What about adding an edit link to the items inside the media library? That would fit into our "What happens in the media library, stays in the media library" mantra. It also wouldn't clash with a possible "edit in context" functionality that could be triggered from the media item in the field widget.

I think for the edit inside the media library a views field would be nice, that can be added/removed to the media library view by the site builder.

I like the suggestion in #89, though does a modal summoning another modal work ok?

I like the suggestion in #89, though does a modal summoning another modal work ok?

Unfortunatly not, see #2741877: Nested modals don't work: opening a modal from a modal closes the original

I like the suggestion in #89, though does a modal summoning another modal work ok?

I wouldn't do that. Edit icon on the media item like in the screenshot and then directly replace the media library content by the edit form. After saving the media item, the library will be shown again.

> I wouldn't do that. Edit icon on the media item like in the screenshot and then directly replace the media library content by the edit form. After saving the media item, the library will be shown again.

Wouldn't that mean that the selection in the library modal is lost when the user returns to it?

Isn't the point of modals that the form the modal covers keeps its state?

Selection is kept in the same way like you upload a new image

I am currently working on this and will open a MR soon. Still have some bugs to fix.

The steps for updating a media item in #95 (if I’m not mistaken):

1. Click “Add media”
2. Search for your document
3. Click the edit icon

Step 1 and 2 are kind of confusing and not sure why we would force users to take those steps?

I think the problem we should be solving is:
A user has referenced a document (PDF) in an article and is responsible for updating the document or its metadata. So you open your article and expect an edit button for your document, but you don’t see one.

Having something like Media Library Media Modify with “context specific overrides” makes it even harder. I would suggest that context specific overrides should happen in the same form as generic edits. Maybe just have 2 save buttons (we should probably have better labels):

• Save changes for this article
• Save changes everywhere this media is used

Users most of the time know they want to change something, but they don’t know whether the thing they want to change is something they can change contextually, or generically. So they probably shouldn’t have to choose or think about that. They should in my opinion just:

1. Click an edit button directly from the widget
2. Make their changes in a modal
3. Decide on save whether to apply the changes for the current article only, or for all places the media item is used

Since we don't have “context specific overrides” in core yet, I think the modal in step 2 should (for now) explain that all changes you make will be for all places that reference the media item. But in an ideal world we should no longer have to do that when we implement something like contextual overrides in core (which should be super awesome!!).

In general, I agree with @seanB. Users know they need to change something, so we should let them do that. But the nature of entity references means that we inherit UX problems from this, and we should enumerate those problems, and develop some sound strategies for attacking them, before we implement stuff. This issue has always been a UX problem first and foremost, not a technical one.

Since we don't have “context specific overrides” in core yet, I think the modal in step 2 should (for now) explain that all changes you make will be for all places that reference the media item.

The problem with a message like "These changes will apply everywhere the media is used!" is that people will immediately wonder: Where else is this media used, and how do I see a list of those places? There's no way to do this in core, obviously, but I think Entity Usage might have that capability. So that sets up an eventual need to move something like the Entity Usage module, or at least its API, into core. Or some way for Entity Usage to plug in to this edit screen so it can present users with a link to the list of usages.

My first feeling is we should probably create some designs for each step in the ideal user flow. After we agree on the ideal world scenario, we can split that up in manageable tasks and implement them step by step.

1. The media library widget containing an edit button, which we already have a design for in #3023807: Override media fields from the reference field
   
2. The edit media modal
   1. Only allows editing the generic media item
   2. Allows editing the generic media item, as well as allowing contextual changes
3. Show a list of places that reference a media item

I hope we should be able to do 1 and 2.1 in this issue, and hopefully can do 3 in a followup (since that is probably an epic on its own). I think 2.2 is probably a very nice improvement but probably also an epic on its own (I think we have #3023807: Override media fields from the reference field for that).

> Save changes for this article
> Save changes everywhere this media is used

This 2-button idea won't work if the media already has overrides -- how will the form know which values to show when it's opened?

This 2-button idea won't work if the media already has overrides -- how will the form know which values to show when it's opened?

This is honestly one of the biggest challenges we have to fix in #3023807: Override media fields from the reference field.

One option would be if you have saved overrides, we could simply show the original value (not editable but as text) above or below the form field to show there are currently 2 values (an original and the contextual override). The form fields contain the override values. We could probably still offer "Save changes for this article" option to update the overrides, and "Save changes everywhere this media is used" to save the values generically and move the overrides back to the original media entity.

Maybe we should create a new issue to create and discuss designs for this, and block this issue and #3023807: Override media fields from the reference field on that one?

> Maybe we should create a new issue to create and discuss designs for this, and block this issue and #3023807: Override media fields from the reference field on that one?

Yeah, it does seem like both should be looked at together.

My immediate idea is to show a modal with 2 tabs, the actual Media entity and the overridden one.

@phenaproxima and I presented this issue at the UX meeting 2 weeks ago. Sadly, there was no clear suggestion on how this issue should be solved.
As a step forward, I put the functionality of #95 into the latest release of
https://www.drupal.org/project/media_library_media_modify. So hopefully we can find a good solution there and put it then later into core.

Patch #71:
Actual: Url::fromRoute('entity.media.edit_form', ['media' => $media_item->id()])->toString()
Recommended: $media_item->toUrl('edit-form')->toString()

I am not sure, but I think the "Edit" button shouldn't be there if the user has no access to edit the media item.

<?php
$element['selection'][$delta] = [
  // ...
  'edit_button' => [
    '#access' => $media_item->access('update'),
  ],
  // ...
];
?>

Edited:
I tried it without sufficient permissions.
When I press the "Edit" button then the AJAX call is broken, nothing happens just the throbber flickering, and there is an entry in the DBLog:

Path: /media/65/edit?_wrapper_format=drupal_modal. 

Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException: The following permissions are required: 'update any media' OR 'update own media' OR 'image: edit any media' OR 'image: edit own media'. Drupal\Core\Routing\AccessAwareRouter->checkAccess() (File: /.../core/lib/Drupal/Core/Routing/AccessAwareRouter.php, line: 120).

@Sweetchuck, the patch works very well in 9.3.0-beta1, I was expecting a pencil icon however the edit button does do the job and the patch does what is needed.

Thanks

The merge request does not apply to 9.3.0-beta1 and is too far behind head, so for now use patch #106

Yes we do need this, it would be really nice to have this feature.

For now, I don't think this issue is going to go anywhere fast, because there are too many outstanding UX questions with no clear solutions we can work towards. Any feature like this must clear the core UX and accessibility gates, which means that anything we come up with has to be reviewed and signed off on by the UX team and accessibility topic maintainers.

The media team knows from hard experience that it's best to come up with a plan first, with close guidance from and collaboration with the UX and a11y folks, before implementing anything. If we want to experiment with different ideas quickly and see what, if anything, sticks...that's exactly what contrib is perfect for. As @chr.fritsch mentioned in #103, he has put the functionality he built into a contrib module. Media Library Edit is another one I'm aware of.

Therefore, given both the need for this feature and the lack of direction/plan for core inclusion, I think what might be best is to postpone this issue. What we need to continue is probably dedicated funding and time from the media team, UX team, and accessibility experts to come up with a version of this feature that would be suitable for core.

@phenaproxima , I tested all three of those suggested options,

the two that worked are patch #106 (works well) and the contrib module media_library_edit worked well also although it did require an additional configuration step.

the other module media_library_media_modify crashes on my build of 9.3.0

So to others, I'd recommend checking out media_library_edit as was mentioned (requires enabling the edit functionality through the entity form settings or consider patch 106 as it requires no additional configuration other than choosing the media_library option in the entity form settings which is likely already the case for most, it seems to be "close" to being ready functional wise without getting too picky on if we use a button or a pencil icon (I'd prefer a pencil but am very pleased either way it goes).

Great work, please don't give up on this one, it's a worthy improvement.

> What we need to continue is probably dedicated funding and time from the media team, UX team, and accessibility experts to come up with a version of this feature that would be suitable for core.

Agreed, this needs a lot of UX work to get the UI right.

Should we at least create a new issue for the overarching UX work that is needed for this and #3023807: Override media fields from the reference field?

> I was expecting a pencil icon however the edit button does do the job

It moves the (X) that removes the image as well.

Was there an earlier patch or version of the MR that had the pencil button, or did I dream I saw it in action because of all the screenshots of the media modify button?

I appreciate that despite this issue being set to postponed, people are going to want to keep the work so far up to date with Core so they can use it in their projects (me included!), but it's getting a bit confusing to have a MR *and* patches.

@joachim you are talking about the module version of the patch.
https://www.drupal.org/project/media_library_edit

When we edit the existing image/file then the file is replaced correctly but the name and the thumbnail of the file/image remain the same as the previously existing image/file.

Add media library state into edit link, this patch based on D9.2.x

i've had issues with multiple domains + the edit button wouldn't refresh the media item anymore.

These have been resolved in: https://www.drupal.org/project/drupal/issues/3266293

More specifically:
Added: 'language' to the edit button:

            'href' => $media_item->toUrl('edit-form', ['query' => $state->all(), 'language' => $entity->language()])->toString(),

and
in the javascript to add the "selected-media" class:

        $(this).closest('.form-actions').find('article').addClass('selected-media');

Add #form_mode into opener parameters, this patch based on D9.2.x

I've filed a parent issue for the UI design work.

I rerolled #119 patch for 9.4.x
(The patch applies on 9.3 as well)

#121 works for me! Thank you :-)

I find if synchronize submitted media edit form have other ajax button, when other ajax button submit, the op buttons will be change the name.
This cause media edit form synchronize submit miss `triggering_element`.
I reduce hook_form_alter scope for limit of media library.

>124

Did patch #125 pass? is the recommendation to use a contrib module for the time being?

@emb03 Result of patch #125

04:05:35 Applying patch /var/lib/drupalci/workspace/jenkins-drupal_patches-150272/ancillary/2985168-125.patch to /var/lib/drupalci/workspace/jenkins-drupal_patches-150272/source/
04:05:35 error: patch failed: core/modules/media_library/js/media_library.widget.js:35
04:05:35 error: core/modules/media_library/js/media_library.widget.js: patch does not apply
04:05:35 error: patch failed: core/modules/media_library/media_library.module:28
04:05:35 error: core/modules/media_library/media_library.module: patch does not apply
04:05:35 Patch Error
04:05:35 The patch attempt returned an error.  Error code: 1
04:05:35 Patch Failed to Apply

@phenaproxima Re:

What we need to continue is probably dedicated funding and time from the media team, UX team, and accessibility experts to come up with a version of this feature that would be suitable for core.

Out of curiosity, how does the above typically happen? (Specifically, dedicated funding and available time)

@Chris Matthews - I think the silence since your comment is your answer! Seriously though, I would expect the main thing to be a company with sufficient project need and budget to emerge to sponsor the relevant people. Unless you're asking about how that's organised?

So yeah I was just curious what all is involved in obtaining said "dedicated funding and time". If it's "simply" a company with sufficient project need sponsoring the relevant people then that's cool.

Rerolled for 9.5

DO NOT USE the attached patch!, see next comment.

#132 is working for me on 9.5.0 + Gin theme: I get an "edit" button for each media item, clicking on it gets the corresponding edit form loaded in a modal; on submission, the modal gets the media item preview in the main node form updated correctly.

I'm also uploading a small variation of #132, with a minor tweak so it's compatible with #2895477: Native browser form validation does not fire when submit buttons use #ajax - #67

Just found that #133 only partially fixes compatibility with #2895477: Native browser form validation does not fire when submit buttons use #ajax - #67; a simpler approach is to just use #132 as is and tweak the patch in the other issue.

I took #132 and I did the necessary changes to Claro to display the edit button in the same way as the delete button, but with a pencil icon.

For people using this patch in combination with Gin, I created an issue over there with the necessary style tweaks to support the changes here: #3330410: Allow media items to be edited in a modal when using the field widget

Just tested #135 manually on 9.5.1-dev, it works correctly.

I'm also attaching a 1min video showing the tests performed.

@jedihe, @DieterHolvoet the demo of patch #135 looks great for D9.5.1-dev

however the patch needs a reroll for 10.1.x.

Re-roll for #135, it also applies on top of current 10.0.x

I'm also including a re-roll for 9.5.x.

Patch #139 for 10.1.x causes JS console errors, due to it using jQuery.once(). Switching to drupal/once.

Fixed missing 'context' parameter in the behavior.

Quick testing of #139 on 9.5.1-dev shows it working fine; same as shown in #137.

tried to fix CCF for #141.

Since comment #109 the issue status is postponed.

Good to see some people are trying to come up with an easy solution.

Maybe a good point to remind everyone, to resolve the tags attached to this issue, to move forward. Like needs tests; needs UX review; etc...

This is why the postponed status is very bad without specifying which related issue(s) has to be solved first.

Are we expect that to grab the attention of a "UX reviewer" for a postponed issue?

At least @joachim realised the problem in comment #111.

I know that to create a consistent UI with a good UX is difficult, especially in a complex software like Drupal, but very important.

I don't want to offend anybody, but the over simplified summary of this issue is:
To add an "edit" button somewhere in the most flexible CMS of the world with the most awesome community takes 4 and half years and still counting.

I see two options.

1. Close this issue with "won't fix" status, and redirect the users to the media_library_edit module. (I haven't checked if it works or not)

2. Give a clear guidance to the contributors how to solve this issue in the Drupal core.

Otherwise the maintainers just waste the contributors time and resources.

There is a good reason why there is so many users involved (followers, commenters) in this issue.
Without this feature a content edit form is a UX fail.

Applied the same changes that were made to the 10.1 patch in #140 & #141 to the 9.5 for the once function.

As far as I know the test failed due to the es6 file in the patch so I'll try again without the es6 file in the patch.

If this still fails the test maybe someone else could enlighten me with more information about the error that i've gotten.

My bad, I didn't know I had to use the yarn build function for the JS files.

Rerolled patch from #147 against 9.5.2. No other changes.

How does this patch solve the things listed in its parent issue? There is still a discussion going on in that ticket.
See latest comments #3266964: Design a UI to allow various kinds of alterations to referenced Media entities in a modal

As I understand it, the patches here are interim solution that people are posting to share with other who need this functionality now. They're not meant to be an actual fix to be committed.

@joachim correct, thank you for making that point clear. I have a client who relies on this workaround. The reroll was necessary for that reason, and I shared it to help anyone else who might also be relying on this patch as a workaround.

Reroll from #148 again 9.4.10. No other changes.

Re-roll of #142 for Drupal 10.1.x.

#148 works for me, running Drupal 9.5.3 + Gin theme.

While there are other related issues that would be ideal to solve in core all with one solution, in the meantime I think the Media Library Edit contrib module is a great solution to this particular problem. It works very well, supports D10 and appears to be fairly actively maintained.

In a very specific scenario, when a media entity has more than a single 'file' type field (i.e.: 'book' media entity, containing a 'file' field and a 'cover' field) this patch causes the editing form to not submit properly when adding/removing the second 'file' type field (the user has to click on the 'Save' button twice, so that the changes take place).

I have added some extra lines to make sure that the media library opener is actually the 'Field widget'. This avoids the annoying issue I described above. This patch is based on #148.

I suppose it would not be advisable to get the basic functionality of the Media Library Edit or the Media Library Media Modify contrib module in Core until as phenaproxima mentioned in #109 there is ...

dedicated funding and time from the media team, UX team, and accessibility experts to come up with a version of this feature that would be suitable for core.

It definitely would be nice to have a Core solution but the patch in this issue is working for me.

As described in #157 I came across the same issue where we had to press the 'Save' button twice.
Patch #157 fixes this and works great for me.

@MiguelArber could we perhaps have #158 in the MR for easier review? Or as separate MR otherwise?

I opened MR !4517 with #157 rebased on 11.x.

I am sorry for the delay, I just read the message posted by @Anybody, but I see that @dieterholvoet has already created an MR of the rebased #157. Please let me know if I can help on anything else.

PS: I'm glad to help, thanks for testing @Victornator.

patch #153 work with a Drupal 10.1.5

Updated the patch in comment #157 to work with 10.1, posting here in case anybody else needs it…

Apologies for the noise but am attaching another patch, #165 with a minor update: added an aria-label to the Edit button, in the same fashion as the one present on the Remove button.

This is getting confusing to work on. There's many patches, and two open merge requests. To keep this sane and easier to review, please let's stick to updating MR !4517 only.

I've updated it to reflect the changes #166 and made another minor fix for a PHP warning.

I also agree with comments above that this shouldn't really be postponed without a related issue that it's waiting on. Until we have that, setting this back to "Active". I acknowledge this issue isn't likely to get anywhere without involvement from the UX team, but having the issue set to Postponed doesn't seem that will make it any more likely to happen.

This is postponed on #3266964: Design a UI to allow various kinds of alterations to referenced Media entities in a modal which is marked as a parent.

And it should be set to postponed because there's no point people working on this patch until a UI has been designed.

Thanks @joachim, missed that.

Note that I think the original merge request is what was split off into a module Media Library Media Modify. This allows overrides of fields from the referenced media. AKA contextual edits. That functionality differs quite a bit from the latest patches/MR from this issue, which now just focus on presenting a simple edit button to edit the media item in a modal. Those changes affect the media everywhere it's used. It's simpler and I think has a much better chance of getting into core.

I hope we can agree to keep this issue focused on the simple use case.

> That functionality differs quite a bit from the latest patches/MR from this issue, which now just focus on presenting a simple edit button to edit the media item in a modal. Those changes affect the media everywhere it's used. It's simpler and I think has a much better chance of getting into core.

I agree that the override functionality is a bit specific.

But for core we need editability AND translation, which is two items. Overriding is just a third, but the need for a UI pattern is already there.

I created a patch for 10.2 based on the MR @bkosborne updated.

merged 11.x into !4517

Snapshot of latest state of MR for safe usage with composer-patches on drupal/core:11.1.1

Updating #174 (2985168-175.patch) to include a hunk from #172 that seems to have been missed or accidentally removed (2 hooks in media_library.module).

The re-added hunk:

• Hides the "Delete" button from the modal
• Makes the modal "Save" button submit via AJAX
• Gets the media-item in the node edit form updated with any changes saved using the modal

Minimally tested, it works well with Drupal core 11.1.7 + Gin theme.

#175 works great in v11.1.2

#175 applies to 11.2 either. Thank you.

I added the missing code mentioned in #175 to the MR and simplified it.

My reading of the conclusion of the usability review meeting in July is that we are cleared to go ahead with the proposed solution of using the contextual link pattern.

So back to needs work, and indeed needs a re-roll.

This should work for 11.3.0

Updating patch in comment #184 to include the same tweaks as #175 did. This applies cleanly on 11.3.1.

Both patches from #184 and #185 seem to be manually created; they both differ from the code in the MR.

The code that was missing in patch #184, but was added in the patch #185, is also available in de MR. The MR however contains more changes; for example the changes from e4fd11c9 - Fix validation errors not being shown are not available in both patches #184 nor #185.

Attached is a snapshot of the latest state of the MR for safe usage with composer patches on D11.3 projects.