Re-order + remove broken with the Entity Reference (and File) widget

Adapted patch #92 with patch from https://www.drupal.org/project/entity_browser/issues/2821917 ✨ Allow the edit button's form mode to be selected in Entity Browser field widget settings Needs review

Please, whoever contributes to the issue, maintain order and don't introduce mess.
If you provide a patch, please make sure to provide interdiff with an explanation what was changed, and make sure the patch applies without errors and tests are passing. #102 doesn ot apply, needs work plus it extends the scope with the issue: #2821917: Set form mode for edit button. I restore #92 patch into the list as it is in scope of this issue only.

Attached is a rebase of patch from #92 onto the latest 8.x-2.x, as that batch no longer applied cleanly on the 8.x-2.9 branch.

Actually, disregard that, I think I have some multiple conflicting patches.

Updated the patch in #92 to no longer use jquery once so that it can be used in D1O

$(this).once('entity-browser-remove').on('mousedown', function(e) {
becomes
$(once('entity-browser-remove', $(this))).on('mousedown', function(e) {

@svdhout your patch is broken, has indentation issues. Please check it before uploading.

+++ b/js/entity_browser.entity_reference.js
@@ -50,13 +59,31 @@
     /** @var \Symfony\Component\Validator\ConstraintViolation $violation */
     foreach ($violations as $offset => $violation) {
     + $trigger = $form_state->getTriggeringElement();
     + if (!empty($trigger['#ajax']['event']) && $trigger['#ajax']['event'] == 'entity_browser_value_updated') {
     + // Skip validation if it is triggered by hidden target_id.
     + $violations->remove($offset);
     + continue;
     + }
     // The value of the required field is checked through the "not null"
     // constraint, whose message is not very useful. We override it here for
     // better UX.
     @@ -374,30 +386,21 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
     public function formElement(FieldItemListInterface $items, $delta, array $element, array &$form, FormStateInterface $form_state) {
 
     $entities = $this->formElementEntities($items, $element, $form_state);
     -
     - // Get correct ordered list of entity IDs.
     - $ids = array_map(
     - function (EntityInterface $entity) {
     - return $entity->id();
     - },
     - $entities

The patch is broken, has indentation issues from top to bottom. The CI also fails to apply. Please make sure to check before upload. I kindly refer to the #103 comment here...

Updated the patch, and provided an interdiff

Correctly applied the patch in #108, there was some context moved in between the patch this was derived from and the current HEAD.

Please don't credit me for this trivial change.

I tried applying the patches in #92 and #108 to version 2.9.0 of entity_browser and Drupal 9.4.12. Full disclosure, the field I tested with is also using a field widget from entity_browser_table version 1.4.0.

For both of these patches, editing a node and adding a new entity reference to a field with existing references caused the order of the references to change, which I don't think is expected.

This has some overlap and potential collision with ✨ Add option for field widget to display as table Needs work . It would be best to settle on one solution.

The table layout approach is cleaner.

I was running into something that seemed similar to this where just having a single field was broken in d10. both replace and remove buttons triggered this error:

TypeError: Drupal\Core\Form\FormState::setValueForElement(): Argument #1 ($element) must be of type array, null given, called in /app/web/modules/contrib/entity_browser/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php on line 546 in Drupal\Core\Form\FormState->setValueForElement() (line 79 of /app/web/core/lib/Drupal/Core/Form/FormStateValuesTrait.php).

Since this looked similar I applied it and it helped but I noticed a couple our weird things with the replace button.
1) Most of the time it just breaks with the same error.
2) Sometimes it seems to replace the current widget with a different entity reference from the page.

The primary field being tested is a media entity using a media entity browser with a cardinality of 1.

Not sure if this even is the right issue but if it is let me know if there are things I can help test to finalize this.

Patch #108 works for me. Tested in Drupal version 9.5.9 and Entity browser 2.9.0

+++ b/src/Plugin/Field/FieldWidget/EntityReferenceBrowserWidget.php
@@ -491,9 +504,6 @@ class EntityReferenceBrowserWidget extends WidgetBase implements ContainerFactor
-    elseif ($trigger['#type'] == 'submit' && strpos($trigger['#name'], '_remove_')) {
-      $parents = array_slice($trigger['#array_parents'], 0, -static::$deleteDepth);
-    }

The removal of this code is causing the "Remove" button to not work properly on my site.

This fixes another similar issue where if you Add a new item from the browser and then try to remove it, it relaunches the browser.

Also much nicer UX as it appears to remove things straight away.

Tested with an overly complex data model of: Node -> Custom entity form with IEF -> Paragraph -> Paragraph -> Entity browser widget multi cardinality field for a custom entity.

Tested adding, removing, reordering, and tried my best to break it but it all worked well.

Figured out the issue I reported in #110.

I'm using the Entity Browser - Table Layout → project, which extends the EntityReferenceBrowserWidget.php plugin. The changes from this issue caused breaking changes for the other module's widget.

I created a child issue in that project to provide necessary changes if you apply this patch and are also using that module: 🐛 Re-order + remove broken with the Entity Reference (and Table) widget Active .

A new release is published and the dev branch is changed.
I re-rolled #109 patch in order to apply to the latest release and the 2.x dev branch.

Can we get a merge request version posted so that we can start running the new GitLab CI testing against this?

Opened a merge request with the patch from #117. However, I am leaving the patch for composer patching.

Please note that, tests are failing on core v9.5 which I am not sure how relevant at this point, since it will not be supported further I believe. I leave that to the maintainers.

And as for the test failures with ajax, see this change record → . We also started to have the same failures, and we are asserting on elements appearing on the screen instead of using waitForAjaxToFinish().

Added the latest patch for the MR for composer builds

After using the above patch with Drupal 10 + layout builder, I am getting the following error
RuntimeException: The subform and parent form must contain the #parents property, which must be an array. Try calling this method from a #process callback instead. in Drupal\Core\Form\SubformState->getParents() (line 76 of /Users/maria.mcdowell/Sites/devdesktop/eemmcdowell-test/docroot/core/lib/Drupal/Core/Form/SubformState.php).

I have created a separate issue with all the details " Error when trying to place a new block: RuntimeException: The subform and parent form must contain the #parents property, which must be an array. 🐛 Error when trying to place a new block: RuntimeException: The subform and parent form must contain the #parents property, which must be an array. Active "

Updated patch #123 to fix the above error.

This one worked for us!

D11 tests are failing atm, but 10.4 and previous major tests should be passing.