[Autocomplete] required choice types end up not being required.

[bendavies]

Hi there,

There seems to be an issue with how tom-select is configured, for required, non-multiple choice types.

Given a form type configuration like

$builder->add('isAttending', ChoiceType::class, [
    'choices' => [
        'Yes' => 'yes',
        'No' => 'no',
        'Maybe' => 'maybe',
    ],
]);

You will be left with a select box like:

All well and good, an option is mandatory.

However, convert to autocomplete and you're left with:

see the issue? It's clearable because the clear plugin is added, so you can be left with no value.

this shouldn't be possible, as it isn't with the simple select box above.

We can try fix fix:

$builder->add('isAttending', ChoiceType::class, [
    'choices' => [
        'Yes' => 'yes',
        'No' => 'no',
        'Maybe' => 'maybe',
    ],
    'autocomplete' => true,
    'required' => true,
]);

this has no effect for 2 reasons:

1. the symfony/ux-autocomplete controller still adds the clear plugin (while the easyadmin tom-select controller takes requried into considertion, from which symfony/ux-autocomplete was inspired by)
2. the easy admin version wouldn't work either, because symfony removes the required attribute for select boxes when it's impossible not to submit a value.

Number 2 can be "fixed"/overridden like this:

$builder->add('isAttending', ChoiceType::class, [
    'choices' => [
        'Yes' => 'yes',
        'No' => 'no',
        'Maybe' => 'maybe',
    ],
    'attr' => [
        'required' => true,
    ],
    'autocomplete' => true,
]);

but that would mean we still have to fix the stimulus controller here to not add the clear button if there is a required attribute.

my "fix" above (adding 'attr' => ['required' => true]) is probably too onerous to put on the end user. Maybe the autocomplete form type should do this automatically?

Thanks

[mario-fehr]

Duplicate of #379 and fixed with #401

[bendavies]

I do not agree that those issues are the same as this one - those are about completely overriding the default plugins setup by symfony/ux-autocomplete.

this issue: I think symfony/ux-autocomplete should be more intelligent/correct out of the box with reguards to required select elements.

[maartendekeizer]

@bendavies Agree, not the same but related.
I think best way is to only add the clear button when the select is not required. Another option is that we never set plugins automaticly but I think that gives bad DX

[weaverryan]

Hi there!

Really excellent issue and explanation of the problems Ben - thanks for that. As Ben mentioned, the required "form view variable" is not always printed out as a required attribute on the select element, which is why the current solution of looking for that attribute in JavaScript doesn't work. Could we, instead, read the required variable ($view->vars['required']) in the form type extension -

ux/src/Autocomplete/src/Form/AutocompleteChoiceTypeExtension.php

Line 42 in 8e84d49

public function finishView(FormView $view, FormInterface $form, array $options)

- and then use it to pass in a new "value" to the Stimulus controller?

[bendavies]

yep, that looks doable. we just need to pass a new required attr which will make it into the template

[LucileDT]

@bendavies do you have any news? 👀

[bendavies]

forgot about this. i'll take a look again

[Qronicle]

This is still an issue as far as I can tell. How do we proceed to get the PR merged?

Although thinking about this some more, I feel like the "clearability" of a select is related more to whether there is a placeholder or not. (At least that's how I have always implemented my own js enhanced select elements.)
You can for example have a required select with an empty/placeholder option that forces the user to manually select something. In this case it would still make sense to show the clear button to revert back to the initial, unset state.

I would have to dive into the code to see how the placeholder is deduced, although I wouldn't be opposed to the solution proposed in the PR, as it would still be a major improvement over what there is now.

Edit: I made an attempt at a pull request that implements the placeholder check