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

Unable to insert a carousel into the page dynamically #42

Closed kdonald closed 11 years ago

kdonald commented 12 years ago

jquery-ui-carousel appears to depend on height/width dimensions being set on both the rs-carousel-item(s) and the dynamically generated rs-carousel-mask elements at the time the carousel() is constructed. This prevents a carousel from easily being constructed against an content fragment that is inserted into the page dynamically (as the styles are not applied until the fragment is inserted into the page). I tried setting in-line styles for rs-carousel-items that specify height/width but this didn't work as the generated mask element must have a dimension as well.

It'd be nice to see documented support for more dynamic use cases; e.g. inserting a carousel component when making a ajax page transition.

kdonald commented 12 years ago

Just an FYI: I tried my use case with another jquery-ui based plugin, https://github.com/ryrych/rcarousel, and it appears to work. This particular plugin allows specifying the dimensions of the carousel in JavaScript when constructing the carousel widget.

richardscarrott commented 12 years ago

Because DOM nodes inherently don't have any dimensions until they've been inserted into the document most DOM work of this nature shouldn't be run until the html fragment has been inserted into the document, e.g:

$.ajax({
    url: '/foo',
    success: function (html) {
        $(html).appendTo('body').carousel();
    }
});

I'd be interested as to why you'd want to init code before it's in the document?

Providing the option to explicitly pass in the width and height like rcarousel is a bad approach because a) it's relatively inflexible (what if you were using percentages?) and b) it uses JS to achieve something that CSS should be responsible for (SoC).

The only time relying on nodes dimensions can be a problem is when instantiating the carousel on a hidden element. This is easily solved however by calling .carousel('refresh') upon showing the carousel.

Hope this helps.

kdonald commented 12 years ago

Well, I'm building a multi-module JS app (think Backbone.js), so things are pretty JavaScript centric. In this kind of app I generally prefer if module initialization happens outside of document.ready & before inserting things into the page. An ability to set the size of the carousel using JavaScript meets my needs pretty well, and the ability to work with the carousel as part of a module or fragment that can be reused across my application is also desired (relying on global CSS rules isn't great for that kind of reusability). I'd be fine applying some CSS on the fly to my fragment containing the carousel, but unfortunately setting the dimensions of the carousel ul element or list items doesn't seem to work... it seems I also have to set the mask, too, which is somewhat inconvenient (I also don't quite understand it, since the mask and rs-carousel-items are set to the same thing in your examples...). In general, I've struggled quite a bit trying to get all the external CSS rs-carousel requires configured to where the carousel actually works--one benefit of rscarousel is it just seems to work even if no stylesheet is applied at all (which I've found nice for during testing).

richardscarrott commented 12 years ago

I have heard people mention they like the whole 'plug and play' of other carousel's where external CSS isn't required however I feel that technique is, again, inflexible and a misuse of JS so is ultimately done not because it's a better end result but because it's easier for those less keen to work with CSS which isn't a compelling reason for me.

I'm not entirely sure why you would need to rely on 'global CSS rules' just because the rules aren't written in JS? Perhaps this indicates a CSS management issue?

I'm guessing you're doing a lot of non-DOM work but why exactly do you prefer module initialisation to happen outside of doc ready? Either way it's usually best practice to include scripts at the bottom of the page which would mean you don't have to worry about doc ready anyway and, as you say, it won't solve the issue as the DOM nodes don't have dimensions until they've been inserted into the doc even when CSS is applied.

There's really only minimal CSS required for the carousel to work:

.rs-carousel .rs-carousel-mask {
    overflow: hidden;
}

    .rs-carousel .rs-carousel-runner {
        position: relative;
    }

        .rs-carousel.rs-carousel-horizontal .rs-carousel-item {
            float: left;
        }

and then you just need to apply a width and height (unless it's implied by the items content, e.g. images) to the rs-carousel-item elements.

kdonald commented 12 years ago

I'm using RequireJS to asynchronously load the various modules of my application. A module is activated based on the fragment portion of the URI e.g. #/, #/about, etc. When a module is loaded it creates a "mvc" object that can render a HTML "view" bound to a data "model" that is updated via a "controller". So for example, one module in my app is a view that displays a slideshow of upcoming events. So this module creates a <section/> container fragment that can be inserted to the page and contains a ul.carousel child element bound to a collection model updateable via a websocket. Internally this module sets up the carousel for use -- if I'm forced to reach in and access the carousel elsewhere I'm breaking module encapsulation.

I'm pleased to see from above that the required CSS isn't as verbose as I thought it was. I'll try again with your recommendations. For some reason I've had trouble getting the rs-carousel-mask to set its width and left properties properly without also setting its dimensions to match that of the rs-carousel-items. It might be helpful to users if you had a "simple" demo that shows the simplest possible setup for jquery-ui-carousel. Also, if it is possible where the dimensions of the carousel could have reasonable defaults or be controlled by a single setting instead of needing to be applied to each item, that would be an improvement as well I think. Just some suggestions.

kdonald commented 12 years ago

I created a little example project that shows the dynamic nature I'm after with the carousel: https://github.com/kdonald/carousel. If you run it, you can see you can add and remove elements dynamically. It's using rcarousel at the moment, which has some issues.. it doesn't support the case where there are no carousel items to display. It also doesn't support removing an item dynamically very well. Hope this is useful.

richardscarrott commented 11 years ago

I'm still not convinced a widget of this nature is able to remove it's dependency on the DOM as its so inherent with UI code.

I've made a number of changes to the carousel recently https://github.com/richardscarrott/jquery-ui-carousel/tree/0.10.7 mostly spurred on to support variable width items which actually makes it even more dependent on the DOM.

Not sure if this helps but you're totally free to instantiate the carousel before it's in the document by doing something like this:

var carousel = $('<div class="rs-carousel"><ul><li>1</li><li>2</li></ul></div>').carousel().data('carousel');
// when carousel is added to DOM, if you're using backbone I imagine in the views render method.
carousel.refresh();

I'm not sure if that would still break your rules around keeping things modular but I'm going to close up this ticket for now. If anyone else raises a similar issue then I'll point them to your example and if one day I come across the issue I might just make some changes to this.

Cheers.