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

Forward ref to inputComponent and use that instead of requiring inputComponent to implement .focus() #281

Closed
TylerRick opened this issue Sep 25, 2019 · 1 comment

Comments

@TylerRick
Copy link

TylerRick commented Sep 25, 2019

I'm glad the need to implement .focus() is at least documented, but I am wondering if that requirement is even necessary.

Couldn't we just forward the ref to whichever inputComponent is provided and just call ref.current.focus() on that ref (since it's a ref to an input, which already has a focus() method) instead of specifically requiring it to provide its own focus() method?

Maybe that's just trading one requirement for another, but it seems like it might be a slightly more elegant solution, and a safer assumption to make since it seems like any input component ought to be forwarding refs anyway.

Then you would only need an adapter component (like this one) if the intended input component had a different name for the ref that gets forwarded all the way down to the input (it's inputRef in the case of material-ui).

The other reason for this change is because the current requirement that the component implements .focus() seems to require that component to be a class component. I'm trying to use hooks and functional components wherever possible and trying to figure out how a functional component (which is basically just the render() from a class component) can also implement focus(). I don't think it's possible.

See also: #280, #226

@catamphetamine
Copy link
Owner

@TylerRick

Couldn't we just forward the ref to whichever inputComponent is provided and just call

Yeah, I've only recently discovered how this whole ref forwarding works and it's surely a better way now that React is deprecating class components. I've updated the README to reflect that forwarding a ref is valid.

Maybe that's just trading one requirement for another, but it seems like it might be a slightly more elegant solution,

A much more elegant one.

and a safer assumption to make since it seems like any input component ought to be forwarding refs anyway.

It should, it's just backwards compatibility stuff.
Pre-hooks era, etc.

I'm trying to use hooks and functional components wherever possible and trying to figure out how a functional component (which is basically just the render() from a class component) can also implement focus(). I don't think it's possible.

Hooks should work provided ref is forwarded.

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

2 participants