Skip to content
This repository has been archived by the owner on Apr 16, 2024. It is now read-only.

Move setup outside useEffect hook #1

Merged
merged 1 commit into from
Apr 10, 2019
Merged

Move setup outside useEffect hook #1

merged 1 commit into from
Apr 10, 2019

Conversation

Sirk
Copy link
Contributor

@Sirk Sirk commented Apr 8, 2019

The setup should be executed outside the useEffect method. Because the parent can in some cases be mounted after the child. In this case the lax.elements is reset in the setup function.

The setup should be executed outside the useEffect method. Because the parent can in some cases be mounted after the child. In this case the lax.elements is reset in the setup function.
@arthurdenner
Copy link
Owner

@Sirk, thanks for contributing.

Could you provide an example where the current way of setup is causing problems?

@Sirk
Copy link
Contributor Author

Sirk commented Apr 8, 2019

Hello @arthurdenner !

Will be difficult for me to provide an example at this time.

But to explain, in my case (a Gatsby app).

The didMount of the page (parent) component is executed after the child component.
So the lax.setup() inside the useLax() is executed after the lax.addElement from the useLaxElement() hook. And so it reset the added childs as the lax.setup method do in lax.js.

Will send you an example if I get some time soon !

@arthurdenner
Copy link
Owner

@Sirk, I think the problem may be on the new versions lax itself - checkout out this issue - so I'm holding the merge until make sure the problem is not on the lax library.

@arthurdenner
Copy link
Owner

arthurdenner commented Apr 10, 2019

@Sirk, quoting the author of lax.js:

It is because in previous versions lax would look for components with attribute names to populate elements e.g. data-lax-preset="fadeIn" so it was finding the bubble element when lax.setup() was called. In v1.2 you need to add class='lax' to any elements that you want to be automatically animated by lax, so you can fix your example with className="bubble lax", however it makes more sense to do what I said and call lax.setup() asap.

Your change was released in the version 2.0.2. 🎉

Thanks for the contribution once again!

@arthurdenner arthurdenner merged commit 2369d5a into arthurdenner:master Apr 10, 2019
@Sirk
Copy link
Contributor Author

Sirk commented Apr 10, 2019

Happy to hear ! It will fix the bug as the lax.setup() will be called before each initialization of child react componnents.

@arthurdenner
Copy link
Owner

@all-contributors please add @Sirk for code.

@allcontributors
Copy link
Contributor

@arthurdenner

I've put up a pull request to add @Sirk! 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants