mreq / slick-lightbox

A lightbox wrapper for Ken's amazing slick carousel.
http://mreq.github.io/slick-lightbox/
MIT License
229 stars 103 forks source link

different styles to set containers dimensions #55

Closed stratboy closed 6 years ago

stratboy commented 7 years ago

Hi, thank you for your code.

I just faced a problem where I needed to use the lightbox on a page with fixed header, and needed to keep the header visible.

Solution was something like this:

.slick-lightbox,
.slick-lightbox-inner{
  top: map-get($large, header-height); // move the top
   // reset dimensions
  width: auto;
  height: auto;
  // make fullscreen again, but with real space
  bottom: 0;
  right: 0;
}

By only moving the top, you'll end up with uncentered images, because width/height 100% makes the container still as big as the whole window. Instead, we need them to be as the window-top. In other words, the real available space. This is achieved by using top/bottom props on container (and resetting width/height).

I currently tested this solution in ff, chrome, safari, android. IE not yet.

If you'd like, I could test on IE, make the mods to your css, and send you a pull request.

Regards

mreq commented 7 years ago

Hey, better yet, I can make the resizeSlides function override-able by an option. Will look into that.

stratboy commented 7 years ago

schermata 2017-08-10 alle 11 30 57

as for example. By only move the top (to let space for the header), the image would't be vertically centered because the containers were still as big as the whole window.

stratboy commented 7 years ago

Since this mod:

// instead of width/height
bottom: 0;
right: 0;

is really fast and crossbrowser, I would suggest to use it first, because it lets the users do things with simple css instead of js. They can then just move the lightbox by setting differents top/bottom/left/with. Then yes, you could also make resizeSlides function override-able.

mreq commented 6 years ago

Sorry for the delay. Did you manage to test this in IE8+ (to copy slick browser support)?

stratboy commented 6 years ago

Sorry I do not test on IE8 and 9 since some years ago. I program websites since 1997, I did it when we where testing on Netscape (both on mac and pc, on a 56k connection, html3, you can imagine)... Nowadays, I really don't see the point in testing on IE less than v10 :) Chances are that no user will see your website on IE8, and anyway, chances are that on your website there will be something else other than slick, that will break IE8. Apart from this, as far as I know, even several social networks stopped support for such browsers.

mreq commented 6 years ago

I too support IE10+ on commercial websites and agree fully. However, slick carousel supports IE8+ and since this plugin is just a simple wrapper, it should do the same. Closing this for now.