CSS-Tricks / AnythingSlider

A jQuery Slider plugin for anything.
http://css-tricks.github.io/AnythingSlider/
GNU Lesser General Public License v3.0
1.15k stars 380 forks source link

Heights, images, loading, etc #431

Open chriscoyier opened 12 years ago

chriscoyier commented 12 years ago

Email from Andreas Doelling who didn't have a GitHub account and wanted to leave a comment:

............

I think I have discovered a bug in the new version (1.8.6) of Anythingslider that can be fixed with one small line of additional code as far as I can see. I have no Github account so I hope it is ok to send you my bug report and suggested fix via e-mail.

The problem only appears in sliders with flexible height whose panels contain images (and only in Firefox and Chrome as far as I can see): if images haven’t been loaded when the slider is initialised the panels’ heights are not set correctly.

The relevant lines are these in base.setDimensions():


h = (c.length === 1 ? c.outerHeight(true) : t.height()); // get height after setting width

if (h <= base.outerPad[1]) { h = base.height; } // if height less than the outside padding, then set it to the preset height

t.css('height', h);


The problem is that you set the panel’s height to t.height() when setDimensions() is called initially. Now, when setDimensions() is called a second time on window.load – the height does not get changed because t.height() is t.height() is t.height() … ;)

My quick fix is to reset the height of the panel:


t.css('height', 'auto');

h = (c.length === 1 ? c.outerHeight(true) : t.height()); // get height after setting width

if (h <= base.outerPad[1]) { h = base.height; } // if height less than the outside padding, then set it to the preset height

t.css('height', h);


Do you agree? Or am I totally mistaken? (I had a long working day so, maybe, I’m missing something.)

Anyway, I love Anythingslider and have been using it for a bunch of very different and complex variations. Thanks for this plugin and keep up your great work!

Bye, Andreas

Mottie commented 12 years ago

Hmm, I think you might have a point here :) Nice catch!

I'll include it with the next update.

Mottie commented 12 years ago

I've also been thinking about including some code within the plugin to check when all images have loaded instead of using the window load function; but of course, it will increase the size of the plugin.

Another alternative would be to add an option to use an images loaded plugin of your choice. I only know of two right now... this one I wrote called imagesLoaded, which hasn't been as thoroughly tested as a plugin also named imagesloaded written by David Desandro (author of Isotope and Masonry).

I think I'll just open another issue to get some feedback

lhwparis commented 12 years ago

in my oppinion a fixed "onLoad" will do it for the first time

Mottie commented 12 years ago

@lhwparis Yeah, the window load is great for the initial page load, but if the contents get updated, window load won't fire again (I don't think) but the imagesLoaded script will work as expected.

Mottie commented 11 years ago

Hopefully this issue has been fixed in v1.8.7.

htmlmania commented 11 years ago

I'm getting that problem on this page http://bydbenjamin.co.uk/fy-ystafell/ too. I don't know whats wrong i tried all that has been suggested here but none work.

Mottie commented 11 years ago

@htmlmania: I think these two lines in the style.css file are causing the problems:

  1. Remove the height: auto !important in the img definition

    img { max-width: 100%; height: auto !important; }
  2. Then add a width to this definition, and remove the !important flag:

    div.anythingSlider {
       height: 546px !important;
    }
htmlmania commented 11 years ago

Thanks @Mottie ill give it a try

htmlmania commented 11 years ago

Oh @Mottie i added

div.anythingSlider { height: 546px !important; }

to stop it from going over 2000px tall.

htmlmania commented 11 years ago

No it didn't work at all i just made it worse.

Mottie commented 11 years ago

I'm not sure what was changed, but I still see both of those lines of css in the style.css file. Please don't use !important flags in the css unless absolutely necessary (why?).

So, Instead of setting the height with an important flag, it would be better to just set the max-height - this css does apply to ALL images on the page, so maybe it would be better to add a new entry targeting .slider img:

img { max-width: 100%; max-height: 1000px; }

and actually it would be better to define the slider height and width by targeting the slider itself and not the wrapper that is added by the plugin:

div.anythingSlider {}
ul.slider {
    width: 382px;
    height: 546px;
}

and lastly, you can set the navigationSize option to not show all 35 bullets at once. Also in the docs is a code snippet that autoscrolls that navigation size window to keep the current slide visible.