[Drupal8] IEF FieldWidget merge & refactor

Works good. We should add smart behavior though so that single values are not displayed in a sortable table.

In #2134035: Allow to add existing entities using the single value field widget the difference between the single and multi value fields is solved for D7, while still allowing for the "single required new entity" use case which embeds the entity form fields without any buttons.

This is my understanding of the current status of the widgets in IEF:

IEF has been developed as a part of the Commerce platform and it has been designed to support two distinct use cases which were both needed in Commerce:

Use case 1: Seamlessly integrated form for a single value, required, new reference

This is used in the Commerce Discount module to create a new CommerceDiscount entity inline when creating an offer.

In this use case the referenced CommerceDiscount entity will always be newly created, and it is uniquely bound to the parent entity. From the user interface perspective there is no need to bother the user with buttons to choose between adding an existing or a new entity, or to provide submit buttons. The fields of the CommerceDiscount entity can be integrated in the parent form, and when the parent form is submitted, we can also save the discount fields.

This is of course not restricted to only discounts in Commerce sites, this use case is useful in many potential scenarios. One could consider:

If an parent entity requires a new reference to a single value child entity, then we can always seamlessly integrate the fields of the child entity into the parent form.

Use case 2: Flexible inline form for multi-value references

The second use case is the more familiar form where the user can choose between creating a new entity or adding an existing one, can submit the entity form inline, can have single or multiple references. The user is presented with buttons to add new or existing references, and can edit, delete or reorder existing references that are listed in a table.

The original solution

Historically these two distinct use cases were solved by using two separate widgets. One was called the "single value" widget, while the other was called the "multi value" widget.

This was a perfect solution initially, but when the module started gaining traction people came up with new use cases, and one of these is #2134035: Allow to add existing entities using the single value field widget. This use case closely matches the original single value use case, with one difference: people would like to have the possibility to add existing references as well as new ones. For the rest everything could remain the same: it's a single value so it's ideal from a UX perspective that the child entity should be submitted along with the parent form.

A good example of this use would be adding a single required Client entity to a parent Invoice entity. A new Client could be added inline which would be seamlessly submitted along with the invoice, but for returning customers it should be possible to select an existing Client reference.

The proposed solution

I needed the functionality for #2134035: Allow to add existing entities using the single value field widget for one of my projects so I worked on the patch. In the patch I noticed that the distinction between the "single value" and "multi value" widgets are actually totally unnecessary. In fact, in the resulting patch there is not even a check any more to see which widget type was used! We can simply derive this from the existing options and the cardinality.

Use case 1 (seamless integration) can be used when:

1. The cardinality is 1.
2. The reference is required
3. Only one bundle can be referenced
4. Referencing existing entities is not allowed

If these conditions match, we show the add/edit form inline without buttons. If any of these conditions do not match, we show the table.

This can all be handled from a single widget.

#12 sounds like a good plan, but there are issue we need to resolve first (main being to figure out how autocomplete feature even fits in single widget UX).

Some time ago we had an IRC discussion about this and we agreed that we start with a base class that both widgets extend. Then we keep cleaning stuff up and move code around until everything gets more organized. At that point we see how much code is actually shared between both widget implementations and then we can proceed with the actual merge.

Attached patch introduces base class and moves all appropriate code into it. I also added some very obvious clean-ups.

I submitted the initial patch to #2508107: Port "Single" IEF field widget to D8, which is based of patch in #13

Just a few nitpicks.

1. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormBase.php
   @@ -0,0 +1,273 @@
   +        [
   

   Code style.

2. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormBase.php
   @@ -0,0 +1,273 @@
   +        [
   

   Code style.

3. +++ b/src/Plugin/Field/FieldWidget/InlineEntityFormMultiple.php
   @@ -1038,4 +845,37 @@ class InlineEntityFormMultiple extends WidgetBase implements ContainerFactoryPlu
   +    $has_children = !empty($children);
   

   Can we use FormStateInteface::isValueEmpty here?

Can we use FormStateInteface::isValueEmpty here?

FormStateInteface::isValueEmpty() operates on submitted values while we operate with internally stored variables in this case. We could use FormStateInterface::has(), but that would return TRUE even in case of an empty array.

Fixed array stuff.

@floretan found two issues with this.

Thank you @slashrsm for the fixes. I did some more testing with the new patch and I have two more bugs, both of which occur with the following setup in combination with the patch from #2508107: Port "Single" IEF field widget to D8.

I have an article node type with an optional entity reference field pointing to a gallery (media_entity) which has a required entity reference field pointing to images (different bundle of media_entity). Everything works when I use a multiple IEF for both, but I get these issues when using the single IEF for the field attached to the article node type.

• Create an article without any images in the gallery. I realize that the expected behavior is not fully clear (should no entity get saved, or should we throw an error). What we get is a fatal error combined with a user error that the field is required.
• Create an article with one image in the gallery and save it. Then open the edit form and save again without any changes. Then media_entity throws an error "The media on this page has either been modified by another user, or you have already submitted modifications using this form. As a result, your changes cannot be saved. ". I haven't investigated yet if this is an issue with media_entity or with this patch, but it's the interaction between the two that causes problems

I was playing with this a bit today with the intention of starting some test coverage, but I wasn't able to set up a reference between two entities using IEF. I didn't get any options in the Field UI relating to IEF widgets when creating an entity reference field. I need some pointers to how this currently managed.

So to not come here empty-handed, here are some coding standards fixes :)

@floretan: Both bugs seem to be related to #2508107: Port "Single" IEF field widget to D8. Note that we didn't have a working implementation of "Single" widget before that patch. I'll move this issue back to Needs review and move discussion about those two patches to the other issue. It would be great if we can RTBC patch in this issue soon as there soon might be other patches that will depend on it.

@pfrenssen: AFAIK we didn't port "reference existing entity" feature yet. We could do that as part of #2490914: Use autocomplete element for autocomplete feature (and maybe rephrase it a bit).

@slashrsm, sure let's take it one step at a time. I like this approach with a base class. I do think we will end up eventually with a single widget, but this will allow us to get there in a controlled manner :)

BTW I got some help from @webflo in IRC on where to find the field widgets in D8, I was looking in the wrong place, they're now in the form display page.

@webflo agreed that we should commit this and resolve any issues in follow-ups. Thank you everybody!

Thank you for your work, everyone.

Yes, let's create a follow-up.