-
Notifications
You must be signed in to change notification settings - Fork 47.1k
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
Set value using attribute only on initial option render #6449
Conversation
Tested that you can now type in the middle of a controlled input in Chrome which previously jumped your cursor to the end. Fixes facebook#6445.
How sure are you on this fixing the original issue? What's the summary of why this works? |
This basically reverts #6228. That broke cursor positioning because setting |
I did not like that other fix, I felt like it was janky and was going to lead to edge cases that would bite us. I similarly don't like this one. I'd feel much more comfortable if we went with #6406. We still could; it is rebased and ready-to-go. Anyway, accepting to unblock ( 👍 ) |
Set value using attribute only on initial option render (cherry picked from commit 2b1bd1d)
This traces back to #6449 and then another before that. I think that back then we favored the property over the attribute, and setting the property wouldn't be enough. However, the default path for these are now using attributes if we don't special case it. So we don't need it. The only difference is that we currently have a divergence for symbol/function behavior between controlled values that use the getToStringValue helpers which treat them as empty string, where as everywhere else they're treated as null/missing. Since this comes with a warning and is a weird error case, it's probably fine to change.
This traces back to #6449 and then another before that. I think that back then we favored the property over the attribute, and setting the property wouldn't be enough. However, the default path for these are now using attributes if we don't special case it. So we don't need it. The only difference is that we currently have a divergence for symbol/function behavior between controlled values that use the getToStringValue helpers which treat them as empty string, where as everywhere else they're treated as null/missing. Since this comes with a warning and is a weird error case, it's probably fine to change. DiffTrain build for [343a45f](343a45f)
This traces back to facebook/react#6449 and then another before that. I think that back then we favored the property over the attribute, and setting the property wouldn't be enough. However, the default path for these are now using attributes if we don't special case it. So we don't need it. The only difference is that we currently have a divergence for symbol/function behavior between controlled values that use the getToStringValue helpers which treat them as empty string, where as everywhere else they're treated as null/missing. Since this comes with a warning and is a weird error case, it's probably fine to change. DiffTrain build for [343a45ffa48065e60699bbe68f82d7b62fa02840](facebook/react@343a45f)
This traces back to facebook#6449 and then another before that. I think that back then we favored the property over the attribute, and setting the property wouldn't be enough. However, the default path for these are now using attributes if we don't special case it. So we don't need it. The only difference is that we currently have a divergence for symbol/function behavior between controlled values that use the getToStringValue helpers which treat them as empty string, where as everywhere else they're treated as null/missing. Since this comes with a warning and is a weird error case, it's probably fine to change.
Tested that you can now type in the middle of a controlled input in Chrome which previously jumped your cursor to the end.
Fixes #6445.