segmentio / evergreen

🌲 Evergreen React UI Framework by Segment
https://evergreen.segment.com
MIT License
12.39k stars 832 forks source link

Forward ref to select in Select control #1434

Closed EmanueleCiriachi closed 2 years ago

EmanueleCiriachi commented 2 years ago

I am trying to use ref with the Select control, however I'm getting the

Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

error message. Is there any way that this control will work the same way as TextInputField, that redirects refs to the <input>?

brandongregoryscott commented 2 years ago

Looks like we're passing the ref to the wrapping <Box /> instead of the <Box /> that's actually rendering a <select />.

https://github.com/segmentio/evergreen/blob/ec0e180dc18871c8d6873de7f2a1a65fe1b28b3b/src/select/src/Select.js#L75-L87

https://github.com/segmentio/evergreen/blob/ec0e180dc18871c8d6873de7f2a1a65fe1b28b3b/src/text-input/src/TextInput.js#L56-L66

Should be simple to swap - I don't see any real reason to have it on the wrapping box!

brandongregoryscott commented 2 years ago

Should be resolved in v6.9.6!

jonbeebe commented 2 years ago

@brandongregoryscott What about the SelectField component? I'm still seeing the same error with that component in 6.9.6

brandongregoryscott commented 2 years ago

@brandongregoryscott What about the SelectField component? I'm still seeing the same error with that component in 6.9.6

Ah, good catch - looks like that component isn't even forwarding the ref right now. https://github.com/segmentio/evergreen/blob/890de4300a3d9a820eca6c4f2c8d045cf13bf23d/src/select/src/SelectField.js#L8

Additionally, I think I missed the TypeScript definition updates which still list them as div BoxComponents (should be select instead) https://github.com/segmentio/evergreen/blob/890de4300a3d9a820eca6c4f2c8d045cf13bf23d/index.d.ts#L2246-L2251

jonbeebe commented 2 years ago

@brandongregoryscott great! Thanks for the quick response

brandongregoryscott commented 2 years ago

Forgot to circle back here, but this should be resolved as of v6.9.8+

jonbeebe commented 2 years ago

@brandongregoryscott Yes, it is working as expected for me in 6.9.8. Thanks!