-
Couldn't load subscription status.
- Fork 196
Rewrite visibility sensor to hooks #161
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
base: master
Are you sure you want to change the base?
Conversation
pull latest changes
This PR fixes React `findDOMNode` deprecation warning. https://reactjs.org/docs/strict-mode.html#warning-about-deprecated-finddomnode-usage This makes sure the component is compatible with upcoming concurrent mode. It's a breaking change and requires a major release because it adds a node to the DOM, we could use `display: contents;` as suggested by React.
- added getRef so consumers can specify which element is used for calculations - necessary for absolutely positioned elements in a container - added deprecation notice for passing elements as children - added some new test cases - updated README and examples
|
https://github.com/roderickhsiao/react-in-viewport already has a hook-based API. Maybe just add a link to it in docs? |
|
Anyone know where this ended up? I was looking at this PR and the other one linking to and it seems like they're both sitting on the back burner. Would love to get rid of those deprecation warnings... :) |
|
Hi @josectello it's only missing the approval from @joshwnj and a couple missing tests. Would be really helpful if you have time to review the tests, I can ask Josh to review after and push for a new release. |
This PR:
findDOMNodedeprecation warning Fix findDOMNode deprecation warning #160useVisibilitySensorhook for more powerful integrations.16.8.x.Missing:
actcalls if gets approved.@joshwnj What do you think? It will be more powerful API than adding a custom way of passing ref up from VisibilitySensor.