It would be nice if the user could right-click an embedded entity, click an 'Edit' operation, and have the dialog pop-up with the entity edit form. Saving the form (and the dialog closing) should update the preview of the entity.

CommentFileSizeAuthor
#44 entity_embed-2513086-edit_entity_lang-44.patch17.45 KBporchlight
#43 entity_embed-2513086-edit_entity_lang-43.patch19.24 KBKasey_MK
#42 interdiff_41-42_nolang.txt6.33 KBKasey_MK
#42 entity_embed-2513086-edit_entity_nolang-42.patch12.68 KBKasey_MK
#42 interdiff_41-42_lang.txt448 bytesKasey_MK
#42 entity_embed-2513086-edit_entity_lang-42.patch18.99 KBKasey_MK
#41 interdiff_37-41.txt406 bytesKasey_MK
#41 entity_embed-2513086-edit_entity-41.patch18.66 KBKasey_MK
#37 entity_embed-2513086-edit_entity-37.patch18.17 KBdevkinetic
#36 interdiff_35-36.txt514 bytesChrisSnyder
#36 entity_embed-2513086-edit_entity-36.patch18.17 KBChrisSnyder
#35 interdiff_34-35.txt8.44 KBxavier.masson
#35 entity_embed-2513086-edit-entity-35.patch18.12 KBxavier.masson
#34 interdiff_27-34.txt614 bytesKasey_MK
#34 entity_embed-2513086-edit-entity-34.patch12.19 KBKasey_MK
#27 entity_embed-2513086-edit-entity-27.patch12.19 KBahebrank
#25 interdiff-18-25.txt3.14 KBahebrank
#25 entity_embed-2513086-edit-entity-25.patch12.2 KBahebrank
#18 interdiff.txt6.87 KBbkosborne
#18 entity_embed-2513086-edit-entity-18.patch12.39 KBbkosborne
#17 interdiff.txt0 bytesbkosborne
#17 entity_embed-2513086-edit-entity-17.patch6.87 KBbkosborne
#16 interdiff.txt1.04 KBfloretan
#16 entity_embed-2513086-edit-entity-16.patch10.28 KBfloretan
#15 entity_embed-2513086-edit-entity-15.patch9.55 KBfloretan
#14 entity_embed-2513086-edit-entity-14.patch6.02 KByannickoo
#12 entity_embed-2513086-edit-12.patch1.94 KByannickoo
#10 entity_embed-2513086-edit-10.patch5.6 KByannickoo
#8 2513086-8.patch977 bytesJamesK
#6 2513086-6.patch984 bytesJamesK
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

slashrsm’s picture

Issue tags: +Media Initiative, +sprint
slashrsm’s picture

Issue tags: -sprint +D8Media
Wim Leers’s picture

Title: Right click option to edit entity » Right click option to edit entity: not reference a different entity, but *edit* the entity in a dialog
Status: Active » Postponed

Great suggestions, but seems like an excellent thing to add after a 1.0 release :)

Dave Reid’s picture

Status: Postponed » Active

Yeah, we can leave this active for now, it's not tagged as a 8.x-1.0 blocker.

Wim Leers’s picture

Okay, sorry. Was just trying to make it clear for those looking at the issue queue what still needed to be done.

In Drupal 8 core, things slated for 8.1 were also marked as postponed. That was very helpful. I hoped we could do the same here. But as the maintainer, it's of course your decision.

JamesK’s picture

Status: Active » Needs review
FileSize
984 bytes

Something more easily accomplished is adding an Edit button to the modal on the embed step (the last step, where the user selects the view mode).

This patch depends on having entity_browser with #2727031: Don't use OpenModalDialogCommand for modals fixed, for now, though we could easily recreate the edit dialog.

Status: Needs review » Needs work

The last submitted patch, 6: 2513086-6.patch, failed testing.

JamesK’s picture

Status: Needs work » Needs review
FileSize
977 bytes

Oops, wrong version

yannickoo’s picture

Nice James! The only problem that I experience is after updating the entity does not get update inside the WYSIWYG :)

yannickoo’s picture

I have created a pull request on Github or just check the attached patch :D

Status: Needs review » Needs work

The last submitted patch, 10: entity_embed-2513086-edit-10.patch, failed testing.

yannickoo’s picture

Status: Needs work » Needs review
FileSize
1.94 KB
slashrsm’s picture

Status: Needs review » Needs work
Issue tags: +Needs screenshots, +Needs tests

My 2 cents:

