Minor performance optimization for closestEnabledButton in buttonMarkup#3122
Conversation
|
And so we come full-circle, almost all the way back to what the original implementation used, which used $.fn.hasClass() which basically does a string indexOf() under the hood which this pull request is proposing. The one thing we were trying to do with hasClass() was make sure we didn't get any false positives since some of the button hierarchies built up use classes like ui-btn, ui-btn-inner, ui-btn-text, etc. It would be interesting to weigh the original hasClass() against the current 2 in the jsperf above. |
|
What was the reason we switched to inArray instead of indexOf? Wasnt it something about IE support? |
|
You switched it from $.fn.hasClass() to using split() and Array.indexOf() here: b0db897#js/jquery.mobile.buttonMarkup.js which was a problem because IE7 (WP7) didn't support Array.indexOf so @Wilto switched things to $.inArray(): |
|
Yeah—calling indexOf on an array causes Internet Explorer to go stomping off to its room, slamming doors and yelling about how “when it turns eighteen it is so out of here .” …That was a strained metaphor, but you get what I’m saying: using indexOf on an array breaks IE 6/7/8. |
|
As long as IE7 likes indexOf with strings (and other browsers too of course), I think this PR looks good. |
Minor performance optimization for closestEnabledButton in buttonMarkup
|
@hpbuniat thanks for the patch. |
|
One thing i thought of that i should ask, is there ever a time when ui-btn gets prefixed? |
|
I'm glad i could help. @eddiemonge, i've counted 59 occurrences of ui-btn in the code, none is prefixed. There is also no css that matches to /[^\.]ui-btn/. |
|
@hpbuniat is correct, we don't namespace/prefix ui-* attributes. That is, all of our CSS class names start with ui- and nothing before that. |
I changed the $.inArray to indexOf - results at http://jsperf.com/fsdsgt4.
The biggest impact comes with real mouse usage, as the vmouseout/vmouseover events are fired very often.