kenwheeler / slick

the last carousel you'll ever need
kenwheeler.github.io/slick
MIT License
28.54k stars 5.89k forks source link

Add matchMedia support for responsive capabilities #560

Open kyleva opened 10 years ago

kyleva commented 10 years ago

The size of the viewport is not aligned in CSS and JavaScript (e.g. a CSS media query pixel value and a $(window).width() do not have consistent values). When utilizing the responsive capability for Slick it's difficult to style the breakpoints differently. I hate to suggest adding additional dependencies but it'd be great if you could specify a breakpoint in the configuration and have it align with your CSS like so:

responsive: [
    {
        breakpoint: 991,
        settings: {
            slidesToShow: 1,
            slidesToScroll: 1,
            infinite: true
        }
    }
]

And this CSS:

@media(min-width:992px){
    .slick-styles {
        color:blue;
    }
}

Currently they don't align because the value of the viewport width is not consistent between javascript and CSS. In the checkResponsive method within the for loop the conditional can be modified to use matchMedia (or Modernizr's Modernizr.mq method too) if it's been loaded and that would make the values consistent.

mdmoreau commented 10 years ago

Just ran into this issue, but I think there's a pretty simple solution. The $(window).width() used in the script doesn't include browser scrollbars in the calculation, while CSS media queries do include scrollbars. Swapping out all instances of $(window).width() for window.innerWidth will correct this issue in all modern browsers that support that property (and media queries).

IE8 and below don't support window.innerWidth, so a fallback to $(window).width() would need to be used if it comes back undefined. This is working for me while using respond.js since the values it uses exclude the scrollbars as well.

var winWidth;
if (window.innerWidth) {
    winWidth = window.innerWidth;
} else {
    winWidth = $(window).width();
}

Great script by the way - wish I had found it sooner!

gseguin commented 10 years ago

There is PR #90 for that.

mdmoreau commented 10 years ago

I think it's a little bit different myself. Using window.innerWidth with the fallback would keep things working in IE8 and below using respond.js, while the matchMedia fix is just aimed at IE9+. It sounded like IE8 support was a must for the script (I know it still is for me), so I thought a simple fix might be enough.

It would be nice to add any type of media query though, but it seems like that would be a hassle to get working in IE8 with no matchMedia support. I was a bit confused by the breakpoint setting initially - seems like it could be much more intuitive if it was done in the typical mobile first manner, but I suspect that might cause problems when trying to support older browsers.

kenwheeler commented 10 years ago

@gsequin does that PR include width fallback?

gseguin commented 10 years ago

@kenwheeler no it doesn't. I had misread @mdmoreau's comment, sorry.

kenwheeler commented 10 years ago

@Gseguin @mdmoreau I think both match media and the correct width would be good. I gotta figure out how to do that. Maybe enforcing a media query format that allows parsing so I can fallback to a width check.

mdmoreau commented 10 years ago

Would it be possible to let people specify separate options instead of trying to convert matchMedia to something IE8 can use? Since you want to maintain support for browsers that can't use matchMedia, it seems like it would be easiest to avoid it altogether and just use the innerWidth test with a fallback since that will need to be done anyway.

I was thinking there could be an option where you can specify min or max width as the media query and a width value separately, like how it is now. So instead of just breakpoint: '768px' you could do something like breakpoint: ['min', '768px'].

Then in checkResponsive you could change the width check < to <= or >= depending on which is specified. I think this would take care of the more simple, singular media queries, but it wouldn't handle combinations or height without adding separate checks for those.

kenwheeler commented 10 years ago

The question is, what if people want to combine the two. Is it one or the other?

mdmoreau commented 10 years ago

What about changing how responsive works a little bit?

responsive: [
    {
        min-width: 768,
        max-width: 1024,
        min-height: 320,
        max-height: 640,
        settings: {
            // breakpoint options
        }
    }
]

Then the checks could see which of min/max width/height exist and run the window.innerWidth/Height and fallbacks accordingly.

EnigmaSolved commented 10 years ago

What about leaving responsive as-is, and adding a new option to hook into enquire.js? Why reinvent the wheel? Enquire.js is built for doing different things at different Media Queries, and it accepts true Media Query strings (eg, "screen and (max-width:1000px)") so one knows that their CSS and JS media queries will match. It's what I already use for this kind of stuff. :)

