Select2 default allow clear
Hi @alexgreat70,
thank you for your contribution. Can you explain the reasoning behind this change?
Hi @alexgreat70,
thank you for your contribution. Can you explain the reasoning behind this change?
This change adds the ability to reset the default field values, which is very convenient.
I tested it and agree that it makes a lot of sense. In the form input case it can be a bit confusing, though, if the underlying select does not have a blank option: There should not be a way to empty the select, only to choose another option. On the other hand, if a blank value is present, the new behavior is better since before Select2 did not provide a way to reset the select. Still, the label of the blank option is not displayed.
I'd suggest to
-
change the implementation such that
placeholderandallowClearare only set wheninclude_blankis true for the select input. This info could be passed using a data attribute or by looking for therequiredattribute that Formtastic appears to be setting on the surrounding div. -
as a bonus, consider looking for the
optionwith blank value and using its text as placeholder. This would make sure the blank option label is displayed.
What do you think?
I tested it and agree that it makes a lot of sense. In the form input case it can be a bit confusing, though, if the underlying select does not have a blank option: There should not be a way to empty the select, only to choose another option. On the other hand, if a blank value is present, the new behavior is better since before Select2 did not provide a way to reset the select. Still, the label of the blank option is not displayed.
I'd suggest to
- change the implementation such that
placeholderandallowClearare only set wheninclude_blankis true for the select input. This info could be passed using a data attribute or by looking for therequiredattribute that Formtastic appears to be setting on the surrounding div.- as a bonus, consider looking for the
optionwith blank value and using its text as placeholder. This would make sure the blank option label is displayed.What do you think?
I think this should not be done, placeholder can be specified if it is really necessary, we have been using your gem for a month already and only added feature works for us, it works through fork, we are waiting for confirmation of the merge so that we can use your repository directly in Gemfile
Since the change makes matters worse in the case of include_blank: false, I will not merge the current version. I'll leave the PR open in case somebody else decides to pick it up.