CRUD icon indicating copy to clipboard operation
CRUD copied to clipboard

add placeholder field option to select2

Open pxpm opened this issue 4 years ago • 9 comments

by default, when allows_null => true we show an - as a placeholder.

this PR allow developers to change field placeholder to something they want like Select a Recipe Ingredient

pxpm avatar Jan 19 '21 13:01 pxpm

The inspection completed: No new issues

scrutinizer-notifier avatar Jan 19 '21 14:01 scrutinizer-notifier

@pxpm I think we should use the placeholder of select2. But as far as I understood, it removes our empty option, so we would need to also allow the clear button.

Also, we should do the same for Select, Select_grouped, Select2, Select2_grouped and Select2_nested.

What do you think?

promatik avatar Jan 19 '21 20:01 promatik

Hello @promatik

It does not remove our empty option, it just allow it to be configurable. It fallback to - as previously.

I agree we can do it for those other fields too, I am going to do it.

Thanks 👍

pxpm avatar Jan 20 '21 10:01 pxpm

@pxpm maybe I was not clear, I think we should use the placeholder argument of select2. I tried but it removes our empty option, so we would need to also allow the clear button.

Please take a look when you can 🙌

promatik avatar Jan 20 '21 22:01 promatik

Indeed, that's why in this scenario we don't use the placeholder from select2.

Atleast for me, having a clear button to clear a single selection is kind of meh (I mean, i don't like it, but think there could be people that likes it and I cannot judge). So I am open to introduce allow_clear for those people that want to use the functionality provided natively by select2. I agree with our "enforcement", it's how I use my fields and I think it makes sense, but making it configurable is just the best of the two worlds.

This is mainly due to the fact that when using select2, people expect to read select2 docs and be able to use the field accordingly to a select2 field.

Let me know what you think.

pxpm avatar Jan 24 '21 14:01 pxpm

Ok, let's make it more clear 😅

I agree with;

  • I also don't like to don't have and "empty" option. It would be nice to have the empty option (-) instead of the clear button.

I disagree with;

  • I don't like the placeholder to be the empty option, I don't feel it to be natural and a good UX "Select an Article" shouldn't be the option for those who don't want to select an article ...

That said, I think we should stick to select2 default behavior, a valid placeholder, no empty option, but a clear button. It's different than the default html selects, but it's the same as the other selects we have in backpack.

I think we need the tie breaker @tabacitu 😅

promatik avatar Jan 24 '21 20:01 promatik

@promatik

Rephrasing: from my understanding the select2 default placeholder is an empty option per-se. You just don't add it manually, it gets added by select2.

The only notable difference here is the clear button. That in my opinion makes sense in multiple, so you don't have to clear by hand all the options. When you hit clear select will revert back to Select an Article (the placeholder) so in the end is the difference of clicking clear or selecting the placeholder by hand.

I wouldn't mind having the clear button in select2, as long as I can disable it (I use mainly select2 because of the search functionality, in terms of select itself I would like to stick with the custom html select).

Let me know.

Best, Pedro

pxpm avatar Jan 25 '21 12:01 pxpm

Tie breaker says - this is a feature 😀 Moved to 4.2 😂 Reply if you think I'm wrong.

tabacitu avatar Jan 29 '21 14:01 tabacitu

@pxpm, if "allows_null" is false then the field select the first option by default; it can lead a user to enter wrong data. We can keep a null option all time. If we need to set the required field then can add a required attribute. Here is my suggested code:

$field['placeholder'] = $field['placeholder'] ?? false;

<option value="">@if($field['placeholder']) {{ $field['placeholder'] }} @else - @endif</option>

emrancu avatar Aug 19 '21 15:08 emrancu