search

richardscarrott / jquery-ui-carousel

jQuery RS Carousel is a responsive and touch-enabled carousel written on top of jQuery and the jQuery UI Widget Factory providing a full and familiar API in less than 2.6kB minified and gzipped.
http://richardscarrott.github.io/jquery-ui-carousel/
192 stars 56 forks source link

Adding an element when the carousel is hidden causes an infinite loop #18

Closed itay closed 12 years ago

itay commented 12 years ago

If you add an element to the carousel when it is hidden (either itself or one of its ancestors), then calling the 'add' function on it will cause an infinite loop (my guess is because it's trying to calculate the width).

This might be a bug in JQ-UI - not sure.

richardscarrott commented 12 years ago

You look to be right in that '_getMaskDim' and '_getItemDim' don't return the correct values when hidden which in turn means 'getItemsPerPage' will return 0 and cause the infinite loop.

A simplified demonstration can be seen by running the following in the console:

$(':rs-carousel').hide().carousel('refresh');

A temporary workaround might be to position the carousel off the screen instead of setting it to display none but I imagine this wouldn't be suitable in many situations.

I'll have to have a think about how is best to handle this in the long term... any suggestions would be much appreciated.

itay commented 12 years ago

For now, I simply "defer" the add until it is visible, so I at least don't run into the issue. Not sure of a general fix - I'm far from a JS expert :)

richardscarrott commented 12 years ago

I narrowed the infinite loop down to getNoOfPages which was dividing a number by zero and consequentially returning Infinity. This has now been fixed however because it's not possible to get the dimensions of hidden elements it means that any manipulation done to the carousel whilst hidden will throw out it's state so upon showing the carousel again it would be advisable to refresh.

itay commented 12 years ago

This is good enough for me, and good general advice. If you've significantly changed the carousel context, such as width, it's probably a good idea to refresh it.

Thanks for fixing it!