Refactor IEF based on Commerce InlineForm API improvements as a 4.x major version [#3051990]

It is no secret that the 8.x-1.x codebase is messy. It is a direct port of the D7 code which goes back to 2012. I am not in love with the APIs and the UX.

When the InlineForm API was committed to Commerce at the beginning of this year, some people rightly noticed that it looks like the beginning of a better IEF. I responded by commenting how that code is only about 30% of a full IEF rewrite, and since we have no time for such a rewrite, it's better not to attempt it for now. Fast forward a few months, the InlineForm API is now nice and stable, and I had a day available, so it made sense to do some initial work on the 2.x branch, at least as a guide to future maintainers.

The result is https://github.com/bojanz/inline_entity_form
Intentionally on GitHub for now because I want to discourage people from using it. No tests until the general direction is validated.

It contains the inline form API and the "Embedded" widget (equivalent to the "Simple" widget in 1.x).
It is missing an equivalent to the Complex widget (which will probably also need the "inline_form" handler per entity type).
My idea was to replace it with a "Modal" widget which would show the table and open all forms in a modal.
It would use tempstore behind the scenes. No more $form_state magic. This was initially discussed in #1881616: Move the inline forms (add, edit, remove, reference existing) to modals but put on hold cause it doesn't support nesting. At this point, that feels like a reasonable limitation.

Comments

Comment #1

1 May 2019 at 17:20

bojanz created an issue. See original summary.

Comment #2

bojanz commented 2 May 2019 at 08:57

Issue summary:

View changes

Clarified that the Modal widget would definitely use tempstore.

In the widget form we'd construct a tempstore key: hash(parent entity UUID + field name + time), and use that to store the added/edited entities. The key contains the time so that if the parent form is abandoned, the changes in progress are not retained, it would be confusing UX-wise. This way each parent form load gets its own tempstore.

EDIT: Also, how do we feel about "Table" as the widget name? Feels more descriptive than Modal, multiple widgets could use modals.

Comment #3

geek-merlin

German

Freiburg, Germany

commented 12 February 2020 at 02:52

Good to know that some cleanup work already had been done. Hope we can get traction for this.

@bojanz In #2 /IS you say 2.x is based on a modal widget, but doing some archeology i found you commenting 5 years ago in #1881616-10: Move the inline forms (add, edit, remove, reference existing) to modals:

Exploration of this problem (done by slashrsm and me) showed that modals aren't flexible enough for IEF because you can always have only one.
This means no nested IEFs, no possibility for an entity form to open a modal for their own purpose.

So sadly I'll have to mark this issue as "won't fix" and leave the modals to other contribs.

Can you clarify?

Comment #4

bojanz commented 12 February 2020 at 09:28

@geek-merlin
A percentage of IEF users likes to nest them, up to several levels (e.g. products -> variations -> billing schedules). At the time we were concerned about leaving those users behind, especially since the goal of 8.x-1.x was to provide feature parity to D7. This worry was in part because the 8.x-1.x codebase is a monster, the Complex widget holds most of the real code, and it's not reusable. I'd love to see 8.x-2.x have more reusable elements which would allow people to build all kinds of alternative widgets (such as reference-first, etc) for different use cases.

I don't know if there's a problem with other module's modals as well. If we're editing a variation in a modal, can we use media library for variation images? Can the media library be opened?

Still, I think it's worth exploring, for the gain in UX and code quality. People who nest IEFs can combine different widgets (so, you use the Table widget to manage variations on a product, but then use the Embedded widget to manage another entity on the variation)..

Comment #5

geek-merlin

German

Freiburg, Germany

commented 12 February 2020 at 13:40

@bojanz
Thanks a lot, having that background helps understand. Unfortunately i have first commented here before i noticed #1881616: Move the inline forms (add, edit, remove, reference existing) to modals where that discussion obviously should live to stay focused. Now i cleaned up the mess a bit by copying this over and commenting there.

Comment #6

johnwebdev commented 13 March 2020 at 12:20

I've glanced on the Commerce InlineForm API for a while, and was happy to found that this issue exists.

I'd love to see 8.x-2.x have more reusable elements which would allow people to build all kinds of alternative widgets (such as reference-first, etc) for different use cases.

Big +1. Support asymmetric/symmetric translation is one of those use cases. With more reusable element this would probably be easier to build separate widgets supporting these different cases.