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

Cannot programmatically focus the textarea input in the Textarea component #470

Closed
danparnella opened this issue Feb 11, 2021 · 7 comments · Fixed by #472
Closed

Cannot programmatically focus the textarea input in the Textarea component #470

danparnella opened this issue Feb 11, 2021 · 7 comments · Fixed by #472
Assignees

Comments

@danparnella
Copy link
Contributor

Without using DOM traversal with standard Javascript, I haven't been able to find a way to programmatically focus the textarea input in the Textarea component. My suggestion is to allow for a forwardRef prop on Textarea that would be passed down to the textarea input. That ref could then be used to target the input itself for focusing.

@danparnella danparnella self-assigned this Feb 11, 2021
@chawes13
Copy link
Contributor

I ran into this issue yesterday with our base Input component.

Can we use React.forwardRef? Here's a reference from Styled Components: https://github.com/system-ui/theme-ui/pull/817/files

Ideally this would be applied to all of our inputs, but I'm ok with that being a separate PR to expedite the fix for Artlook

@chawes13
Copy link
Contributor

chawes13 commented Feb 12, 2021

That being said, we will probably run into an issue with how redux-forms Field component wraps our components. From the documentation, it looks like their ref forwarding implementation via getRenderedComponent only works if the component is a class component.

I initially thought we might be able to get away with something like the below, but haven't tested it. I did see one SO post that said it wasn't working for them.

<Field
  name="content"
  component={(props) => <Input ref={inputRef} {...props} />}
/>

This approach may have some downsides when it comes to react dev tools and component display names. It's also not very dev friendly. Another workaround could be to also accept a forwardedRef prop (since forwardRef is reserved by redux-form) and pass that into our input. Something like,

export default React.forwardRef(function Input ({ forwardedRef, ...inputProps }, ref) {
  return (
    <input ref={forwardedRef || ref} {...inputProps} />
  )
})

@danparnella
Copy link
Contributor Author

Yeah, that was originally was my hope (to just export our component with forwardRef), but yes I ran straight into that class restriction ☹️ . Your last suggestion is basically what I ended up doing in my band-aid override, but without the fallback to using the correct forwardRef implementation (and I think I could use forwardRef in my Field in artlook because we're using an older version of redux-form), so I think what you have is the way to go.

I plan on getting a PR up for this today!

@danparnella
Copy link
Contributor Author

@chawes13 I'm running into some bumps in the road represented by this issue. Does that make sense in that I need to define prop types and default props after exporting Textarea as a forwardRef?

@chawes13
Copy link
Contributor

chawes13 commented Feb 12, 2021

@danparnella Did you try the solution outlined in this comment? facebook/react#16653 (comment)

I think you would want to do something like

const Input = React.forwardRef(function Input (props, ref) { return <input ref={ref} {...props} /> })
Input.propTypes = propTypes
Input.defaultProps = defaultProps

export default Input

@danparnella
Copy link
Contributor Author

Got it, got it. Ok, just wanted to make sure I didn't go too off the rails with our patterns 👍 .

@chawes13
Copy link
Contributor

Uncharted territory 🤪

@danparnella danparnella linked a pull request Feb 12, 2021 that will close this issue
5 tasks
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 a pull request may close this issue.

2 participants