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

Add ref support to Box #53

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

DannyDelott
Copy link

Closes #52

This PR adds ref support to Box using the forwardRef API provided in React 16.

According to the docs, this should be treated as a breaking change:

When you start using forwardRef in a component library, you should treat it as a breaking change and release a new major version of your library.

https://reactjs.org/docs/forwarding-refs.html#note-for-component-library-maintainers

@robinweser
Copy link
Owner

Alright, thanks for your work! Given that we‘re going to have a major release, shouldn‘t we add it to all components? Would love a PR update :)
Not actively using the repo atm, so I have no time to do so

@DannyDelott
Copy link
Author

Thanks for the quick response! I'm happy to convert the other components.

Do you have thoughts on how to convert ScrollView? Specifically, how we should expose its instance methods, ie: getScrollPosition(), scrollTo(), scrollToStart(), and scrollToEnd()? Previously this was done with a ref on the React element, not the DOM node.

@robinweser
Copy link
Owner

Heyhey. Sorry for not getting back for so long. Not sure if we'd want to have that for ScrollView as well then, since it's not a functional component after all.
If you're still up to add that PR feel free to though :)

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

Successfully merging this pull request may close these issues.

Add support for React refs on Box
2 participants