Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

Forward ref to the underlying HTMLInputElement of the TextInput #1106

Closed
wants to merge 1 commit into from

Conversation

ob6160
Copy link
Contributor

@ob6160 ob6160 commented Oct 25, 2021

What is the purpose of this change?

Allow for a ref to be passed to the TextInput component so that a reference to the input element can be obtained.

Reason for the change

A planned usage of the TextInput component needs to listen on the blur and invalid events for a specific field so we can run validation code based on the built in HTML input validation.

Example of how it helps in our use case

Here we register two handlers, one to validate on the blur event, the other to act if an invalid event is fired on that specific field once validated.

carbon (3)

…Input component so that it can be accessed in an external application
@ob6160 ob6160 requested a review from a team as a code owner October 25, 2021 17:17
@github-actions github-actions bot added the Text input Changes to the text input component label Oct 25, 2021
@SiAdcock
Copy link
Contributor

SiAdcock commented Oct 26, 2021

Thanks @ob6160. I totally agree we should be forwarding refs in the way you've implemented here, for all components. I'm wary that the React docs recommend treating this as a breaking change as the behaviour of refs will be observably different. I'm not sure to what extent people are dependent on the current behaviour, so I'd need to do a bit of investigation.

Before I do, I wonder whether the behaviour you're trying to achieve here can be accomplished using Synthetic Events? It appears React supports onInvalid and onBlur. Is it possible to tap into these events rather than use refs in this case?

@ob6160
Copy link
Contributor Author

ob6160 commented Oct 26, 2021

Thanks @ob6160. I totally agree we should be forwarding refs in the way you've implemented here, for all components. I'm wary that the React docs recommend treating this as a breaking change as the behaviour of refs will be observably different. I'm not sure to what extent people are dependent on the current behaviour, so I'd need to do a bit of investigation.

Before I do, I wonder whether the behaviour you're trying to achieve here can be accomplished using Synthetic Events? It appears React supports onInvalid and onBlur. Is it possible to tap into these events rather than use refs in this case?

Ah that's a really good point about it being a potentially breaking change! Okay, I'll await the results of your investigation. I'd be happy to take some time to make the PR to add ref forwarding for all components if it's all fine!

Will look into synthetic events.. it's the first I've heard of them (under that name)!

@ob6160
Copy link
Contributor Author

ob6160 commented Oct 26, 2021

Aha! I hadn't heard of "Synthetic Events" as the name for the pattern on<EventName> before. TIL! I can totally use those for my use case. I'll close this PR and we can continue the discussion in guardian/csnx#261 as you suggested

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Text input Changes to the text input component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants