Extra Default value field when adding a field with an unlimited values

quietone → credited drugan → .

Uploading the patch from the other issue.

And adding credit.

The above patch fixed the problem in the "Default value" section but introduced a problem to the content-adding section.
The "Add another item" button is not working on the first click, added screen recordings below.

Note: The problem only exists in the content-adding section, not in the "Default value" section.

I have fixed the issue in the MR:
https://git.drupalcode.org/project/drupal/-/merge_requests/3994
Please review.

This is missing test coverage.

Fixed test failures and added new tests, please review

Ran the tests without the fix and they fail so removing that tag.

The proposed solution is empty though, issue summary should be updated completely please

Thanks.

Did a Standard install
Added a Text (Plain) filed
Set to unlimited
Verified 2 default fields
Applied patch and verified I only see one

Ran tests without the fix and got
Behat\Mink\Exception\ExpectationException : 2 elements matching xpath "//table[@id='field-unlimited-values']/tbody/tr//input[contains(@class, 'form-text')]" found on the page, but should be 1.

And

Behat\Mink\Exception\ExpectationException : A field "default_value_input[yes8p0w5][1][target_id]" appears on this page, but it should not.

Think this is good.

The proposed solution is pragmatic but it seems a bit strange to adjust the range for Field UI in \Drupal\Core\Field\WidgetBase which is quite low level code. The root cause to the problem is that \Drupal\field_ui\Form\FieldConfigEditForm::form actually adds an item to the item list, and therefore it's rendered as if there was one value. 😅 It's hard to remove that because the current implementation is tied to an object that is used in few places. We have been working on untangling that in 📌 Save FieldStorageConfig at the same time as FieldConfig Fixed . Therefore I propose we adopt the minimum subset of that issue needed to address this issue.

Changes looks good to me. Tested manually and things are working as expected.
One nit to address if that make sense.

+++ b/core/modules/field_ui/src/Form/FieldConfigEditForm.php
@@ -123,6 +130,7 @@ public function form(array $form, FormStateInterface $form_state) {
+    $items = $this->getTypedData($form['#entity']);

Can this be removed from here and added after first assignment at line 118?
Something like
`$items = $this->getTypedData($form['#entity']);
$element = $items->defaultValuesForm($form, $form_state)
`

And later
`if ($element) {`

Trying to address #15 with a code comment 👍

+++ b/core/modules/field_ui/src/Form/FieldConfigEditForm.php
@@ -219,9 +230,14 @@ protected function actions(array $form, FormStateInterface $form_state) {
+    $this->entity = parent::buildEntity($form, $form_state);

Should this be $this->buildEntity()?

And can we have a follow-up to investigate adding a buildEntity call to EntityForm::validateForm()?

Thanks @tim.plunkett! Totally makes sense to call for $this instead of parent. 🤦‍♂️ I also opened a follow-up and added a todo that points there: 🐛 Extra Default value field when adding a field with an unlimited values Fixed .

(note, @lauriii mentioned this issue in his comment, but used the correct nid in the @todo.)

Thanks for the quick fix!

larowlan → closed merge request !3994

Committed 93a509c and pushed to 11.x. Thanks!

I don't think there's any risk of disruption here, there's unlikely to be anyone sub-classing that form with an existing getTypedData method, so backported to 10.1.x too.

Thanks folks

Automatically closed - issue fixed for 2 weeks with no activity.