Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
It would be really useful to have an edit form for a selected entity directly in your form and not over an "Edit" button.
Proposed resolution
Use IEF to display the form in the FieldWidgetDisplay
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#30 | 2858438-30.patch | 16.77 KB | volkerk |
#20 | greyed-out.mov | 567.77 KB | oknate |
#19 | 2858438_19.patch | 10.4 KB | chr.fritsch |
Comments
Comment #2
chr.fritschHere is a patch
Comment #3
mtodor CreditAttribution: mtodor at Thunder commentedAdded missing schema.
Comment #4
naveenvalechaNice work @mtodor, Quick review to move this forward. Having tests will be great.
It would be nice to inject the inject the current service.
One of my favourite function but we need to remove it.
Comment #5
mtodor CreditAttribution: mtodor at Thunder commentedComment #6
mtodor CreditAttribution: mtodor at Thunder commentedLet's try to move this forward. All issues listed in #4 should be addressed with provided patch.
Comment #7
volkerk CreditAttribution: volkerk at Thunder commentedInlineEntityForm is returned as a entity display leading to paragraphs 'collapse' button triggering the wrong widget: (EntityReferenceBrowserWidget->extractFormValues()) which has no implementation for handling the state of the InlineEntityForm.
Also the structure of the form_state is messed up by this (additional 'display' key in parents).
Steps to reproduce:
1) Create node type with paragraph field.
2) Create paragraph type with entity reference.
3) Use EntityBrowser widget and configure its ENTITY DISPLAY PLUGIN to 'inline_entity_form'.
3) Create node, add paragraph, add referenced entity.
4) Save the node.
5) Edit node; edit paragraph; change values in paragraph form.
6) Collapse the paragraph form.
7a) Save the node > changes will be lost.
7b) Open paragraph form > changes will be lost.
Comment #8
LeStat CreditAttribution: LeStat at EPAM Systems commentedComment #9
LeStat CreditAttribution: LeStat at EPAM Systems commentedPatch update for 8.x-2.x.
Comment #10
LeStat CreditAttribution: LeStat at EPAM Systems commentedComment #11
Vidushi Mehta CreditAttribution: Vidushi Mehta at gai Technologies Pvt Ltd commentedComment #12
LeStat CreditAttribution: LeStat at EPAM Systems commentedTo provide compatibility with https://www.drupal.org/files/issues/entity-browser-view-context-2865928-...
Comment #13
LeStat CreditAttribution: LeStat at EPAM Systems commentedComment #15
pminfCould someone please provide a more detailed description or some screenshots what to expect? I assume, that after selecting one or multiple entities in the entity browser the corresponding forms of these selected entities should be open immediately. Using patch from #6 this is not the case and I also can't find an additional form display widget setting for this.
Comment #16
Primsi CreditAttribution: Primsi at MD Systems GmbH for MD Systems GmbH commentedI see #6 also included tests. Are those not usable in the latest patches?
Comment #17
chr.fritschBringing back the tests from #6 to #13
Comment #19
chr.fritschRemoved ctools dependency and switched to WebDriverTestBase
Comment #20
oknateI tested this briefly. Overall it works nicely.
1) On a default install, when I hover over the form, it's greyed out (https://www.drupal.org/files/issues/2019-07-07/greyed-out.mov). This is with seven admin theme and an entity browser using modal display, single widget selector plugin, no selection display and one view widget.
2) On a default entity browser field there's a separate edit button that opens a modal. I would think if you are using this display, that should be suppressed. I guess it's easy enough to turn off. So that's not a must-have. But maybe we could suppress it when using this display?
3) is there a way to have the inline entity form closed?
Comment #21
tstoecklerJust found this patch, have something similar (although not quite as elegant ;-)) on a project as well. Still trying to figure out some issues, but here's one thing I noticed:
This can just be
$options = $this->entityDisplayRepository->getFormModeOptions($this->configuration['entity_type'])
The parent call is bogus, it just happens to work because the parent returns an empty array. And the foreach is unnecessary.
Comment #22
tstoecklerSome more nitpicks:
I think this should be added to
entity_browser_entity_form.schema.yml
because the plugin is (rightly) added toentity_browser_entity_form
not toentity_browser
.I think this should be called
EntityForm
for consistency with theEntityBrowser/Widget
plugin.Should be protected and use
@inheritdoc
Comment #23
tstoecklerTwo more nitpicks, and then something substantial ;-):
this->configuration['entity_type']
is not declared anywhere. So I think we need to add that setting and add some Ajax logic to the form which dynamically updates the available form modes when you update the entity type.Similar to the plugin itself, we can drop the "Inline" part of the class name here for consistency with the other test.
Now the interesting part:
The problem I was having with my custom implementation of this was that nesting another inline entity form inside of the inline entity form embedded here would lead to the inner form values not being submitted. I can reproduce this on a clean install with this patch. Will add a failing test for that and hopefully a fix, we'll see...
Comment #24
tstoecklerOK here's a (failing) test to prove my problem. Didn't fix the nitpicks mentioned above yet. I did go ahead and change the test a bit to use exported configuration as that seems to be more in-line with the rest of the Entity Browser test coverage. Thus, two interdiffs. One, just with the refactoring and a second one with the test coverage for the nested form. I amended that into the existing test instead of creating a separate test method, as - even though it's somewhat of a different use-case - testing the nested case requires properly filling in values for the "middle" entity, so we end up duplicating the "non-nested" test coverage. I'm certainly up for suggestions to improve the structure of the test, however.
Will not get around to a fix today, I guess, hopefully tomorrow.
Comment #26
tstoecklerAlright, this makes the test pass. I will try if this also fixes the issue for my project. I had also looked at
entity_browser_entity_form_inline_entity_form_reference_form_alter()
andentity_browser_entity_form_reference_form_validate()
andentity_browser_entity_form_reference_form_submit()
which the former sets up, but I didn't have muchlookluck with that yet. To be honest the intimate details of both Inline Entity Form as well as Entity Browser are a still very cloudy to me, so anything that works is good enough for me.I took the time to fix up the in #21 / #22 / #23 and also some other small things I noticed. I left a @todo for the entity type configuration for now didn't want to get held up on that now.
Again providing two interdiffs: One for the test fix, one for the cleanups.
Edit: Minor language fail
Comment #27
tstoecklerOK, this does seem to fix the problem for me, so I think we can go ahead here. Reviewed the code once more with a bit more detail, found a bunch of nitpicks, but nothing important... (well but for the missing entity type select, per the above).
I think just
AccountInterface
should be fine here."The entity type manager."
"The entity display repository."
"The current user account."
Oops, forgot to finish the sentence here.
Inject the module handler
There should not be an if here, the form mode should be assumed to exist.
"Tests the ..."
I think this is somewhat non-standard for functional tests and also not consistent with the other test, so I would vote to remove it.
Let's remove this.
"The ..."
Also not sure what the value is of putting this is a separate variable, in particular why making it static?
It's a minor point, but I think we could be more explicit by removing this permission and adding the "actual" ones that are needed, i.e. "view article content", "update article content", etc.
Comment #28
volkerk CreditAttribution: volkerk at Thunder commentedI addressed all points from #27 except 11. and 12. for which I could not find a solution.
I would like to make a point against changing the plugin id from “inline_entity_form" to “entity_form”.
This patch is available for over 2 years now and lots of people are using it, the naming change will break their configuration. Providing an update hook would be a solution, but it only ever has been a patch.
Comment #30
volkerk CreditAttribution: volkerk at Thunder commentedRollback 6. Inject module handler: defaultConfiguration() is called in constructor.
Comment #31
volkerk CreditAttribution: volkerk at Thunder commentedComment #32
sokru CreditAttribution: sokru as a volunteer commentedNot sure how to this should have been tested so I used following steps:
1.
drush en content_translation, language, entity_browser, media_entity_browser_media_library, media_entity_browser_media, entity_browser_entity_form, inline_entity_form -y
2. Add a another language
/admin/config/regional/language
3. Add new entity reference field to article content type. Use "Users may translate this field".
4. Use EntityBrowser widget and configure its ENTITY DISPLAY PLUGIN to 'Entity form'.
5. Create new article and add media entity.
6. Translate the article and edit media entity alt text.
I expected alt text of media entity to be translated but its not, instead the source language media entity alt text got changed.
Saving the node gives following php warnings: