Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React hooks #41

Closed
AndrewThian opened this issue Apr 6, 2019 · 11 comments
Closed

React hooks #41

AndrewThian opened this issue Apr 6, 2019 · 11 comments

Comments

@AndrewThian
Copy link

Loving the library man! Keep it up :D super intuitive and yet customizable.

Been using 1.1.0 with React hooks and it was great! Just updated to 1.2.3 and something broke. Lax is still firing the update on scroll change but for some reason, the listening element is not reflecting the change.

React version: 16.8.6;

Currently out and on a mobile device, but would gladly give more information if you need!

@alexfoxy
Copy link
Owner

alexfoxy commented Apr 6, 2019

Hey! Glad you're enjoying it :)

I didn't write the react hooks lib but I'll gladly take a look if you can provide an example!

@AndrewThian
Copy link
Author

Ahhh, hooks are a native react implementation! Lemme double check if it's also breaking using React classes and I'll report back ASAP :)

If not then, I'll leave an example here for how to replicate the scenario!

@alexfoxy
Copy link
Owner

How did u get on?

@AndrewThian
Copy link
Author

AndrewThian commented Apr 10, 2019

hey! so sorry, got caught up with work work. But here are some findings!

React version 16.8.6

Findings:

  1. Class implementation of lax.js works [obviously].
  2. If you define lax.setup in a class constructor on the root element, using hooks to add lax.addElement also works.
  3. However, if you use useEffect hook to initialize lax.setup it fails to update the subscribed element

I'm double checking on a code sandbox to see if it's consistent too! One moment

Edit:
Here's a react sandbox:
https://codesandbox.io/embed/nryzvz71wp

Basically, there are two types of setups:

  1. [Fails] using a function and react hooks useEffect to setup lax
  2. [Passes] using a class constructor and setup lax

I'll be diving deeper later but this is all I have for now!

@arthurdenner
Copy link
Contributor

arthurdenner commented Apr 10, 2019

@AndrewThian, if you move the lax.setup call to outside of the useEffect hook, it works as expected, but I'm not sure if that's the right solution because as you said, it worked properly with v1.1.0.

@alexfoxy, a pull request was opened on my library, use-lax, to move the call to outside of the hook, but I think the more problem is on lax itself.

@alexfoxy
Copy link
Owner

lax.setup() should be called before anything else as it sets it's elements equal to []. I debugged your example and useEffect() in bubble.js gets called before useEffect() in App.js.

You should only call lax.setup() in App constructor.

@arthurdenner
Copy link
Contributor

@alexfoxy, nothing changed in lax within 1.1.x and 1.2.x? Because it works properly with previous versions using lax.setup inside the hook (after the component mounts).

@arthurdenner
Copy link
Contributor

Check out this sandbox with lax.js@1.1.0 and then upgrade it to the 1.2.0.

@alexfoxy
Copy link
Owner

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.

@AndrewThian
Copy link
Author

ahhhhhh, totally my bad. Didn't read the README well enough! Thanks so much for explaining @alexfoxy really helped out a lot. You're right, I think just shifting the lax.setup outside of the useEffect hook would mean lax is initialized with the creation of the function just like the class constructor.

May I suggest a update in documentation for those using React hooks and migrating from 1.1.x to 1.2.x? :) Thanks again!

@arthurdenner
Copy link
Contributor

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.

I see, thanks for the explanation.

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

No branches or pull requests

3 participants