Fix external js updates to multi-selects #1070

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

tjarratt commented Mar 6, 2013

I ran into some issues where updated multi selects from external
javascript (using $(select).trigger("liszt:updated") would frequently
get out of state because the #choices variable on the Chosen object
was no longer the same as the actual number of options selected.

Instead of having a global variable on the chosen jquery object
(which is relatively fragile because of external changes to the
select element), just calculate it as needed. It's not called often
and this is a fairly fast lookup for jquery and prototype, so I think
this is a safe change.

This is my first pull request on a js project using coffeescript, so
any feedback is welcome. I'm already using this patch in a production
setting, so if you'd like I could describe the actual use case that produced
this error condition, or provide a minimal reproduction of the issue but
in my opinion this is a fairly clear change that resolves some of the bugs
one would encounter using chosen with ember.js, or another UI heavy
library where updates to the select element are not always through user
interaction.

Tim Jarratt Fix external js updates to multi-selects
I ran into some issues where updated multi selects from external
javascript would frequently get out of state because the .choices
variable on the Chosen object was no longer the same as the actual
number of options selected.

Instead of having a global variable on the chosen jquery object
(which is relatively fragile because of external changes to the
select element), just calculate it as needed. It's not called often
and this is a fairly fast lookup for jquery and prototype, so I think
this is a safe change.
2387829
Contributor

pfiller commented Apr 26, 2013

Thanks @tjarratt

I was nervous about the way selected options were being queried, so I did some testing. It turns out that looping through the raw javascript select object's options is much faster, so I moved the counting method to the abstractChosen method.

Your commit has been added to this PR: #1171. Please let me know if this still solves your issue and any other thoughts you might have.

pfiller closed this Apr 26, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment