cisco-sbg-ui / magna-react

magna-react.vercel.app
5 stars 0 forks source link

`ref` passed to `ATextarea` is not assigned the underlying `textarea` #390

Open MikkCZ opened 1 year ago

MikkCZ commented 1 year ago

Describe the bug ATextarea component allows to be give a ref.

https://github.com/cisco-sbg-ui/magna-react/blob/09f029c7007677bac18200502682eba02ea0e62e/framework/components/ATextarea/ATextarea.js#L46

However this ref is made into combinedRef which is in turn passed to the AInputBase component.

https://github.com/cisco-sbg-ui/magna-react/blob/09f029c7007677bac18200502682eba02ea0e62e/framework/components/ATextarea/ATextarea.js#L154C11-L167

https://github.com/cisco-sbg-ui/magna-react/blob/09f029c7007677bac18200502682eba02ea0e62e/framework/components/ATextarea/ATextarea.js#L211-L215

To Reproduce Steps to reproduce the behavior:

  1. Create ATextarea with custom ref.
  2. Type anything into the rendered textarea.
  3. Try accessing .value on the current ref, it will be undefined.

Expected behavior The ref current value is the internal textarea and .value gives the current input value.

rwharrisjr commented 1 year ago

@MikkCZ Would adding inputRef and attaching that to the <textarea ...> add the functionality you expect? The forwarded ref going to the base component wrapper can be useful for some situations, and it would likely break things to change that. Adding an inputRef could pass to the textarea, and a similarly an update could be made to ATextInput that works the same way.

MikkCZ commented 1 year ago

Yes and no.

I encountered this issue when trying to pass ATextarea to react-mde's textAreaComponent, which creates it's own ref and passes it to the textarea component itself. It would still require a wrapping component, that would take the ref prop value received from react-mde and drill it to it's internal ATextarea's inputRef prop.

My current workaround (using a wrapping component) is to pass a callback to the ATextarea's ref prop, call querySelector("textarea") on it, and the result store to the ref from react-mde. You can see in the internal repository https://github.com/advthreat/incident-manager/commit/163245c01c7901356dcca3388bbf3740c45ddc89.

rwharrisjr commented 1 year ago

I'm not seeing a way to change the functionality of the current ref to account for that without breaking other uses. Your solution might be the best for now. In another repository I mimicked the css for the text area to match ATextarea, but that obviously doesn't support the validation states (and I assume those are needed in some of your use cases).

https://github.com/andrerpena/react-mde has a version 12 in the works that appears to be composable, and it provides a hook to grab a ref to set on the textarea element. In this case I think the inputRef would work. But this isn't an option until version 12 is available.