Problem/Motivation
Most of the open bugs in the queue share root causes in three parts of the element, and several of the stalled fixes conflict with each other because they patch the same two functions with different assumptions:
- Row state is a single
items_countinteger in form state storage, and the extra empty row is hard-wired into the render loop. This is behind #3199298: Don't add an empty element in some cases, #3310054: Validation failure wipes the multi field values when cardinality is set to unlimited, #3229995: Items are removed incorrectly with Ajax and #3394768: Nested multivalue field's button add two fields intead of one., and it makes a per-row remove button (#3185539: Add a delete button) hard to add without leaking button values into the submitted data. - "Is this row empty" is one blanket rule in
validateMultiValue(). It can't be right for every child type at once (#3482809: Saves unchecked checkboxes, #3232914: text_format elements are not supported). - Rendering borrows core's
field_multiple_value_formtheme hook, so the element is stuck with exactly the row structure core fields use. That blocks a remove/operations column and options like disabling sorting (#3200306: Add support for non-orderable multivalue form elements).
The 2.x meta (#3608015: [meta] Version 2.x) lists goals that all need these internals changed first. Rather than fix them piecemeal, this issue swaps the internals in one step, before any behavior changes.
Proposed resolution
Rework the internals with no behavior change. The existing test suite passes without a single change to a test file, which is the point: the tests define the element's contract and this issue proves the new internals honor it.
- Element state becomes an ordered list of stable row IDs plus a next-ID counter, replacing
items_count. Adding (and later removing) rows changes the list, never renumbers surviving rows. - The element gets its own theme hook,
multivalue_form_element, with a preprocess adapted from core'sFieldPreprocess. Markup and classes stay the same, so tabledrag and admin theme styling are unaffected. The preprocess already renders an_actionscolumn, ready for the remove button. - Structural row keys (
_weight,_actions) are declared in one constant and stripped from submitted values in one place. - The emptiness check moves to a dedicated
isRowEmpty()method. Semantics are identical for now; it becomes the single override point when per-type empty detection lands. - The dependency on
FieldStorageDefinitionInterfaceis dropped. core_version_requirementbecomes^11.3 || ^12. The theme registration usesinitial preprocess(11.3+), and a new major is the moment to drop Drupal 10.
Remaining tasks
Review and merge. Follow-ups are separate issues under #3608015: [meta] Version 2.x, in roughly this order:
- #3185539: Add a delete button
- #3199298: Don't add an empty element in some cases
- #3218021: Option for max cardinality
- #3232914: text_format elements are not supported
- #3252441: Invalid date when using datetime type inside multivalue
- #3280346: entity_autocomplete support
- #3482809: Saves unchecked checkboxes
- #3199172: Handle default values of nested elements
- Drop the single-child default value shorthand (no issue yet).
API changes
None for forms using the element. Sites overriding field-multiple-value-form.html.twig to affect this element should override multivalue-form-element.html.twig instead — which no longer drags core field widgets along with it.
Issue fork multivalue_form_element-3608087
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
darvanenstep 1 ready for review :)
Comment #4
darvanenSorry for the delay, ready for review.
Comment #5
sardara commentedLooks good, thanks!
Comment #7
sardara commented