Perhaps you could name the additional option mediaqueries, or just mq? And have it accept the same kinds of strings that Enquire.js accepts, which would then be passed to Enquire.js to handle the logic. If the user tries to use this option without Enquire.js you'll want to decide how to handle that of course (eg, log to console, alert, silently fail, throw an error).

Users that are content with the current responsive abilities can continue to use it (and I'd suggest continuing to support it). But those that require more may very benefit from Enquire.js for other aspects of their JS anyway. And they can make their decisions about legacy browser support and how far back they need to polyfill.

Oh, and I'd make it either/or regarding responsive and mq. Perhaps mq takes precedence? Or throw an error if they try to use both (since it would be redundant).

I think this plan could keep you from unnecessary work, help keep Slick from unnecessary extra code, while still enabling more powerful options for those who want/need them.

:)

mdmoreau commented 10 years ago

I like the idea of a new mediaQueries option for more advanced stuff - makes a lot of sense to keep things separate. It sounds like this should even do the trick for IE8 and below with the media-match polyfill and respond.js with matchMedia removed.

Seems like this would be a good solution if you're willing to require an extra script. The only benefits I see for the other method would be that everything is included within slick and respond.js wouldn't need to be adjusted to work properly.

EnigmaSolved commented 10 years ago

I forgot about Respond.js. Obviously since I already use Enquire.js I'd prefer support for that, but I imagine there will be folks that will just want Respond.js support. Could things perhaps be constructed in such a way as to allow the user to define which script will process the media query? There would still be limits to this as not all media query scripts will interface in whatever manner Slick ends up needing.

CC @WickyNilliams (developer of Enquire.js) and @scottjehl (Respond.js) in case either would like to chime in.

kenwheeler commented 10 years ago

I think an enquire.js integration would be much more likely, as it actually fires events upon hitting certain breakpoints, where respond.js simply polyfills media queries on legacy browsers.

How do you gentlemen feel about a slick method that can be called via enquire that contains the same settings a responsive option setup would, and causes slick to take those options and rebuild if necessary?

EnigmaSolved commented 10 years ago

You know, as I'm thinking about all this, do we even need a new method in Slick, or could we just use the slickSetOption(option,value,refresh) method and call it in Enquire.js however we want? I've not looked into what's happening behind the scenes with how the responsive option handles making changes to the layout, but it seems possible to me that slickSetOption() could be utilizing the same underlying methods for refreshing/rebuilding Slick.

EnigmaSolved commented 10 years ago

Okay, in looking at the code for responsive it looks like a refresh is called, which gets the currentSlide, performs a destroy() followed by an init().

Whereas slickSetOption() calls unload() (which it looks like removes dots, arrows and some Slick classes) followed by reinit() (which I think also retains the current slide).

So at my initial looking, these both seem pretty similar, with perhaps the main difference being that the responsive route does a complete destroy() whereas slickSetOption() seems to not involve a destroy() of the entire Slick (but does remove some parts and rebuild per the new options). It's not immediately obvious to me whether a complete destroy() is necessary or not for our Media Query scenarios? Perhaps it would depend on what settings are being changed? I would imagine (though I've not checked) that one could change all the same settings via slickSetOption() as one can via responsive, in which case maybe some of this code could actually be combined? Just thinking out loud (so to speak). :)

mdmoreau commented 10 years ago

I guess I'm a bit confused about what constitutes IE8 support. It seems like if the goal is not to worry about respond.js then we can assume that the initial, non-breakpoint-specific settings are what IE8 will use - essentially a fallback. Right now slick does support IE8/respond.js because it's using the window width instead of actual media queries.

If that's the case and require.js doesn't need to be supported, couldn't matchMedia support be used to allow media query breakpoints for IE9+ using the responsive settings like proposed initially by @gseguin? IE8 would then just get the fallback settings as it doesn't understand the media queries. I might be misunderstanding something, but it seems like a switch from the current implementation to the new one would remove existing IE8/respond.js support.

I do like the slickSetOption() idea a lot though. It seems like using that you could set the fallback settings for IE8 using slick, and then have enquire.js separately enhance the slideshows for any browsers that support media queries. Doesn't seem like this would support respond.js, but if that's not an issue then it should work great.

EnigmaSolved commented 10 years ago

If we're using Enquire.js, then older browser support can just be governed by which polyfill one uses in conjunction with Enquire.js: http://wicky.nillia.ms/enquire.js/#legacy-browsers

The media-match polyfill apparently supports IE 6-9.

Or if one just wants to polyfill CSS3 browsers missing the matchMedia API you can just use the smaller polyfills (linked on the Enquire page) by Paul Irish / Scott Jehl. And in that case you can define a shouldDegrade state in Enquire.js to set what non-media query browsers should get.

So if we went with slickSetOption() (or something similar) direction then any Slick fallback state (if needed, depending on polyfill chosen) can be easily defined in Enquire.js.

Enquire.js is pretty slick (no wordplay intended)! I've been using it a while and like it a lot! :)

mdmoreau commented 10 years ago

Just did a little tweaking of an existing project and migrated all slick responsive settings to enquire.js. It worked perfectly in modern browsers out of the box, and IE8 got the fallback settings from slick. I did have to re-include the slick fallback settings in the unmatch portion of each enquire.js media query though to get things to adjust properly when resizing the window outside of the media query.

To get IE8 support with respond.js I needed to load 1) respond.js with matchMedia removed 2) media-match polyfill 3) enquire.js in that order. I got this working perfectly pretty quickly on a fairly complex slideshow, so it seems like it should be a solid fix. Slick might not even need to be changed at all then - just mention using enquire.js with slickSetOption(). Great suggestion @EnigmaSolved!

mdmoreau commented 10 years ago

One potential issue - it seems like if you have multiple breakpoints sometimes enquire doesn't process them in the correct order. For example, with min-width 32em and 48em media queries the slideshow is only adjusting to the correct slidesToShow when resizing to increase the width of the browser, but decreasing it totally ignores the 32em media query regardless of the enquire.register() order.

mdmoreau commented 10 years ago

The only way I've been able to get it to work resizing both directions is to use unmatch to set slidesToShow to the next smallest media query's value. I've only tested with min-width, but here's the basic logic I've used.

slick default slidesToShow: 1 enquire min-width: 32em, match slidesToShow: 2, unmatch slidesToShow: 1 enquire min-width: 48em, match slidesToShow: 3, unmatch slidesToShow: 2

The main issue I see with this approach is that once again you're limited to the media queries that you can use. If only using one type of min/max width/height it seems like it would work, but if you're not transitioning between two predictable breakpoints, I'm not sure how it would work.

From some of the issues listed on the enquire.js page it sounds like you can't really control in what order match and unmatch fire, so media queries don't typically behave how you think they would. Have you dealt with anything like that before @EnigmaSolved? I don't have much experience using enquire.js.

EnigmaSolved commented 10 years ago

@mdmoreau, I'll have to think on all that some, and also do some testing of my own. I've been using Enquire.js with just one breakpoint between the desktop and mobile versions (I use Foundation 3 for my site, so I'm matching the media queries it uses), which is a pretty simple, straightforward setup. I've not done more complicated setups yet with Enquire.js. But I would be wanting to use more breakpoints with Slick (which I just started experimenting with over the weekend, with the responsive option), so I'd like to try those out in Enquire.js and see what I run into. That will also better help me understand some of the things you're raising/asking/observing.

The goal of Enquire.js (as I understand it) is to mimic CSS media query behavior in JS, so in theory if you construct your media queries like would be needed in order for them to behave properly in CSS, then you should get predictable behavior in Enquire.js. But I could be mistaken. :)

I'll also be interested to hear from @kenwheeler regarding any performance considerations (or other things to consider) regarding the difference between how responsive works and slickSetOption(), a la my earlier observations (https://github.com/kenwheeler/slick/issues/560#issuecomment-60696979).

mdmoreau commented 10 years ago

Yeah it seems like enquire.js should be able to work like CSS, but I think part of the problem is that JavaScript just doesn't work the same in allowing cascade. When I had two min-width queries, the largest was working correctly, but when scaling down the window, the next one down never kicked in. Scaling up works correctly because a new query becomes true, so that match gets triggered.

