[useCombobox] Cursor jumps to end of input when using controlled value

[leoweigand]

• downshift version: 5.45
• node version: 13.12
• npm (or yarn) version: 6.14.4

Relevant code or config

  const [value, setValue] = useState("");
  const { getInputProps } = useCombobox({
    items: [],
    inputValue: value,
    onInputValueChange: ({ inputValue }) => {
      setValue(inputValue);
    }
  });

  return <input {...getInputProps()} />;

What you did:
I am passing a value into inputValue and update it using onInputValueChange.

What happened:
The cursor always jumps to the end of the input.

Reproduction repository:
I created a minimal example here: https://codesandbox.io/s/usecombobox-controlled-input-issue-vj8o6?file=/src/index.js

Problem description:
It seems that there are two consecutive re-renders happening on keydown, the first one with the old value (resetting the input to it’s state before the keypress), and a second render with the new value, moving the cursor to the end. I saw that there was a similar issue in #217, but maybe unrelated?

Suggested solution:
I also created a similar example without using downshift here. It seems, that we either need any keypress to only trigger one re-render (ideal) or make sure the first one contains the up-to date input value.

[silviuaavram]

Hi! It may be related, found this in downshift.js:

    // we want to call `onInputValueChange` before the `setState` call
    // so someone controlling the `inputValue` state gets notified of
    // the input change as soon as possible. This avoids issues with
    // preserving the cursor position.
    // See https://github.com/downshift-js/downshift/issues/217 for more info.
    if (!isStateToSetFunction && stateToSet.hasOwnProperty('inputValue')) {
      this.props.onInputValueChange(stateToSet.inputValue, {
        ...this.getStateAndHelpers(),
        ...stateToSet,
      })
    }

We may need something similar here as well. i probably will hate it, since I tried to create a unified state reducer - on change callers for all state props, and this will be different. Can you submit a fix please?

[fabb]

Simple fix on the client side:

const [value, setValue] = useState("");
  const { getInputProps } = useCombobox({
    items: [],
    inputValue: value
  });

  return <input {...getInputProps({
                onChange: e => {
                  setValue(e.target.value)
                }
  })} />;             

It is not possible to call setValue inside onInputValueChange with the current way downshift is calling that asynchronously. This is too late for React since the input value first will be reset to the previous value, and only after that set to the new value, which causes the curser position to be reset. It also causes issues with deadkeys.

[silviuaavram]

You can also avoid any downshift hack and control the cursor position yourself. Just have a state value with the cursor position value, which you get from the input onChange, and make sure it keeps its value.

It's unfortunate to have such hacks, but calling onInputValueChange or onStateChange before setting the state is also a hack.

[bhollis]

@fabb I'm doing this and it helps me work around the issue and also avoid a double render, but it's a bummer that I now have to reimplement a lot of Downshift's input value handling.

[ericArbour]

@fabb's manual controlling of the input worked for me as well, with the minor correction that setInputValue should be setValue for future readers.

[alexblack]

I'm hitting this issue too, I was expecting to be able to use useCombobox with a controlled inputValue, but the cursor jumps to the end when the user tries to edit an existing input value in the middle

(Using it as a controlled input is working well for me, following the suggestions above, eg handle changes to the input value yourself, and simply tell useCombox box what the current value is using props)

[silviuaavram]

It is unfortunate to have this bug. I could not figure out a way to fix it, we have something implemented in the Downshift component but I don't think it works with the hooks. If someone can find a way to fix it, we will be very happy!

[alexblack]

It is unfortunate to have this bug. I could not figure out a way to fix it, we have something implemented in the Downshift component but I don't think it works with the hooks. If someone can find a way to fix it, we will be very happy!

I believe I got it working with useCombobox:

1. Don't use their event handler onInputValueChange
2. Instead handle the value change from your <input/> yourself
3. Also don't use setInputValue, instead tell it the value through props, eg useCombobox({ inputValue })

[dlong500]

@silviuaavram Not understanding why this is closed, as there are various hacks mentioned but no actual solution. I do really appreciate all the hard work that has gone into the downshift components but I'll admit that I'm getting a bit overwhelmed with the number of hacks I've had to introduce to workaround various issues and to get desired behavior. I'm getting worried that my own implementation based on useCombobox is going to be a bit fragile especially because every new workaround has the potential to introduce side-effects in all the other workarounds.

[silviuaavram]

@dlong500 what do you mean by the overwhelming number of hacks? I think our api allows consumers to tweak downshift to their desired behaviour while still sticking to the API. And, looking at the Downshift workaround for this specific issue, it's also a hack.

Anyway, I will re-open the bug, and if I could figure a way to fix it, I will. But I prefer to review an already proposed solution since my actual time to write code for the library is limited. Thanks!

[dlong500]

I didn't mean to imply an "overwhelming" number of hacks but rather that I've been getting overwhelmed a bit. I'll have to take a step back to review everything before I can come up with an actual list of issues, but there have been a number of things that were either confusing or just didn't work as expected. A lot of it comes down it little weird things like pressing tab, having inputBlur fire, but then having focus remain on the input when setting the state to close the menu (which I worked around).

I'll be honest that the combination of the stateReducer, downshift-specific events, using built-in events on the input, and needing a controlled component has been a bit confusing. It seems that there are a variety of ways to accomplish the same thing, but then I've run into odd issues in several places. Having multiple ways of doing things and ultimate flexibility is great! There have just been a few "quirks" that have had me re-implement things in different ways until I could figure out something that works (and sometimes while trying to fix one problem another quirk would appear).

I'm not beyond admitting that some of the quirks may be my own faulty code, though several of the issues (like this one) are clearly faced by a bunch of others. I'll try to put together a codesandbox at some point to highlight some of my issues and the workarounds. Again, I appreciate all of your hard work and don't mean to vent at you specifically. It's been a rough couple days...

[silviuaavram]

Due to the state setup we have for the hooks (React.useReducer) there's no good way to fix this (at least none I can think of). The only time we can know the new state is after already triggering dispatch.

Yes, we can call onInputValueChange before dispatch but if the user has a state reducer function that alters the state on the way, our arguments in onInputValueChange can differ from the actual new state.

I also don't want to call the handler inside the reducer functions, as they should be kept pure.

Best I can do at the moment is update the documentation for the onInputValueChange prop and propose the workaround with onChange if it's used to update an inputValue state variable which will control the hook's state.

If anyone has any better idea, I'm all years. In the meantime, I will create a PR to update the documentation. Thank you!