New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select element as a non-native menu is too slow on Samsung Galaxy S3. #5625

Closed
ghost opened this Issue Feb 11, 2013 · 10 comments

Comments

Projects
None yet
6 participants
@ghost

ghost commented Feb 11, 2013

The select element with data-native-menu="false" which brings up a dialog component which has only 95 li elements including optgroup elements makes the list very slow in Samsung Galaxy S3 when packaged as a PhoneGap application. When our app is taken using Chrome browser, the list scrolls smoothly, but the item click which marks the checkbox is very slow. It seems like the list items is re-created on each item click. Why so?
Btw the select menu allows for multiple selections.
Thanks.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Feb 11, 2013

Member

@kadaj

Please provide a test page. See the contributing guidelines for instructions and our JS Bin template.

Is hardware acceleration enabled in the manifest file?
Also, what Android version is running on your Galaxy S3 device?

Member

jaspermdegroot commented Feb 11, 2013

@kadaj

Please provide a test page. See the contributing guidelines for instructions and our JS Bin template.

Is hardware acceleration enabled in the manifest file?
Also, what Android version is running on your Galaxy S3 device?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Feb 12, 2013

@uGoMobi

S3 is running Android v4.1.2.
PhoneGap version is v2.2.0.
Nothing about hardware acceleration is specified in the manifest file. Since the target SDK was set as 8 hardware acceleration is off.
jQuery v1.8.3.
jQuery mobile v1.2.0.

Testing
The web page can be accessed by going to https://mcare.needstreet.com and click the register button. In the register page click the specialization menu. But it work almost okay in a browser. The main problem is within the packaged app. The app can be tested by downloading the apk (v1.2.0) from play store at the link https://play.google.com/store/apps/details?id=com.needstreet.health.hp

When hardware acceleration is enabled, the scrolling becomes smooth, similar to the Chrome browser rendering, but the dialog init animation is broken. Also the delay between the clicking the list item and drawing the checkbox remains the same.

ghost commented Feb 12, 2013

@uGoMobi

S3 is running Android v4.1.2.
PhoneGap version is v2.2.0.
Nothing about hardware acceleration is specified in the manifest file. Since the target SDK was set as 8 hardware acceleration is off.
jQuery v1.8.3.
jQuery mobile v1.2.0.

Testing
The web page can be accessed by going to https://mcare.needstreet.com and click the register button. In the register page click the specialization menu. But it work almost okay in a browser. The main problem is within the packaged app. The app can be tested by downloading the apk (v1.2.0) from play store at the link https://play.google.com/store/apps/details?id=com.needstreet.health.hp

When hardware acceleration is enabled, the scrolling becomes smooth, similar to the Chrome browser rendering, but the dialog init animation is broken. Also the delay between the clicking the list item and drawing the checkbox remains the same.

@Don12

This comment has been minimized.

Show comment
Hide comment
@Don12

Don12 Feb 14, 2013

I would have to agree about the slowness of the non-native when it contains a larger number of items on Android devices. I had to use the native select menu because users did not know if an item was selected or not with the dialog taking 2 or 3 seconds to disappear.

One suggestion for the select widget would be to show the item as selected as soon as it is selected and before the dialog page disappears. The native menus do this.

Don12 commented Feb 14, 2013

I would have to agree about the slowness of the non-native when it contains a larger number of items on Android devices. I had to use the native select menu because users did not know if an item was selected or not with the dialog taking 2 or 3 seconds to disappear.

One suggestion for the select widget would be to show the item as selected as soon as it is selected and before the dialog page disappears. The native menus do this.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Feb 14, 2013

Member

We do show an item as selected as soon as it is selected (active state). But the JS that adds the active class for this, so if the whole script runs slow you probably don't get to see this either.

Member

jaspermdegroot commented Feb 14, 2013

We do show an item as selected as soon as it is selected (active state). But the JS that adds the active class for this, so if the whole script runs slow you probably don't get to see this either.

@kadamski

This comment has been minimized.

Show comment
Hide comment
@kadamski

kadamski Aug 25, 2013

I've got the same problem @kadaj described on Firefox OS device. When using non-native select menu with multiple enabled and more than a couple of items, clicking is very slow.

