Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
Inline Entity Form was frequently cited in the thread that added forms modes:
#2014821: Introduce form modes UI and their configuration entity
Currently Inline Entity Form does not take advantage of the ability to add and configure multiple form modes.
Proposed resolution
Make form mode configurable at the widget level by adding on to InlineEntityFormBase::settingsForm()
Remaining tasks
- Post initial patch
- Find out why passing a form mode name to getFormObject other than 'default' throws an error.
User interface changes
And additional select list will be added to the field widget configuration.
API changes
I don't know if this change would be considered an API addition.
Data model changes
The settings form seems to handle this additional option without any special consideration.
Comment | File | Size | Author |
---|---|---|---|
#51 | ief-form_mode_select-2510274-51-TEST_ONLY.patch | 12.63 KB | tedbow |
#51 | ief-form_mode_select-2510274-51.patch | 24.21 KB | tedbow |
| |||
#49 | ief-form_mode_select-2510274-49.patch | 24.21 KB | tedbow |
| |||
#49 | ief-form_mode_select-2510274-49-TEST_ONLY.patch | 12.63 KB | tedbow |
| |||
#48 | ief-form_mode_select-2510274-48.patch | 12.61 KB | tedbow |
|
Comments
Comment #1
stevectorHere is an initial patch. It is built on #2491527-16: [Drupal8] IEF FieldWidget merge & refactor so the patch likely won't apply cleanly to HEAD.
The widget config seems to save correctly. However using it records this error in the log:
To test this functionality I've
I gather that the custom form display mode somehow needs to be associated with a form class. I don't know how to do that. Anyone have insight on how to render a non-default form mode?
Comment #3
stevectorIn researching something else, I came across the documentation for
hook_entity_type_build
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...The example code shows a form class being set for a form mode
Is implementing this hook required to use a form mode that was added through the UI?
Comment #4
slashrsm CreditAttribution: slashrsm commentedComment #5
slashrsm CreditAttribution: slashrsm commentedComment #6
stevectorI think this issue is somewhat blocked by this core bug I opened: #2511720: Allow form modes to use default operation if a form operation is not explicitly set
Comment #7
Micha1111 CreditAttribution: Micha1111 commentedAnother related issue here #2530086
Comment #8
bojanz CreditAttribution: bojanz at Centarro commentedI think IEF should define and use "Inline" form mode by default.
When designing the D7 module we always said that the inline context was very different (different enough to warrant entirely different forms at the time), that still feels true.
Doing that means we don't need a setting.
Comment #9
slashrsm CreditAttribution: slashrsm commentedWhile working on #2508107: Port "Single" IEF field widget to D8 I noticed another issue related to form display. If we have a reference field that references the same entity type and uses the same form display type as "host" node simple widget won't work. Core is statically caching form objects which in our case results in both forms (IEF and host) sharing the same object. This causes all sorts of strange problems.
We can avoid this problems if:
a) we use dedicated form type for IEF
b) allow to configure form type for IEF and check for collision (and throw an error)
Comment #10
Eyal ShalevA patch against the current dev version.
This patch adds a setting for the display mode that provides all the entity form displays for the targeted entity.
And passes the loaded display mode in the render array to be used to build the form.
Comment #16
Eyal ShalevFixed the paths in my previous patch.
Made it possible to override the inline form object by using the operation variable.
To override form object you need to add the path to a form class in the entity annotation with the same name as the description.
Drupal\foo\Entity\Foo.php
core.entity_view_mode.foo.inline.yml
Comment #17
Eyal ShalevComment #22
Eyal ShalevIf the display mode is not provided in the render array added a default value.
Added display_mode to the inline_entity_form schema file
Comment #23
Eyal ShalevComment #24
tassilogroeper CreditAttribution: tassilogroeper commentedI ran into a problem with the dropdown not displaying options, when applying this patch.
Comment #25
tassilogroeper CreditAttribution: tassilogroeper commentedSo when trying to inline a taxonomy_term the patch will not work. Due to the fact, that for taxonomy_terms the form is created on the fly (if you use all the defaults) and will not return any "entity_form_display" configuration. Thus, I just add a default option for the dropdown and it works.
Also I suggest to set a constant for the "default" view_mode.
Comment #26
tassilogroeper CreditAttribution: tassilogroeper commentedComment #27
tassilogroeper CreditAttribution: tassilogroeper commentedComment #28
axe312 CreditAttribution: axe312 at Wunder commentedI might help and have already experience with this since I am one of the maintainers of https://www.drupal.org/project/view_mode_selector.
Also I should be online in #drupal-contribute during this week.
Comment #29
dawehnerI'm wondering whether a fallback is the right thing to do? Should we not just validate that the configuration of the
display_mode
is valid during configuration time?That is a bit confusing, why do we not execute a special entity query which just returns the one we need? Getting all of them first and then just using one of them seems a bit pointless, or is this just me?
This method is just used internal, should we make it protected instead?
Nitpick: We could use
@return string[]
to be a bit more specific.Comment #30
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedWe should query the actual available form mode not the configured entity view displays.
I think we should rename the function as well.
I think the name DEFAULT_FORM_MODE is more accurate.
Comment #31
webflo CreditAttribution: webflo at UEBERBIT GmbH commentedReplace the entire function with EntityDisplayRepository::getFormModes
Comment #32
tassilogroeper CreditAttribution: tassilogroeper commentedSo webflo and I sat together on this one.
- added two constants for default display_mode and default operation
- pay attention: the initial patch mixed up the form mode ("default") with the form handling ("default", "edit" etc.), thus we made the distinction here
- I removed redundant fallbacks for fetching the form mode
Comment #33
tassilogroeper CreditAttribution: tassilogroeper commentedpatchfoo missed me
Comment #35
tassilogroeper CreditAttribution: tassilogroeper commentedComment #37
tassilogroeper CreditAttribution: tassilogroeper commentedupdate test stubs with the new form field
Comment #38
Eyal Shalev@tassilogroeper The reason I combined the form handler and the display mode was to allow for custom form classes to be used.
Without it the default class will always be used.
Comment #39
bojanz CreditAttribution: bojanz at Centarro commentedJust clarifying that I'm now +1 on having this setting.
We should just call this #form_mode.
The reroll can wait until we fix #2667710: Rewrite the base inline form handling.
EDIT: No longer blocked, let's go.
Comment #40
tedbowOk, I am starting on re-rolling this.
Comment #41
tedbowOk here is re-roll
I changed 'display_mode' to 'form_mode' in variables and '#ief_form_mode'
I also made some other changes because we are no longer use the $controller of the entity.
It seems like don't actually need to be storing the object here just the form mode settings like 'default', 'inline', etc
This not need as it used in getFormObject which is not being called anymore
This method is not need anymore because we don't need the instance just the string of the form_mode.
I left this in but do we need to rely on iefHandler for getEntityFormModes? Since we know that entity type from the field definition we could just form modes direclty here. There is no other place getEntityFormModes is being used and we could remove it. Is there any case where the handler would want to limit the options of form modes? Maybe?
We don't need to get the instance here. We can simply
Even if the form mode has been deleted it won't be problem because \Drupal\Core\Entity\Entity\EntityFormDisplay::collectRenderDisplay check for the existence of the form mode and will use 'default' if not available.
Comment #42
tedbowOk we need some tests for this.
To start of with in InlineEntityFormElementWebTest and InlineEntityFormSimpleWebTest
We could
Do need some kind of dependency check? Should you able to delete a form mode if a instance is using it?
Comment #43
tedbowYay! current tests pass.
I just notice that we probably need to update \Drupal\inline_entity_form\Plugin\Field\FieldWidget\InlineEntityFormBase::settingsSummary
To show the form mode setting.
I checked locally and the widget does respect the form mode.
Comment #44
bojanz CreditAttribution: bojanz at Centarro commentedGenerally looks good.
Needs fixing:
1) public function getEntityFormModes(); doesn't belong on the inline handler
It's used in the widget, add it to the widget (base class)
2) const DEFAULT_FORM_MODE = 'default'; isn't needed cause default is already the fallback
Ideally our configuration would could a dependency on the selected form mode, but I'm unsure how that can be done from inside the widget plugin.
Comment #45
tedbowAdded changes mention in #44
Also added InlineEntityFormBase::calculateDependencies
To add dependency on the Entity Form Mode.
Updated InlineEntityFormBase::settingsSummary to show form module in summary.
Comment #47
tedbowarrrgggh. always. test. locally.
Forgot to update \Drupal\inline_entity_form\Plugin\Field\FieldWidget\InlineEntityFormComplex::__construct
to match.
Will fix soon
Comment #48
tedbowFixed InlineEntityFormComplex::__construct
And phpdocs
Comment #49
tedbowOk added tests and test_only patch.
Comment #50
tedbowSome notes about the new tests:
Moved the $user property to the base test class. All classes need this.
The element test was creating a user but never using/logging in with that user. It didn't matter before because there is no permission check on the custom from. Now to test the different fields on theform display the user needs 'administer nodes' permission.
Basically just taking the previous tests and running for each form mode.
This was just an old debugging line I had left in from previous test. Sorry
Added this function to the base class so that we can check form displays from all test classes in the future.
Add new form display for just this bundle for now.
Added "inline" form mode to node for testing.
Comment #51
tedbowoh no TEST_ONLY passed!
I wasn't sending NULL as second arg here.
Updated patchs
Comment #54
bojanz CreditAttribution: bojanz at Centarro commentedRenamed #ief_form_mode to #form_mode, made the setting required and defaulting to 'default'. Committed.
This was an important one, thanks tedbow!
Comment #56
mogio_hh CreditAttribution: mogio_hh commentedJust tested the form mode feature in the latest DEV.
Although there is a select control to pick the "form view mode", "default" is rendered - no matter what is selected.
Does this patch fix this problem?
To me it seems as if this patch went already to beta but its now broken again?!
Thanks
Comment #57
digihumsteve CreditAttribution: digihumsteve commented@mogio_hh I have the same issue you report in #55. Renders the default form mode, whatever is set in form mode.
Comment #58
brooke_heaton CreditAttribution: brooke_heaton as a volunteer commentedThis was a great feature, but I'm confused as to why IEF does not respect or consider the Twig template of said Form mode? If you override the form mode template, nothing happens when the IEF is rendered. It's odd.
Comment #59
thronedigital CreditAttribution: thronedigital commentedfacing this issue as #58
Any suggestions would be appreciated