jquery-archive / jquery-mobile

jQuery Mobile Framework
https://jquerymobile.com
Other
9.7k stars 2.41k forks source link

search box sloooow on iphone #1564

Closed markusweb closed 13 years ago

markusweb commented 13 years ago

we have created a mobile version of our website. there is also an iphone app built with phonegap. in both cases the search box is slow when there are many list entries. we have three lists, one with about 20 entries and two longer lists, over 100 entries. on the short (20 entry) list there is a small delay of about 1 seconds when hitting letters. you can't do a search on the long lists. you have to wait several/many seconds. iphone does not respond at all. is there a way to make the search function faster? in firefox on mac/pc there is no problem at all. only on iphone, it doesn't matter whether it is safari in webapp or phonegap.

markus

toddparker commented 13 years ago

Are you using alpha 4.1 or the nightly build? We mad e alton of improvements since 4.1

markusweb commented 13 years ago

i am using the alpha 4.1 version. just tried the "latest" version http://code.jquery.com/mobile/latest/jquery.mobile.js but there is also a long delay, i have to wait at about 11s in the list of 100 entries. with the alpha 4.1 it was about 15s.

toddparker commented 13 years ago

Thanks for following up. I just marked this as a blocker because it shouldn't be nearly that slow.

StevenBlack commented 13 years ago

Tracing this, I notice typing an uppercase first character generates TWO keyup events, thus two loop-passes, the second coming from releasing the caps key.

Therefore it's probably advisable here to ignore keyups from the caps, option, command, and arrow keys...

It would be interesting to know how the various devices isolate keyup and change events. This line could generate double events because of the double bindings via .bind(keyup change). I don't have an iPhone to see for sure... https://github.com/jquery/jquery-mobile/blob/master/js/jquery.mobile.listview.filter.js#L26

@markusweb, can you check that your generate markup is clean, in other words, the immediate children of the list are all li elements? Currently the code iterates all children so it's worth checking the list markup for cruft.

markusweb commented 13 years ago

@stevenblack: yes, all LI with some dividers between (a-z). does it matter if there were loaded many pages into the dom? will the search be slower? our app is an event calender app which loads infos from our webserver...

StevenBlack commented 13 years ago

@markusweb, I've looked into that and the lists and their list items are nicely isolated, so the number of pages, and the number of other search filter lists in the DOM should not factor much.

I can see an optimization here, in particular the values of the list's item.text().toLowerCase() can be cached -- this gets evaluated on every keyup for every item in the whole list, which is redundant. This is especially costly since the items can potentially have many child nodes. https://github.com/jquery/jquery-mobile/blob/master/js/jquery.mobile.listview.filter.js#L44

toddparker commented 13 years ago

Steven - Do you want to go in and try to make some optimizations to the search feature? Before A4, you could search against a few hundred list items on an iPhone 3GS/4 pretty easily so there is a lot of speed to get back to here.

StevenBlack commented 13 years ago

I'm on it. Dibs.

nsaleh commented 13 years ago

Hi, I just want to mention that I have been/and am working on this and created a pull request https://github.com/jquery/jquery-mobile/pull/1475 . Wether my modifications are pulled or not maby they are inspiring.

StevenBlack commented 13 years ago

@nsaleh Thanks! I didn't know about that pull request -- I'm fairly new to watching this aspect of the project. I wish there was a way in GitHub to filter pull requests by package.

Great ideas there, and some nice insight in the comments of the pull request, particularly by @hakanson.

I'm going to run metrics on your pull code now...

StevenBlack commented 13 years ago

@toddparker I just benchmarked @nsaleh 's pull request and it's sweet. https://github.com/jquery/jquery-mobile/pull/1475

On a 1000-item list I measured the first letter response at about the same, but subsequent letters were dramatically better, factor of 10 to 30 better.

@nsaleh, just one thing: if you still have this branch... can I ask you to make all indents tabs? The source I looked-at was a mishmash of space and tab indents. Otherwise don't sweat it.

Also @nsaleh, on a subsequent pull request, if you want to implement @hakanson 's class-based hiding idea, you can use class .ui-screen-hidden for that. But do that later, pull 1475 is fine as-is, tabs aside if you can fix that, if not no sweat.

nsaleh commented 13 years ago

@StevenBlack thx for testing. Yes i Hope i can make it sweat. i m having some IDE probs. I ll add Class based Hiding too considering your naming advice i think i ll have it Done Monday evening, maby earlier.

nsaleh commented 13 years ago

done SHA: 4ffb0a5f33ecf7a0df5caa1ed69dc02b57126195

toddparker commented 13 years ago

I think this issue can be closed with this commit. Thanks so much for your help @nsaleh! https://github.com/jquery/jquery-mobile/commit/1387fd457fb4cbcc88219c1598e26c3ac0dfe2f4