Needs work
Project:
Entity Embed
Version:
8.x-1.x-dev
Component:
CKEditor integration
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
26 Jun 2015 at 21:35 UTC
Updated:
4 Dec 2025 at 11:30 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
slashrsm commentedComment #2
slashrsm commentedComment #3
wim leersGreat suggestions, but seems like an excellent thing to add after a 1.0 release :)
Comment #4
dave reidYeah, we can leave this active for now, it's not tagged as a 8.x-1.0 blocker.
Comment #5
wim leersOkay, 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.
Comment #6
JamesK commentedSomething 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.
Comment #8
JamesK commentedOops, wrong version
Comment #9
yannickooNice James! The only problem that I experience is after updating the entity does not get update inside the WYSIWYG :)
Comment #10
yannickooI have created a pull request on Github or just check the attached patch :D
Comment #12
yannickooComment #13
slashrsm commentedMy 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.
Comment #14
yannickooFor sure we should make this configurable per button so I started to provide an
Edit onlycheckbox 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
I'm uploading my patch so that you can start from that :)
Comment #15
floretan commentedI took this patch a little bit further, but needed to change a few conceptual aspects:
Comment #16
floretan commentedThere 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.
Comment #17
bkosborneThis 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.
Comment #18
bkosbornePatch errors...
Comment #19
marcoscanoStill needs some love, but it doesn't hurt to trigger the testbot.
Comment #20
sudishth commentednot working #18 for me
Comment #21
icurk commentedWhen 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 327Does anybody have a clue why this is happening?
Comment #22
kasey_mk commented#18 is working for me. Drupal 8.5.5, Entity Embed 1.x-dev
Comment #23
wim leersWon'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?
Comment #24
wim leersLet'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.
Comment #25
ahebrank commentedReroll.
Comment #26
kasey_mk commentedThanks 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.
Comment #27
ahebrank commentedReroll.
Comment #28
hannessMarking as NW again, as both issues are fixed.
Comment #29
anruetherOne 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
Comment #30
geek-merlinOverriding 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:
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.)
Comment #31
geek-merlin(reverting erroneous IS changes)
Comment #32
kasey_mk commentedAnyone 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.
Comment #33
anruetherAs a sidenote: In #27 the form mode used for editing an entity does not seem to be configurable.
Comment #34
kasey_mk commentedRe-roll of #27 to remove code deprecated in D9.
Comment #35
xavier.massonRe-roll of #34 and add translation support.
Comment #36
chrissnyderRe-roll of #35 to add entity edit access check. The edit button only shows if the user has permission to edit the entity.
Comment #37
devkinetic commented@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.
Comment #38
webdrips commented#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.
Comment #39
danflanagan8The 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).Comment #40
devkinetic commented@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.
Comment #41
kasey_mk commentedThis 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
trueorfalseinstead of1or0since 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: 1becomesinline_entity_edit_enabled: trueComment #42
kasey_mk commentedUpgrading 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).
Comment #43
kasey_mk commentedRe-roll (just the lang version) for 1.4.0
Comment #44
porchlight commentedre-roll for 1.5
Comment #45
thatguy commentedPatch #44 works well but I noticed an issue that the cached entity data is shown when embedded to the editor. Added fix for that.