metafizzy / flickity

:leaves: Touch, responsive, flickable carousels
https://flickity.metafizzy.co
7.53k stars 602 forks source link

Height not resizing properly with imagesLoaded and Firefox/IE #205

Closed infn8 closed 5 years ago

infn8 commented 9 years ago

I have a reduced test case here http://codepen.io/infn8/pen/pJGJjJ It is forked from this pen.

Steps to reproduce the issue:

  1. Load the full page in Mac Firefox http://codepen.io/infn8/full/pJGJjJ
  2. Perform a Hard Refresh of the page (CMD + SHIFT + R) in order to force the browser to re-download the large images
  3. Observe how the slider height does not change until a window re-size occurs

I suggest making your browser window thin to begin with.

Running latest OSX and Latest Firefox

desandro commented 9 years ago

Thanks for reporting this issue and for providing a reduced test case :grinning:

I'm not sure if I see an issue.

You may be noticing how slider is not resized until the window has completed resizing. This is a performance feature. Instead of resizing every millisecond and costing your users multiple unnecessary reflows, Flickity resizes the slider after the window has been resized. There is a slight delay, 150 ms, but it keeps your page feeling snappy.

infn8 commented 9 years ago

The issue here is that it STAYS un-resized. so the first time a user visits the page there is a massive amount of negative space under the slider. I think your approach is great for performance and works well in the other browsers it appears. but it's not working here. Perhaps there is an event i could hook into for the creation of the slider the first time so i can trigger a resize manually?

infn8 commented 9 years ago

I've made a screen cast to explain the issue and why I believe it to be an issue.

https://www.youtube.com/watch?v=yJDkY4z1Riw

desandro commented 9 years ago

Ah, Thanks for that. So the issue is: sometimes, in Firefox, the slider is not resized to fit the images. I've been able to reproduce this issue, but it doesn't happen every time. Taking a look at it, the problem lies somewhere with imagesLoaded. The bug occurs when it is not triggered on every cell. This may be related to desandro/imagesloaded#191.

You may want to use lazyLoad as an alternative solution to imagesLoaded for now.

I hope to resolve the issue with imagesLoaded, which will resolve this issue.

infn8 commented 9 years ago

Thanks David. I'll give lazyload a try. I'll also subscribe to bug 191 on images loaded.

p-drolima commented 8 years ago

Not sure if my issue is related but after I read the whole thread I believe it is. It just looked like Flickity was firing before images and cells where being loaded. I had a go with $(window).load rather than document.ready and all is fine now.

I know its been a few months but hopefully this might help somebody else.

desandro commented 8 years ago

Using $(window).load does address this issue, but generally I recommend against it as $(window).load waits for all assets and contents to be loaded before firing: images, video, even iframes. Only use this solution as a last resort.

p-drolima commented 8 years ago

yeh you are correct of course. do you advise any other solution, because the imageloaded and lazyload didnt work for me at all?

Thank and keep up the excellent work ;)

slimndap commented 8 years ago

Adding this to the end of my script solved it for me:

jQuery(window).load( function() {
    window.dispatchEvent(new Event('resize'));
});
p-drolima commented 8 years ago

I would recommend changing the title as this seems not be browser specific. On my case, it works fine on Firefox but not on Chrome.

markseuffert commented 8 years ago

I am also running into this problem. I tried different Flickity configurations. Without imagesLoaded:true the slider is empty when opening a page, with imagesLoaded:true it works most of the times but has an incorrect height after a page refresh, percentPosition:false didn't have any effect as far as I could see. Happens on Firefox or Chrome.

Is there a recommended workaround with vanilla JS?

4aficiona2 commented 8 years ago

I also experience this issue with the latest Chrome version. Neither with imagesLoaded: true, nor lazyLoad: true, nor both worked for me in general. There were always pageloades where the slider was not correctly initialized. As soon as I resized the window like mentioned above the slider had the correct size / images got displayed.

I now use @slimndap snippet which works for me and fires the Resize event. If the images are initially there the slider gets initialized correctly, if not the slider gets its proper size after the Resize event gets triggered.

desandro commented 8 years ago

@slimndap not a bad hack.

Rather than triggering a window resize event, you can also call Flickity's resize method on window load.

// jQuery
$( window ).on( 'load', function() {
  $carousel.flickity('resize');
});
// vanilla JS
window.addEventListener( 'load', function() {
  flkty.resize();
});
benjibee commented 8 years ago

I'm having this same issue in the latest Firefox, Chrome and Safari. It works better on page load, but I have a fullscreen gallery (thumbnails below) that launches and needs to be resized.

If I simply call flkty.resize(); directly after instantiating Flickity (which is done after the fullscreen is shown) as is suggested here it doesn't work. This is using imagesLoaded: true. I actually call it a second time and it works, but I think that's just because of the 150ms delay.

You can also see the overlapping images issue seen with wrapAround: true ;)

davidhellmann commented 7 years ago

Hm I've the same problem. Image Overlapping and huge margin bottom.


        window.addEventListener( 'load', function() {
            flkty.resize()
        });

does not help. Hm

solemone commented 7 years ago

I have the same problem. Chrome 57 Mac. It seems to me that the error only occurs when Flickity is loaded from the disk cache but the images are not. In addition: I initialize in HTML. Perhaps this is important for error detection.

The onload hack works for me. However, I needed a solution for several Flickity instances that were initiated in HTML. I solved it this way:

window.addEventListener('load', function() {
  var elements = document.getElementsByClassName('js-flickity');
  for (var i = 0; i < elements.length; i++) {
    var flkty = Flickity.data(elements[i]);
    flkty.resize();
  }
});
davidhellmann commented 7 years ago

Thanks @solemone use it so but the it's the same

// Load Resize Hack
window.addEventListener('load',() => {
    const sliders = Array.from(document.querySelectorAll('.js-imageSlider'))
    if (sliders) {
        sliders.forEach((slider) => {
            var flkty = Flickity.data(slider)
            flkty.resize()
        })
    }
})
ghost commented 7 years ago

This issue happens because when using data attributes Flickity is initialized on 'DOMContentLoaded'. At the time some images still might be not loaded. Correct way would be to initialize Flickity on 'load' event. Images will be loaded and Flickity slides will have correct size without the need to call resize. @desandro , is there any way option to use load event instead of DOMContentLoaded could be added to Flickity? Or is it possible to do manual initialization with data attributes? I'd like to have configuration options on HTML side.

desandro commented 7 years ago

@vaidotas-adf Thanks for reporting this issue. Could you open a new issue and provide a reduced test case? See Submitting Issues in the contributing guidelines.

daltonrooney commented 6 years ago

Is this still a problem, even when using lazyload? I've got a basic test case here which is not resizing on load in Chrome 69 or Firefox 62 on Mac.

desandro commented 5 years ago

I'm closing this issue as its grown old. Please open a new issue if you run into resizing issues with imagesLoaded or lazyLoad

@daltonrooney Looks like your issue is caused by the placeholder data URI being in a square resolution. Flickity is reading that size instead. That looks to be a different edge case.