This makes sense because the smaller media query is true even while the larger is true, but there's no trigger when switching from one to another when both are true - this is why the unmatch function is there it seems. My guess is that for max-width queries it will work in reverse - scaling the window down will work, but scaling up will do nothing.

In some quick testing it seemed like specifying exact ranges for media queries worked a bit better since match is triggered scaling both up and down. So you'd need to use (min-width: 32em) and (max-width: 47.99em) with (min-width: 48em). That's not ideal in my opinion, but I'm not sure how you'd get around that.

Seems like enquire.js needs to loop back through every media query and recheck whenever one gets unmatched, but I thought that was what it already did. You'd then need to order the queries in the reverse order as the CSS and pick the first that ends up being true, simulating a similar cascade effect.

EnigmaSolved commented 10 years ago

Yeah, that all makes sense. And now that I think about it, enquire.js has the ability for you register more queries and handlers at will (ie, spread out within your code), so that could get difficult to try to sort into a certain order (within enquire.js) because what should/would end up determining the "order" of the media queries? I actually think that doing things like (min-width: 32em) and (max-width: 47.99em), (min-width: 48em) is a great way to go (even though it is a bit more verbose) because it makes it super-clear what will happen when. As you've illustrated well, the async nature of JS makes the CSS cascade principles difficult/complex to replicate. So I think just defining the range for each media query is a great way to go.

Having said all that, it'd be worth reaching out to @WickyNilliams (https://github.com/WickyNilliams/enquire.js) to see if he has any thoughts about all this and/or if there are potential improvements he could make to enquire.js that would help with this. He seems very approachable and easy to work with. :)

EnigmaSolved commented 10 years ago

I reached out to @WickyNilliams. He's going to try to drop by this thread at some point. :)

mdmoreau commented 10 years ago

A couple of jsfiddles for reference:

min-width demo max-width demo

You can see how it works when scaling the window up for min and down for max, with the exception of the first rule (always true). If it's possible to loop back through all the media queries whenever one is unmatched, then it seems that something like this should work.

In those demos the script would take the last true media query and use it, but it might be easier to reverse the order and take the first true one. Would be great to keep it as similar to how CSS works if possible, but I'm not sure how much effort that would take.

WickyNilliams commented 10 years ago

Long thread, probably gonna be a long answer :)

I've never used slick, though I have come across it. It would take time for me to read through the docs, give it a whirl, and get an good understanding of the lib - so apologies in advance if I am slightly off with some details, I'll just speak on what I've gleaned from this thread.

I'm with @EnigmaSolved on this. I think it's misguided to try and add this functionality directly into the lib, as it feels like a case of (well-intended) feature creep. It's better for users and maintainers to have a smaller, more focussed feature set. Media queries are a completely separate concern in code, and it would be wiser to instead focus on making it really easy to integrate with other libs that can handle this concern.

Ideally it should be so easy for developers to integrate with other libs that no extra code or support is necessary e.g. for enquire:

var $slick = $(".some-element").slick(myOptions);

enquire.register("(min-width: 40em)", {

  match : function() {
    // whatever is appropriate
    $slick.updateSettings(someNewOptions);
    // could even call some methods?
  },

  unmatch : function() {
    // reverting back to what we initially had
    $slick.updateSettings(myOptions);
  }

});

Some developers might even want to use the raw matchMedia API:

var $slick = $(".some-element").slick(myOptions);

function handleMediaQuery(mql) {
  if(mql.matches) {
    // whatever is appropriate
    $slick.updateSettings(someNewOptions);
    // could even call some methods?
  }
  else {
    // reverting back to what we initially had
    $slick.updateSettings(myOptions);
  }
}

var mql = matchMedia("(min-width: 40em)");
mql.addListener(handleMediaQuery);
handleMediaQuery(mql);

I'm not sure if this exact functionality exists in Slick, or if something comparable does (or even if i've got the API entirely wrong!), but that's how I would want/expect it to work for these reasons... You avoid convoluting your API or suffering feature creep, developers get to work however they are familiar with MQs in JS, and you may even to delete a bunch of code! You could also make your current responsive solution an optional addon for devs who are happy with how it works now. Maybe it requires more code than that to integrate slick with other libs? Then you could write some "official" functions/objects that could marry slick with some other lib.

