yaodingyd / react-flickity-component

A React.js component for using @desandro's Flickity
314 stars 51 forks source link

fix #75: `imageloaded` to properly wait on image to resize carousel #80

Closed yaodingyd closed 4 years ago

yaodingyd commented 5 years ago

The issue is because before the imagedloaded code is in cDM life cycle, where the element carousel used for watch for imageloaded actually doesn't have any images inside, because we append children in renderPortal function, which happens after cDM after a state change.

The fix is to move this part of logic in renderPortal after carousel element actually contains images so imageloaded can function properly. To trigger renderPortal we need a state change so I add another state flickityCreated, and set it to true when we create flickity.

When static is set we don't use portal to render, but still we need to change flickityReady state.

Currently version works in Chrome larger breakpoints is somewhat lucky. I tried in small breakpoints and it doesn't work either. My wild guess is in these large breakpoints Chrome is doing some optimization on image loading so it actually work

theolampert commented 5 years ago

@yaodingyd I can't get this to work locally, can you try with the example here https://github.com/theolampert/react-flickity-component-example ?

if you run something like: npm link && npm run prepublishOnly -- --watch on your branch

I'll add a dev script in another PR soon

yaodingyd commented 5 years ago

@theolampert updated. Could you try with both static set and unset?

theolampert commented 5 years ago

@yaodingyd I get an infinite render loop now, I think theres an issue in how we're using state to determine if the carousel has been created. Maybe it's better to use a static variable to set a flickityCreated state? I will take a closer look.

yaodingyd commented 4 years ago

I think I fix the infinite render. I added a check in setReady to only trigger when it's not. The previous infinite render is because it keeps setting ready.

The setReady is in a setTimeout call because createPortal is an async operation and DOM will be updated later, and I need to make sure when imagesloaded is called, children is rendered in DOM.

I added a third state for dynamically adding/removing slides. Since setReady would only be called when it's not, we need a way to set ready to false. I'm comparing number of cells for now.

yaodingyd commented 4 years ago

@theolampert can you take a look?

theolampert commented 4 years ago

@yaodingyd sorry haven't had a chance to propery test this. Will take a look tonight.