websemantics / Image-Select

Image Select is an extension of Chosen, a jQuery plugin that makes long, unwieldy select boxes much more user-friendly. It provides image support for Single and Multi select HTML tags.
http://websemantics.github.io/Image-Select
MIT License
148 stars 44 forks source link

Same Values = Wrong Image #9

Closed quickstick closed 9 years ago

quickstick commented 9 years ago

While I'm picking on y'all... just thought I should point out a possible issue.

Admittedly, this issue helped me find a bug before it became a problem further down the track... but it might be worth consideration.

Line 101:

if(img_src != undefined && selected.selected == value){

This part selected.selected == value causes a problem if the same value="" attribute is used on two or more <option> elements.

The selection shows the image from the last <option> that has that same value attribute - even though the selection text is correct (so it's purely an aesthetic problem).

I know why it's been done that way - I mean, the chances of you having more than one value the same is slim, but it is always possible - so you might be able to identify the selected image by $().index(); or something like that.

Hopefully that makes sense, and you can devise some way around it. If you want to recreate it:

<select>
<option value="1" data-img-src="...">Option 1</option>
<option value="1" data-img-src="...">Option 2</option>
<option value="3" data-img-src="...">Option 3</option>
<option value="4" data-img-src="...">Option 4</option>
</select>

Then select Option 1 - you should find Option 2's image is the one that appears.

Don't get me wrong, this 'problem' brought the fact I had the same value twice to my attention, so I appreciate it's existence, you may not want to fix it for that reason, but I'll leave the thought with you.

Again, thanks for a great plugin.

websemantics commented 9 years ago

Thanks for pointing out the value issue. Tried to use index as you suggested but the on change event would only provide the value of the selected option, so that didn't work. I wouldn't see many cases using the same value of different select options though, so I guess it should be ok for now,