The reason is that each time an item is clicked, self.select.trigger( "change" ); is fired, which in turn triggers self.refresh(). The only reason that is triggered (at least I couldn't find any other) is to call self.setButtonText() and self.setButtonCount() so that the button shows up to date values. But refresh also does self._buildList() which takes a lot of time.

I have made a simple workaround to this problem - changed the code so that change is not triggered when self.menuType === "page" (if it's page, we can be sure that we have some more elements on the list so we will encounter this problem and since we don't see the button, I don't see the reason to update it). Instead, I'm calling self.setButtonText() and self.setButtonCount() in close when self.menuType === "page".

In addition to that, I think that even when self.menuType === "overlay", it's not necessary to trigger change (and do self._buildList(). Calling only self.setButtonText() and self.setButtonCount() seems to be enough for me. Unless I'm missing something?

Now I don't see any lag when selecting an item and everything seems to still be working OK. Please note that today is my first day of using jquery mobile and I only spend about an hour on figuring out this problem so I may be wrong. I only checked this on 1.3.2 so far but I will take a look at HEAD tomorrow. I may create a PR to if anybody is interested in my changes.

kadamski commented Aug 25, 2013

I've got the same problem @kadaj described on Firefox OS device. When using non-native select menu with multiple enabled and more than a couple of items, clicking is very slow.

The reason is that each time an item is clicked, self.select.trigger( "change" ); is fired, which in turn triggers self.refresh(). The only reason that is triggered (at least I couldn't find any other) is to call self.setButtonText() and self.setButtonCount() so that the button shows up to date values. But refresh also does self._buildList() which takes a lot of time.

I have made a simple workaround to this problem - changed the code so that change is not triggered when self.menuType === "page" (if it's page, we can be sure that we have some more elements on the list so we will encounter this problem and since we don't see the button, I don't see the reason to update it). Instead, I'm calling self.setButtonText() and self.setButtonCount() in close when self.menuType === "page".

In addition to that, I think that even when self.menuType === "overlay", it's not necessary to trigger change (and do self._buildList(). Calling only self.setButtonText() and self.setButtonCount() seems to be enough for me. Unless I'm missing something?

Now I don't see any lag when selecting an item and everything seems to still be working OK. Please note that today is my first day of using jquery mobile and I only spend about an hour on figuring out this problem so I may be wrong. I only checked this on 1.3.2 so far but I will take a look at HEAD tomorrow. I may create a PR to if anybody is interested in my changes.

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Aug 25, 2013

Member

@kadamski

Thanks for looking into this. We are going to review the selectmenu widget for 1.5. It would be good to only rebuild the list if needed. For the table widget we do something similar now, we split the refresh method in a refresh and rebuild method. We won't review PR's for this widget until we are done with the refactor, so it's probably better to wait with a PR. If you still see ways to improve the performance of this widget after we are done, we will be happy to receive a PR from you!

Member

jaspermdegroot commented Aug 25, 2013

@kadamski

Thanks for looking into this. We are going to review the selectmenu widget for 1.5. It would be good to only rebuild the list if needed. For the table widget we do something similar now, we split the refresh method in a refresh and rebuild method. We won't review PR's for this widget until we are done with the refactor, so it's probably better to wait with a PR. If you still see ways to improve the performance of this widget after we are done, we will be happy to receive a PR from you!

@kadamski

This comment has been minimized.

Show comment
Hide comment
@kadamski

kadamski Aug 25, 2013

I've done some more research today. First of all, git version does not have this performance issue. It still calls refresh after each click but it doesn't call self._buildList(). This is because in git version, this._isRebuildRequired() is working correctly.

In 1.3.2 it always returned 'true' since options.text() was always !== list.text(). This seems to be because of additional <span> element with white space inside li lists on 1.3.2 (which added additional character to the list.text()). This span is no longer there in git version so both options.text() and list.text() returns the same string now.

Conclusion:
While I'm still not sure calling refresh and checking if rebuild is required is actually needed, the real problem was in different place in the code. It seems that what was (or is) broken was/is _isRebuildRequired() function which compares text representation of original options list and li list generated by jqm to decide if any option node has changed (and rebuilding li representation is required). While this does not trigger problems now (it was fixed in some other place in code) it can bite again in future, if somebody change how li is generated out of option list again.

kadamski commented Aug 25, 2013

I've done some more research today. First of all, git version does not have this performance issue. It still calls refresh after each click but it doesn't call self._buildList(). This is because in git version, this._isRebuildRequired() is working correctly.

In 1.3.2 it always returned 'true' since options.text() was always !== list.text(). This seems to be because of additional <span> element with white space inside li lists on 1.3.2 (which added additional character to the list.text()). This span is no longer there in git version so both options.text() and list.text() returns the same string now.

Conclusion:
While I'm still not sure calling refresh and checking if rebuild is required is actually needed, the real problem was in different place in the code. It seems that what was (or is) broken was/is _isRebuildRequired() function which compares text representation of original options list and li list generated by jqm to decide if any option node has changed (and rebuilding li representation is required). While this does not trigger problems now (it was fixed in some other place in code) it can bite again in future, if somebody change how li is generated out of option list again.

@lukaseckert

This comment has been minimized.

Show comment
Hide comment
@lukaseckert

lukaseckert Sep 2, 2013

I`m currently experiencing a similar issue. The select box has only 3 items:

<label for="toolbarActionSelect" class="select"></label>
<select name="toolbarActionSelect" id="toolbarActionSelect" data-native-menu="false" data-mini="true">
    <option>Aktion wählen...</option>
    <option value="btnCreateNewTemplate">Vorlage anlegen</option>
    <option value="btnOpenNewFolderPopup">Ordner anlegen</option>
    <option value="btnListAll">Alle auflisten</option>
</select>

It takes about 2 seconds to open the custom select menu and about 1 second to close it after a click. The select control sample on the JQM demo page is even slower. Tested with JQM 1.3.2 and a HTC Desire X (works fine under Windows/Chrome).
Both on the default Android browser and in the desktop version of Chrome a blue rectangle is flickering before the menu popup becomes visible (at the same position).

Besides, it is annoying that the page is scrolled down for some lines as soon as popup of the custom select menu opens. This might be a result of the dropdown box being at the very top of the page. Is there any possibility to show the popup below the dropdown control (as it would be on a native dropdown menu) instead of centering it on it?

lukaseckert commented Sep 2, 2013

I`m currently experiencing a similar issue. The select box has only 3 items:

<label for="toolbarActionSelect" class="select"></label>
<select name="toolbarActionSelect" id="toolbarActionSelect" data-native-menu="false" data-mini="true">
    <option>Aktion wählen...</option>
    <option value="btnCreateNewTemplate">Vorlage anlegen</option>
    <option value="btnOpenNewFolderPopup">Ordner anlegen</option>
    <option value="btnListAll">Alle auflisten</option>
</select>

It takes about 2 seconds to open the custom select menu and about 1 second to close it after a click. The select control sample on the JQM demo page is even slower. Tested with JQM 1.3.2 and a HTC Desire X (works fine under Windows/Chrome).
Both on the default Android browser and in the desktop version of Chrome a blue rectangle is flickering before the menu popup becomes visible (at the same position).

Besides, it is annoying that the page is scrolled down for some lines as soon as popup of the custom select menu opens. This might be a result of the dropdown box being at the very top of the page. Is there any possibility to show the popup below the dropdown control (as it would be on a native dropdown menu) instead of centering it on it?

@Ruffio

This comment has been minimized.

Show comment
Hide comment
@Ruffio

Ruffio Dec 30, 2014

@arschmitz Shouldn't this issue be marked as 1.5 as suggested on the 25th of august 2013 by @jaspermdegroot as Selectmenu is a part of 1.5?

Ruffio commented Dec 30, 2014

@arschmitz Shouldn't this issue be marked as 1.5 as suggested on the 25th of august 2013 by @jaspermdegroot as Selectmenu is a part of 1.5?

@arschmitz

This comment has been minimized.

Show comment
Hide comment
@arschmitz

arschmitz Jan 9, 2015

Member

@Ruffio no we are going to be replacing the current selectmenu with the one from jQuery UI in the near future so we will not be putting any substantial changes to the current selectmenu.

Also note this MAY no longer be an issue anyway there have been many related perf improvements made in the last year and a half.

Im going to close this as wont fix ( though if there are simple things that could improve it with out ay real refactor we would be open to suggestions / pr's )

Member

arschmitz commented Jan 9, 2015

@Ruffio no we are going to be replacing the current selectmenu with the one from jQuery UI in the near future so we will not be putting any substantial changes to the current selectmenu.

Also note this MAY no longer be an issue anyway there have been many related perf improvements made in the last year and a half.

Im going to close this as wont fix ( though if there are simple things that could improve it with out ay real refactor we would be open to suggestions / pr's )

@arschmitz arschmitz closed this Jan 9, 2015

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