Cursor jumps to end of input when inputValue is a controlled prop
robin-drexler opened this issue ยท 25 comments
downshift
version: 1.11.0node
version: 6.9 (and codesadbox.io, could not find out, what they're using)npm
(oryarn
) version: yarn 1.1.0 (and whatever codesandbox.io uses)
Problem description:
- Visit this codesandbox
- type text
- move cursor to the left
- type a letter
- the letter will be added at the correct position, but the cursor will move to the end of the input instead of staying where you have typed
Relevant code or config
class Demo extends React.Component {
state = {
inputValue: ""
};
render() {
return (
<div>
<Downshift
onStateChange={data => {
return (
data.hasOwnProperty("inputValue") &&
this.setState({ inputValue: data.inputValue })
);
}}
inputValue={this.state.inputValue}
>
{({ getInputProps }) => (
<div>
<input {...getInputProps()} />
</div>
)}
</Downshift>
</div>
);
}
}
Thanks for the perfect reproduction issue!
I'm not sure why this is happening. It'll take a little digging. I'll try to get to it when I get the chance, but if you could give it a whirl that'd be great :)
It seems @kentcdodds posted before I could share this ๐
As I try to debug this, it seems that the onStateChange
update lags behind the native update of the caret/cursor. I used the function from this Stack Overflow answer to track the caret position in the input
's onInput
event handler, as well as in the Downshift wrapper's onStateChange
event handler.
GIF of my test:
This looks like the setState
call replaces the value of the text field after typing has finished, essentially overwriting what's in the field, and therefore moving the caret to the end of the text.
To be honest I'm not sure if Kent had intended for the input being "watched" by Downshift to be used as a controlled component in this way.
My solution
I made a test branch for this issue that seems to provide a solution to this issue.
There's a short description in the README, but basically I took that input and made it a class; the "controlled" part is contained within that input's class, while Downshift can still grab the input and use it internally.
Hmmm... Interesting. I would expect that even when setState
is called, react would make sure that cursor position would be preserved.... ๐ค
I'd love to solve this in a way that folks controlling the inputValue
don't need to worry about it...
Also, thanks so much for a thorough dig @geoffdavis92! That's really helpful!
@kentcdodds happy to help.
From my testing, it seems this is the default behavior of any change of an HTML input
's value
attribute, as demonstrated with this Pen.
It smells like: https://stackoverflow.com/questions/28922275/in-reactjs-why-does-setstate-behave-differently-when-called-synchronously
The issue also vanishes if one moves the this.props.onStateChange
call above the setState
call in internalSetState. Just for testing of course, as quite some other things break in that case.
I tried to reproduce the same behavior in a smaller example without Downshift, but wasn't able yet.
Is there anything async I'm missing?
I've noticed the debounce, but removing it also does not seem to do the trick.
Reproduced without downshift: https://codesandbox.io/s/036r6xkr7w
The issue seems to be that upon setState
children are re-rendered.
When this happens, the value
property of the underlying input field dom node already has changed to the new value, while the result of getStateAndHelpers
still contains the old value from the prop. Only after this.props.onStateChange
is called, the prop also has the new value.
Since react already has rendered the input with a new value, it now has to touch/change the value again, leading to the cursor moving to the end.
A possible fix that came to mind could be to put the controlled props into the state of Downshift
and sync them in componentWillReceiveProps
instead of reading them from props later on. It could also be done for inputValue
only and the rest kept as is.
I'd be open to look at a PR for that, but I'd reeeeally like to avoid it if there's any other way to do it...
Alternatively, here's an easy solution: https://codesandbox.io/s/pj5n3xx07
Will that work?
Also thought about something like this, however, we currently use:
if (data.hasOwnProperty('inputValue')) {
this.props.onChange(data.inputValue);
}
in onStateChange
to call a passed in onChange
hander. This works for the input's onChange
, but also when the user has selected something from the list. The input's onChange
does not trigger in that case.
But I think this could be fixed by using the input's onChange
in addition to onStateChange
and by adding an additional check for the presence of selectedItem
there to avoid invoking the passed in onChange
twice.
Feel free to open up a pull request and we can talk about it. But know that I probably wont accept a PR that attempts to synchronize state with props. I'm sure we can do this another way. Maybe an onInputValueChange
prop?
Is this usable when using "multi selection"?
I'm asking this because i've tried to remove my onChange
handler of input
and use onInputValueChange
to get updates on the inputValue changes.
The issue is that once an item is selected and added to the array of the controlled selectedItem
prop, it triggers onInputValueChange
with the whole contents of the array basically (here).
I can try and filter the updates i receive in onInputValueChange
to only actually input changes but dont think its the right way to go.
This function also triggers for me when i'm selecting an item (when isOpen
changes to false) but i might be missing something on my side on this..?
Hi! I'm afraid I don't have too much time right now, but I made this last night and it might help you: https://github.com/kentcdodds/advanced-downshift
Notice that I just set the selectedItem to null and instead of controlling the input value, I use reset to clear the input.
WOW!! That looks awesome! Going to have to change some code on my side ๐
A lot of code in the render functions but still nice :) Does it have some performance impact when you do all those render inside render dynamic stuff?
In any case I understand your point and its a very nice approach ๐
Does it have some performance impact when you do all those render inside render dynamic stuff?
Maybe? But not enough to be concerned about.
Good luck!
Hi @kentcdodds
Sorry, but I haven't figured out how you fixed the cursor jumping problem in your advance downshift demo without using onInputValueChange()? And also, if I want to pass in default selected recipients, how would you do that? I tried defaultSelectedItem
prop but it doesn't work in your demo...
Thanks!
I used the codesandbox from advanced downshift and did not have the cursor jumping issue:
Thanks @austintackaberry but I still don't know how this was achieved without onInputValueChange().
@kentcdodds
Sorry, but I haven't figured out how you fixed the cursor jumping problem in your advance downshift demo without using onInputValueChange()? And also, if I want to pass in default selected recipients, how would you do that? I tried defaultSelectedItem prop but it doesn't work in your demo...
Thanks!
NVM, I figured it out.
- @kentcdodds 's demo is working because in his demo
inputValue
is not a controlled (by us) prop, so that the cursor problem will not happen at all. - In his demo,
selectedItem
is assignednull
so that this API is disabled, thusdefaultSelectedItem
is not working either, as expected.
If I understand correctly, the defaultXXX
props will only be useful when the corresponding control props are uncontrolled. Please correct me if I'm wrong @kentcdodds
Thanks!
Below is a demo that has inputValue
and defaultSelectedItem
as control props. The defaultSelectedItem
prop works as expected and there is no jumping cursor issue.
https://codesandbox.io/s/5konk5137n
Let me know if this isn't what you're looking for! ๐
@austintackaberry Thanks! This is what I'm looking for. I think you can also use the onInputValueChange()
prop which is already provided. But either works.
If you are using a value from the redux store in the textfield and updating the value of the redux store onChange. The problem is observed from react-redux version 6.0.1 and higher. Change your react-redux version to 6.0.0 and the cursor jumping problem should be solved.
I also had this problem, with the useCombobox hook.
The only solution I found so far is to let combobox controls the input state, and forward this state to parent components in useEffect.
useEffect(() => {
if (combobox.inputValue !== value) {
onChange(combobox.inputValue);
}
}, [combobox.inputValue, value]);
That sounds like a workaround for an unrelated bug @slorber. Could you open a new issue for that to be tracked?
I am still running into this problem when:
inputValue
is controlled from outside (useState
)onStateChange
is set in theuseCombobox
call
I'm encountering this on v6.1.7
and on v7.1.12
.
@slorber's workaround fixes the issue.
@john-rodewald and anybody else who is running into the same issue with hooks
this worked for us and doesn't require useEffect
: #1108 (comment)