- I would make this feature configurable. There are cases where you wouldn't want Edit button. This could totally live on the embed button.
- Would be nice to have a test.
- If changes to display configuration are done and edit opened after that they are lost. Even more confusing; when user is returned to the embed display configuration it seems that changes are respected, but they are silently ignored when the form is submitted.
- If there are no other changes (other than Editing the entity) preview won't be rebuilt (shows embedded entity as it looked before the edit).

Would also be nice to add a before/after screenshot and embed it into the issue summary.

yannickoo’s picture

Status: Needs work » Needs review
FileSize
6.02 KB

For sure we should make this configurable per button so I started to provide an Edit only checkbox to the settings form (and yes we can rename that one :D)

I changed the only defined route to a real controller and that controller checks whether the dialog should be displayed in the regular way or whether the entity form is needed.

Remaining tasks

  • Implement AJAX behavior for the submit (delete?) button
  • Update the preview of the embedded entities

I'm uploading my patch so that you can start from that :)

floretan’s picture

I took this patch a little bit further, but needed to change a few conceptual aspects:

  • The configuration option is now called "Enable inline editing" to better reflect the behavior it enables.
  • The editing of the entity is now a dedicated step in the already existing modal form.
  • When double-clicking on an inline entity in the WYSIWYG editor or when using the right-click menu option, and when "Enable inline editing" is enabled, then the "edit_entity" form step is opened.
  • After saving the entity editing step in the modal, the user is sent to the "embed" form step where all the existing options are still available. When submitting that step the dialog is closed and the preview is updated correctly.
  • The editing form builds on top of inline_entity_form, so it works with or without entity_browser. The only issue I see with that is that we use the entity_browser_entity_form (part of entity_browser) module to be able to create entities directly in the modal, which itself uses inline_entity_form, but we depend on inline_entity_form directly for editing. This can be a bit confusing for site-builders.
floretan’s picture

Status: Needs review » Needs work
FileSize
10.28 KB
1.04 KB

There is one bug with the patch in #15: when editing an already embedded entity, the caption, alignment and selected view mode are lost.

Apparently the information provided by the editor doesn't stay in the form state between steps, so I needed to add the information to the form structure, which isn't that great, and it doesn't work for the view mode (which stores data as JSON). The attached patch works (as mentioned, with the exception of the selected view mode), but there should be a cleaner way to persist the information in the form state.

bkosborne’s picture

This is great! A huge user experience improvement.

I think rather than bringing users directly to the edit form for the entity, they should be brought to the normal embed dialog again and have the button there to edit if they want. My reasoning is that users will often just want to modify the alignment / caption / display mode of the the embedded entity and not the entity itself. In those cases they'd have to hit the "Back" button on the entity edit form.

Here's an updated patch that removes the direct entry to the edit form and hides checkbox for enabling the functionality of the inline entity form module is disabled.

Leaving this as "needs work" due to the issues outlined in #16. Some of this was over my head or I'd try and fix them. Hoping to dive in more in the coming days.

bkosborne’s picture

marcoscano’s picture

Status: Needs work » Needs review

Still needs some love, but it doesn't hurt to trigger the testbot.

sudishth’s picture

Status: Needs review » Needs work

not working #18 for me

icurk’s picture

When trying to save the edited entity, the console gives me an error
Missing file with ID image/png. in /var/www/html/web/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php on line 327
Does anybody have a clue why this is happening?

Kasey_MK’s picture

#18 is working for me. Drupal 8.5.5, Entity Embed 1.x-dev

Wim Leers’s picture

Won't this introduce as much confusion as it solves?

Won't this result in content authors expecting to be only updating the one embedding instance that they started the edit from, rather than it impacting all other places it was embedded too?

Wim Leers’s picture

Title: Right click option to edit entity: not reference a different entity, but *edit* the entity in a dialog » [PP-2] Right click option to edit embedded entity: not reference a different entity, but *edit* the entity in a dialog
Status: Needs work » Postponed
Related issues: +#2577887: Entity Embed 8.x-1.0.0 release, +#2577891: Entity Embed 8.x-1.0.0-rc1 release

Let's do new features after stabilizing Entity Embed; let's do it after #2577887: Entity Embed 8.x-1.0.0 release, which itself is postponed on #2577891: Entity Embed 8.x-1.0.0-rc1 release.

ahebrank’s picture

Kasey_MK’s picture

Thanks ahebrank; #25 lets us stay in business with the ability to edit existing paragraphs embedded within CKEditor's WYSIWYG.

The only thing we can't do is to change a referenced entity within that referenced entity; e.g., change the image selected in a media field within a paragraph - but being able to update the caption, link, etc. without having to delete and recreate the whole paragraph is really nice.

ahebrank’s picture

