Problem/Motivation
Widgets for fields with unlimited cardinality show a "Remove" button for each row. When a row is removed, weights are recalculated in WidgetBase::deleteSubmit(). The code ignores the submitted weights and assigns new ones in the order the items were submitted, which is the order of the rows in the DOM.
When items are reordered via drag and drop, tabledrag moves the table rows in the DOM, so they are submitted in the chosen order and the bug is hidden.
When "Show row weights" is used (or JavaScript is disabled), the rows do not move in the DOM and the order is only indicated by the weight selects. Removing an item will discard the weights and the widget will show the remaining items in the original order.
Steps to reproduce
- Create an unlimited-cardinality field (e.g. a plain text field) on a content type and go to the node add form.
- Enter values
1,2,3in three items. - Click "Show row weights" on the widget table.
- Using the weight selects, set the weights to
2,0,-2so the intended order is3, 2, 1. - Press the "Remove" button of the item containing
2.
Expected: the rebuilt widget shows 3, 1.
Actual: the rebuilt widget shows 1, 3.
Proposed resolution
In WidgetBase::deleteSubmit(), sort the remaining items by their submitted _weight before renumbering the weights.
Remaining tasks
Review.
User interface changes
None.
Introduced terminology
None.
API changes
None.
Data model changes
None.
Release notes snippet
Removing an item from a multiple value field widget no longer discards the item order set through the row weight selects.
Issue fork drupal-3619744
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
sardara commentedTest-only commit failed as expected.
Comment #4
sardara commentedTests green with the fix.
I added only JS tests, if needed I can add also a functional test but it would be mostly the same as the JS one.
Comment #5
sardara commentedCredit for the code belongs to @darvanen which fixed the issue for multivalue_form_element in #3185539: Add a delete button.
Comment #6
smustgrave commentedThanks for reporting. Can the MR be updated for main please as fixes needs to land there first.
Comment #7
sardara commentedRebased off main.
Comment #8
darvanenI've done a code review, I'm out of practice doing this on core so it's possible I've missed something but I reckon this one is ready to rock.
Comment #9
quietone commented