search

brianvoe / slim-select

Slim advanced select dropdown
http://slimselectjs.com
MIT License
1.03k stars 195 forks source link

Working Select Fails when Converted to SlimSelect - BUG IS: Doesn't handle duplicated values #543

Closed dbareis closed 1 month ago

dbareis commented 4 months ago

Describe the bug SlimSelect doesn't handle multiple options with the same values. What I think happens is which ever option you choose it will select the first one found with that value.

Recreate it in Codepen DONE: https://codepen.io/Dennis-Bareis/pen/zYXXVmw

To Reproduce Steps to reproduce the behavior:

  1. Click Option 2 or 3 they won't work

ExampleOfFailing Select.zip

brianvoe commented 4 months ago

That is what it does. It only has a value to select the option.

If you have an improved implementation please submit a pr. cause multiple select could be handled better too

deh-code commented 2 months ago

Hi, i'm facing the same issue.

I noticed that store.ts can be used to set selected options by id, wouldn't it be better to store ids instead of values for selected options?

I know this might require some critical refactoring, but i'd be willing to make a PR if you think the id approach is a good idea.

brianvoe commented 2 months ago

You are more than welcome to submit a pr

deh-code commented 2 months ago

Hi, I just submitted a PR. It passed all the tests, i also run a dev server and all components worked just fine.

Please let me know your opinion about the implementation and if i missed something.

brianvoe commented 2 months ago

oh wow this looks like a lot more than i was expecting.

Tests were something that were recently added so im not sure they will cover all the bases to ensure everything is still working.

Ill try to set aside some time tonight to go through things and either get it merged in or at least give you some feedback.

Thanks for putting the time in I really appreciate it. Its becoming more rare that people contribute.