-
Notifications
You must be signed in to change notification settings - Fork 87
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
fix: text input mask fixes #2581
fix: text input mask fixes #2581
Conversation
Thanks for the PR @nicholasguyett. I'll be taking a look at this and we will be incorporating in our next release. I'm currently looking through how to test this locally and appreciate your summary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 🌈
I was able to test and build the project locally. Also, based on the How To Test section this looks like was already tested in a private repository.
useEffect(() => { | ||
// Make sure this component behaves correctly when used as a controlled component | ||
setValue( | ||
maskString( | ||
((externalValue ?? defaultValue) as string) ?? ``, | ||
mask, | ||
charset | ||
) | ||
) | ||
}, [externalValue]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on this and using useEffect
API hook.
// Ensure the new value is available to upstream onChange listeners | ||
e.target.value = newValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch here on ensuring other onChange
listeners get the value here too.
@all-contributors please add @nicholasguyett for bug, code |
We had trouble processing your request. Please try again later. |
Summary
Fixes the incorrect pass-through of the
inputRef
propAdds support for
defaultValue
Adds support for using
TextInputMask
as a controlled componentFixes support for externally set
onChange
hooksNote: The
maskString
function contains the unmodified original masking implementation, just moved into a separate function so it can be called in bothonChange
and when setting the initial state.Related Issues or PRs
How To Test
Manually tested this change in an external (currently private) application that uses both
defaultValue
andonChange
.Screenshots (optional)