hanness’s picture

Status: Postponed » Needs work

Let's do new features after stabilizing Entity Embed; let's do it after #2577887: Entity Embed 8.x-1.0.0 release release, which itself is postponed on #2577891: Entity Embed 8.x-1.0.0-rc1 release.

Marking as NW again, as both issues are fixed.

anruether’s picture

One might think that contextual links could be used here. Only that they would have to open in a modal otherwise we would drive people away from a form with unsaved content. I haven't found anything on that, so throwing in the idea and tentatively relating to #3076283: Entity embed is breaking Quick Edit / Contextual links

geek-merlin’s picture

Issue summary: View changes

Overriding fields of referenced media (later any entity) is done in #2994702: Allow editors to alter embed-specific metadata, as well as `data-align` and `data-caption` and #3023807: Override media fields from the reference field.

So this is providing a simple way to edit the referenced entity. Which will change all references for that entity.

So i completely agree with @Wim Leers #23:

Won't this introduce as much confusion as it solves?
Won't this result in content authors expecting to be only updating the one embedding instance that they started the edit from, rather than it impacting all other places it was embedded too?

I'd say yes, editing in a popup suggests that the edited thing is part of the current thing which is misleading.

I'suggest changing this to an edit link with target=_blank. (This is not without problems, but imho far less problems.)

geek-merlin’s picture

Issue summary: View changes

(reverting erroneous IS changes)

Kasey_MK’s picture

Anyone else experiencing the modal not opening at the top of the form? When I edit an existing embedded paragraph, my modal window starts about 1/3 down the page.

anruether’s picture

As a sidenote: In #27 the form mode used for editing an entity does not seem to be configurable.

Kasey_MK’s picture

Re-roll of #27 to remove code deprecated in D9.

xavier.masson’s picture

Re-roll of #34 and add translation support.

ChrisSnyder’s picture

FileSize
18.17 KB
514 bytes

Re-roll of #35 to add entity edit access check. The edit button only shows if the user has permission to edit the entity.

devkinetic’s picture

@ChrisSnyder This is a noteworthy addition, but I believe you were looking for "update" vs "edit" for the operation. Here is a version of your patch from #36 with just that change.

webdrips’s picture

#17 doesn't work for custom fields added to the media type. In fact, it did not load the entity edit form in the way I was expecting at all.

I was hoping to be able to set custom field values, such as a taxonomy reference, for a document media type.

To clarify, I'm using this within a embedded entity in the WYSIWYG editor.

danflanagan8’s picture

The patch in #37 is throwing this error when I click the edit button:

Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "content_translation.manager". in Drupal\Component\DependencyInjection\Container->get() (line 156 of /app/docroot/core/lib/Drupal/Component/DependencyInjection/Container.php).

devkinetic’s picture

@danflanagan8 without digging into things too much, I can say that service comes from core/modules/content_translation. The patch adds logic that seems to see if the entity is translatable. I would try looking in src/Form/EntityEmbedDialog.php::initIsTranslating() and seeing why your entity is qualifying for translation. If it is indded translatable, perhaps the way it is performing it incorrect. In either case, if you need a workaround you can simply enable content_translation.

Kasey_MK’s picture

This is a re-roll of #37 with a schema added for the new configuration field under embed.embed_type_settings.entity.

Missing schemas discovered with config_inspector
installed and enabled - errors are on the Status report page (/admin/reports/status).

Please note that your configuration .yml files may need to be updated to use true or false instead of 1 or 0 since this field is now defined as a boolean and not an integer. For example, in config/sync/embed.button.[NAME].yml,
inline_entity_edit_enabled: 1 becomes inline_entity_edit_enabled: true

Kasey_MK’s picture

Upgrading to entity_embed 1.3.0 with this patch, I got this error when trying to edit an embedded entity:
ArgumentCountError: Too few arguments to function Drupal\entity_embed\Form\EntityEmbedDialog::__construct(), 6 passed in /app/web/modules/contrib/entity_embed/src/Form/EntityEmbedDialog.php on line 143 and exactly 7 expected in Drupal\entity_embed\Form\EntityEmbedDialog->__construct() (line 123 of /app/web/modules/contrib/entity_embed/src/Form/EntityEmbedDialog.php)

I figure we can add $container->get('language_manager') to line 144 so there are 7 arguments (entity_embed-2513086-edit_entity_lang-42.patch) or remove the addition of language/translation managers (entity_embed-2513086-edit_entity_nolang-42.patch).

Kasey_MK’s picture

Re-roll (just the lang version) for 1.4.0

porchlight’s picture

re-roll for 1.5