codedance / jquery.AreYouSure

A light-weight jQuery "dirty forms" Plugin - it monitors html forms and alerts users to unsaved changes if they attempt to close the browser or navigate away from the page. (Are you sure?)
508 stars 145 forks source link

Are you sure turns Chosen Plugin slow #76

Open rafam31 opened 9 years ago

rafam31 commented 9 years ago

I realized my large select (with Chosen) get very slow when i was using AreYouSure...

So, first i thought the problem was in chosen.. but, wasn't..

The problem were here: https://github.com/codedance/jquery.AreYouSure/blob/master/jquery.are-you-sure.js#L53-L57

I think this loop $.each is unnecessary, and i changed these lines to:

val += $field.find('option:selected').val();

Apparently, everthing works fine. So, i suggest this change, if possible.

codedance commented 9 years ago

Out of interest, how long is the list?

I'll need to go back through the code history. There is a bit of history to this section of code:

1) The first implementation was done using string concat on selected options. 2) The 2nd implementation used .val() on the selector. We then ended up having a few bug requests filed. From memory around multi-select cases in some browsers. 3) Then it was reverted back to the old method.

If it is a considerable performance issue, it might be worth digging through the issue history and seeing how we can phase in without the regression.

codedance commented 9 years ago

Related:

The revert: https://github.com/codedance/jquery.AreYouSure/issues/55

https://github.com/codedance/jquery.AreYouSure/issues/49 https://github.com/codedance/jquery.AreYouSure/issues/29 https://github.com/codedance/jquery.AreYouSure/issues/48

Thougt's welcome :-)

chekm8 commented 9 years ago

Had the same issue with huge selects and implemented the change per rafam31. Worked great. Thanks!