AnythingSlider icon indicating copy to clipboard operation
AnythingSlider copied to clipboard

Heights, images, loading, etc

Open chriscoyier opened this issue 12 years ago • 11 comments

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

chriscoyier avatar Sep 16 '12 16:09 chriscoyier

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

I'll include it with the next update.

Mottie avatar Sep 17 '12 00:09 Mottie

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

Mottie avatar Sep 17 '12 00:09 Mottie

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

lhwparis avatar Sep 17 '12 07:09 lhwparis

@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 avatar Sep 17 '12 17:09 Mottie

Hopefully this issue has been fixed in v1.8.7.

Mottie avatar Dec 07 '12 22:12 Mottie

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.

htmlmania avatar Feb 03 '13 13:02 htmlmania

@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;
    }
    

Mottie avatar Feb 03 '13 23:02 Mottie

Thanks @Mottie ill give it a try

htmlmania avatar Feb 04 '13 06:02 htmlmania

Oh @Mottie i added

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

to stop it from going over 2000px tall.

htmlmania avatar Feb 04 '13 07:02 htmlmania

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

htmlmania avatar Feb 04 '13 07:02 htmlmania

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.

Mottie avatar Feb 04 '13 14:02 Mottie