The point I'm trying to make is that it feels like an outside concern that you should let some other lib handle.

An aside, relating to enquire: @mdmoreau you need to provide upper and lower bounds for you MQs if you want it to work as intended http://jsfiddle.net/8fkgaw4v/1/. Or you can use unmatch callbacks to undo whatever you did in the match callback, like reverting to black: http://jsfiddle.net/8fkgaw4v/2/

EnigmaSolved commented 10 years ago

Thanks @WickyNilliams! That's super-helpful! :)

One clarification: Some of our discussion in this thread was regarding grappling with potential differences between how media queries can be approached in JS vs in CSS. Because enquire.js accepts the same media query strings as are used in CSS that got us thinking about how (or if) to achieve the cascade nature of CSS with media queries in JS (and in Enquire.js in particular). So for example, we wondered if we could define queries in a particular order, similar to how one can in CSS, and then have them function like in CSS (that's why @mdmoreau's recent examples lack lower bounds--we're experimenting conceptually with simulating CSS ordering).

But as we've been discussing things and as I've thought more about how I understand Enquire.js to work (eg, the ability to register queries at-will throughout one's code, thus making it difficult to define a "order" to the queries) it seems that it would be very difficult/costly to try to replicate that (CSS behavior). And thus defining upper and lower bounds (as you mention above) is the best way to define predictable media query behavior in JS (unless one is working with only one query and/or maybe certain very simple situations where unmatch is sufficient). This seems especially true once one gets into much more complicated media queries (which could be a nightmare to try to order correctly, I would think).

https://github.com/kenwheeler/slick/issues/560#issuecomment-60831792 and https://github.com/kenwheeler/slick/issues/560#issuecomment-60843432 are some of the above discussion.

I believe others will appreciate having this clarified (though I read your excellent comment as speaking to this some already). Am I on target with my above reasoning, and in particular with how Enquire.js works and what your approach is with that script (both in terms of how you've built it, and how you recommend it be used)?

mdmoreau commented 10 years ago

Getting back to the original issue of CSS and JS values not matching up, is there a reason that https://github.com/kenwheeler/slick/issues/560#issuecomment-60500201 shouldn't be used for the current responsive settings? If the new media query options can be handled outside of the script by enquire.js or something else, it still seems like it would be a good idea to normalize the values as the slick responsive settings will probably be sufficient for a lot of projects.

kenwheeler commented 10 years ago

@mdmoreau agreed.

mdmoreau commented 10 years ago

Did you want me to create a PR for this? I've seen a couple similar ones, but from what I can tell none of them seem to address the IE8 fallback width.

kenwheeler commented 10 years ago

yeah send one up and lemme take a look

mdmoreau commented 10 years ago

Noticed that some innerWidth stuff got added in with the new respondTo options - did you still want me to make the PR? There are a couple instances of $(window).width() which I was going to replace with a new function (uses innerWidth for modern browsers, width for IE8 fallback), but I'm not sure if that's still necessary. I'll try to do some testing to find out, but figured it was worth asking about before.

kenwheeler commented 10 years ago

Let me know the results of your tests. I still want to include the arrows refresh for sure.

mdmoreau commented 10 years ago

I think it looks good as-is from what I've tested. The only other instances of $(window).width() seem like they're used just to check whether the window has changed in size at all, but the values themselves aren't used in any calculations that would affect anything.

kyleva commented 9 years ago

Did you guys have a conclusive result from this? I'm super curious to see how it went (this was my first time being involved/able to view an active thread on GitHub -- pretty cool to me). I came up with a solution that worked very well for my use case and would share it here except my unminified source files are at another location. Very cool to see you guys collaborate and come together on a solution, though. Thanks a bunch.

EDIT: I'll share my solution tomorrow when I have access to my unminified source. Just want to contribute in any way I can!

mdmoreau commented 9 years ago

I think it's all fixed from what I tested a while back. var windowWidth = window.innerWidth || $(window).width(); got added based on a separate issue, which seems to have done the trick.

desandro commented 9 years ago

You might want to try Flickity. Flickity uses your own CSS to handle size and positioning, so you can easily use media queries. See demo http://codepen.io/desandro/pen/azqbGV You can even enable/disable the carousel within CSS with Flickity's watchCSS option