Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
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
Comment | File | Size | Author |
---|---|---|---|
#13 | 3001970-13.patch | 15.44 KB | chr.fritsch |
#12 | empty_element_error.jpg | 281.95 KB | marcoscano |
#11 | 3001970-11.patch | 14.11 KB | chr.fritsch |
#10 | empty_term_in_ER_field_value.jpg | 99.14 KB | marcoscano |
#9 | 3001970-6.patch | 24.35 KB | marcoscano |
Comments
Comment #2
marcoscanoI'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? :/
Comment #3
chr.fritschThx 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.
Comment #4
chr.fritsch@marcoscano: I opened https://github.com/thunder/select2/pull/35. Would be nice to get your feedback
Comment #5
marcoscanoSorry 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 theselect2
element, like so: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: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:
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.Comment #6
chr.fritschThx 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.
Comment #7
chr.fritschFor using select2 in exposed views filter you could do:
Comment #8
chr.fritschAdded some tests. Ready for review
Comment #9
marcoscanoJust 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.
Comment #10
marcoscano@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:
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:In this node validation function, just prior to the
::buildEntity()
, the form state has directly the term object for the field value:Comment #11
chr.fritschPuh, 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.
Comment #12
marcoscanoThanks! 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:
Comment #13
chr.fritschAdded a test for that and fixed it.
Comment #14
marcoscanoThanks! 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 abecause 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++
Comment #16
chr.fritschThanks for your help @marcoscano
Committed and fixed