Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

selectAll fails with duplicate option values #1197

Open
sosherof opened this issue Apr 21, 2021 · 2 comments
Open

selectAll fails with duplicate option values #1197

sosherof opened this issue Apr 21, 2021 · 2 comments

Comments

@sosherof
Copy link

sosherof commented Apr 21, 2021

Multiple select options can have the same value. This bit of code below from the selectAll function only finds and selects the first option, based on the value of the input. Or rather, it finds the same option multiple times and never selects the duplicates down the list.

$('input:enabled', allOptions).each($.proxy(function (index, element) {
             var value = $(element).val();
             var option = this.getOptionByValue(value);
             $(option).prop('selected', true);
}, this));

This impacts the button text (in this case, the nSelected text gets used because there's a count mismatch) and which options are indeed in a selected state.

I couldn't think of a great way to handle this while allowing for filtering, etc. without a fundamental change. My suggestion is that the build event assign a unique ID to each option (perhaps the select element's ID plus a counter) and use that in a data attribute of the input so every input can be correctly related directly to its option and vice-versa.

I suppose another way to handle this is detect if there are duplicate option values, then further use the text of those options to match them up. While this might be a simpler approach, it seems more prone to problems. This approach [probably] only requires modification to the getOptionByValue function (and everywhere it's called) to also accept the text of the option.

@s-eckard
Copy link
Collaborator

s-eckard commented Jul 9, 2021

I'm right now working on a solution to this problem by generating IDs for each option. This solves another issue on the fly where labels of checkboxes right now don't have a "for"-attribute.
But I'm not quite convinced from my current solution.. I don't want to depend on the ID of the select because this would force selects to have an ID. This is something everybody always needs to remember if they use the multiselect even if they don't need the ID for their own code/style.
My solution right now is to have a counter for the created multiselects and out of this generate a unique ID for each option.
But this just don't feel right to have a global counter to fix this...

If there are any better ideas how to generate a unique ID for options I would like to hear them ;)

@ditor2
Copy link

ditor2 commented Sep 24, 2021

I ran into the same problem with version 1.1.1. I was surprised because I thought that the link between option-s and the inputs is their position (of course this wouldn't allow changing options manually - but that weren't a problem for me).

What if multiselect would use ids if all the option had an unique id and do the current value based linking as a fallback when there's no ids? (With dataprovider support, eg. {label: 'Option 1', title: 'Option 1', value: '1', selected: true, id:'myUniqueId_1'} )
Then a line in the documentation may cite that an unique id must be supplied to the options by multiselect user when using same value on multiple options.

s-eckard added a commit to s-eckard/bootstrap-multiselect that referenced this issue Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants