-
Notifications
You must be signed in to change notification settings - Fork 4.2k
added clearInputOnBlur property to clear input value onBlur #4207
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
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset is good to goLatest commit: a2b9843 We got this. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit a2b9843:
|
001ffdd to
a2b9843
Compare
If you had read the comments on #3242 you would have seen a mention of a solution in #3189 (comment) (by yours truly). But for the lazy: const KeepInputOnBlurSelect = ({...props}) => {
const [inputValue, setInputValue] = React.useState('');
const handleInputChange = React.useCallback((value, meta) => {
if ([ 'input-blur', 'menu-close' ].indexOf(meta.action) === -1) {
setInputValue(value);
return value;
}
return inputValue;
}, [inputValue]);
return <Select {...props} inputValue={inputValue} onInputChange={handleInputChange} />;
} |
|
I would not merge this. The solution Rall3n posted isn't a workaround - it's a solution. The idea is that we use the existing callbacks to adjust the value at the form level (as opposed to on a per input basis). The inverse (merging this) would also open the door to allowing such props as: |
|
I agree with all points with one caveat. This seems absolutely essential to consider adding I agree that it can be straightforward enough to set up, but to be honest, it is a lot of boilerplate code to override what is already a somewhat controversial behavior, that I believe most people would prefer to easily opt out without the need of having to create controlled inputs simply to make an input required. On its own, I agree, this is the encouraged solution. But looking at it as a necessary prerequisite for every user supporting validation, I think that it deserves consideration. Then again, I could be missing an easier implementation for text validation, so open to feedback. |
|
@Rall3n workaround/fix works flawlessly... and I am kind of ok with adding some local state to control/observe input actions BUT: if you have a searchable input ala advanced/experimental usage in docs mobile devices such as iOS automatically open the keyboard which in return cannot be hidden without bluring the input and thus clearing it (which is the default behaviour). The user experience becomes horrible if you want to search for a term and afterwards hide the keyboard and swipe too your desired option. I understand the argument of bloating the API, but from the end user's point of view having a prop for the mentioned problem is probably a necessity. |
|
This was recently mentioned by Jed as something for consideration. To add context, the reason why the default behavior was set to clear the input was because the Menu will otherwise filter out all the options except for the currently selected (or defaulted) value. For instance, when viewing the first example on the documentation home page, opening the menu would only show the option "Ocean" which is confusing for a user to open a dropdown and only see one option. I'm not sure there is a right approach for this, but given that there are some very common use cases, it merits more discussion. |
New property
clearInputOnBlurfor clearing input value when a blur event is triggered. Defaults to true.this PR is an up-to-date version of #3242, as it was never reviewed and now has a couple of merge conflicts.
I understand the property was removed a couple of years ago, but there is still no solution for this specific problem.
Also, I renamed the original
onBlurResetsInputtoclearInputOnBlurfor more clarity, as properties starting with "on" should be reserved for function handlers in React.