Problem/Motivation

It seems that #3000613: Select2 element not properly rendered as a RenderElement introduced a but where if you use the render element in a custom form (or altering views exposed filters to use this element), you will get a warning such as:

Notice: Undefined index: #key_column in Drupal\select2\Element\Select2::validateElement() (line 226 of modules/contrib/select2/src/Element/Select2.php).

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcoscano created an issue. See original summary.

marcoscano’s picture

Status: Active » Needs work
Issue tags: +Needs tests
FileSize
680 bytes

I'm not very familiar with the module, but this seems to do the trick for me.

Edit: I wanted to trigger tests but I see there's no automated test configured on d.org for the project yet? :/

chr.fritsch’s picture

Thx for the patch. This module has no automated tests on d.o. because I don't know how to download the select2 library on testbot.

It would be best to clone https://github.com/thunder/select2 and open a PR.

It also would be nice if you could provide some sample code, so that I am able to reproduce this issue.

chr.fritsch’s picture

Status: Needs work » Needs review

@marcoscano: I opened https://github.com/thunder/select2/pull/35. Would be nice to get your feedback

marcoscano’s picture

Sorry for not being able to contribute investigating this a bit further at this moment... :(

My 2 failing scenarios were:

1) In a hook_form_views_exposed_form_alter(), I've changed the select element to use the select2 element, like so:

$form['field_foobar']['#type'] = 'select2';

This worked fine so far when the view rendered the exposed filters, but started to fail since I updated the code to a more recent version. It seems that keeping the '' empty option in single elements is a problem and confuses the exposed filters. For now, I've been able to bypass it by also setting the element to multiple:

$form['field_foobar']['#multiple'] = TRUE;

because in my use case it actually makes sense to be able to select several values, and this has a side effect that the '' empty option is not populated.

The patch in #2, in combination with this workaround, seems to be working fine so far for us.

2) In a node with an entity reference field, we've set the field widget to use the "select2" widget.
Then you save a node without selecting anything in the select2.
Then (don't ask why) in a custom validate/submit callback I need to do something like:

$node = $form_state->getFormObject()->buildEntity($form, $form_state);

And with a more recent codebase, this breaks because it triggers the whole form building chain, and then we end up with
Argument 1 passed to Drupal\Core\Field\WidgetBase::massageFormValues() must be of the type array, string given, called in /var/www/html/docroot/core/lib/Drupal/Core/Field/WidgetBase.php on line 381
because $form_state->getValue('field_foobar') is '' while ::massageFormValues() expects it to be [].
I'm able to workaround this though by setting [] in the form state before calling ::buildEntity(), so this scenario wasn't much of an issue for us.

chr.fritsch’s picture

Thx for your comment @marcoscano

I pushed some changes to https://github.com/thunder/select2/pull/35 and I think I am done.

I will add more test coverage but would also like to hear your feedback @marcoscano.

chr.fritsch’s picture

For using select2 in exposed views filter you could do:

function hook_form_views_exposed_form_alter(&$form, \Drupal\Core\Form\FormStateInterface $form_state, $form_id) {

  if (isset($form['status'])) {
    $form['status']['#type'] = 'select2';
    $form['status']['#select2']['allowClear'] = FALSE;
  }

}
chr.fritsch’s picture

Added some tests. Ready for review

marcoscano’s picture

FileSize
24.35 KB

Just uploading here the patch from #6 so it can be more easily targeted with composer.
(I know we can add .patch to the PR route on github, but it stops working when the PR is merged... :) )

Edit: Well, the .patch trick on github doesn't seem to work as I expected... this patch apparently includes all changes from intermediate commits in that PR. It seems to apply correctly though.

marcoscano’s picture

@chr.fritsch thanks for working on this!

With the previous patch applied to the latest dev, now we have a failure on the entity reference widget (in my case a taxonomy term). When being submitted empty, it tries to create a term with an empty label.

I can see two failing scenarios of the same issue, in my use case:

1) A media entity with a non-required "Categories" taxonomy field, where no new terms can be created from the widget, using the Select2 ER widget. Without populating anything in the select2, when the form is validated (for example in the AJAX rebuilding after the media file is uploaded), we see in the logs:

Drupal\Core\Entity\EntityStorageException: Missing bundle for entity type taxonomy_term in Drupal\Core\Entity\ContentEntityStorageBase->doCreate() (line 86 of /app/docroot/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).

2) A node entity with a non-required "Type" taxonomy field, where new terms can be created from the widget, using the Select2 ER widget. When I submit the node with the select2 empty, a term is created with an empty label.

(If in this scenario I perform my $node = $form_state->getFormObject()->buildEntity($form, $form_state); during form validation, I get the error:

TypeError: Argument 1 passed to Drupal\Core\Field\WidgetBase::massageFormValues() must be of the type array, object given, called in /app/docroot/core/lib/Drupal/Core/Field/WidgetBase.php on line 381 in Drupal\Core\Field\WidgetBase->massageFormValues() (line 528 of /app/docroot/core/lib/Drupal/Core/Field/WidgetBase.php)

In this node validation function, just prior to the ::buildEntity(), the form state has directly the term object for the field value:

chr.fritsch’s picture

FileSize
14.11 KB

Puh, this is a tough one...

So I had to make a decision because it's not possible that the render element behaves on the one hand like a normal select element, but on the other hand like entity autocomplete element.
So I decided to implement select2 in the same behavior as a select element. That means I moved the logic to create new entities from the element to the widget.

marcoscano’s picture

FileSize
281.95 KB

Thanks! But we're still not quite there I guess...

If you don't select anything, '' is being sent as the element value. For example, the following scenario fails:

- A File media entity with a non-required taxonomy field using the ER select2 widget.
- Without entering anything in the select2, when you upload the file, the AJAX validation fails with
An illegal choice has been detected. ...
Which seems to be because:

chr.fritsch’s picture

FileSize
15.44 KB

Added a test for that and fixed it.

marcoscano’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! It works pretty well in manual testing :)

The only detail still bothering me a bit is when I do the $node = $form_state->getFormObject()->buildEntity($form, $form_state); call from inside an AJAX callback (I guess it could be a validate/submit function too). When there are no values in the select2 I get a

TypeError: Argument 1 passed to Drupal\Core\Field\WidgetBase::massageFormValues() must be of the type array, string given, called in /app/docroot/core/lib/Drupal/Core/Field/WidgetBase.php on line 381 in Drupal\Core\Field\WidgetBase->massageFormValues() (line 528 of /app/docroot/core/lib/Drupal/Core/Field/WidgetBase.php)

because the widget passes '' to the ::massageFormValues(), instead of [].

However, this may be a very specific case only I am experiencing, and it is easily workaroundable by changing '' by [] in my custom code before calling ::buildEntity(). Because of that I'm marking this RTBC, I do believe the patch is a great improvement from what we had before.

Awesome work @chr.fritsch, thanks a lot!

@chr.fritsch++

  • chr.fritsch authored 2875d0c on 8.x-1.x
    Issue #3001970 by marcoscano, chr.fritsch: Warning being thrown when...
chr.fritsch’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your help @marcoscano

Committed and fixed

Status: Fixed » Closed (fixed)

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