Downshift: inputValue is set to empty string on select of already selected item

[klis87]

• downshift version: 2.0.0 +
• node version: 9.11.1
• npm (or yarn) version: yarn 1.7

Relevant code or config
See updated Custom select with downshift example - https://codesandbox.io/s/18ll7293vq
I updated version of Downshift to 2.0.0 and made Downshift using controlled selectedItem prop.

What you did:
I tried to use Downshift for a custom select component according to examples.

What happened:
Everything works, apart from one thing - if you have an item selected, and then you open dropdown again and you pick the same item, for some reason inputValue is set to empty string. However, if you select a different option, then inputValue is updated correctly to itemToString of a newly selected option

Reproduction repository:
https://codesandbox.io/s/18ll7293vq

Problem description:
Problem described above started to happen since version 2.0.0, for v1.31.16 it works as expected - you can downgrade codesandbox to v1.31.16 and it will work correctly.

[kentcdodds]

Yeah this is a bug. Could you do a little digging and see what we can do to fix it? Thanks!

[klis87]

@kentcdodds Unfortunately I dont have too much time at the moment to really dig into it, but I went through all the code and this https://github.com/paypal/downshift/blob/master/src/downshift.js#L272 is really suspicious to me.

In below snippet

inputValue: this.isControlledProp('selectedItem')
          ? this.props.defaultInputValue
          : this.props.itemToString(item),

I am not sure why inputValue update depends on whether selectedItem is controlled prop or not, in my opinion onSelect inputValue should always be set to this.props.itemToString(item).

Also, I am not sure why but inputValue update is duplicated in case a selected item has changed in https://github.com/paypal/downshift/blob/master/src/downshift.js#L979 , is it done on purpose? Anyway, this probably explains why this bug only happens when selected item is the same.

[kentcdodds]

Oh right. This is happening because of #243.

I think what's going on is this. When an item is selected, the downshift state needs to be "reset" (menu closed, highlighted index set to its default value, etc). Originally we set the inputValue to the itemToString of the item, but #243 caused a change in that behavior under the assumption that a componentDidUpdate will cause the inputValue to change anyway. In this case a componentDidUpdate doesn't fire and so that doesn't happen.

Here's a workaround (that I don't love) which fixes the issue by controlling the inputValue and ignores inputValue updates when an item is clicked: https://codesandbox.io/s/74j7jk2y9j

I don't have time to work on issues that don't directly impact me (or my work at PayPal) I'm afraid. If we're going to improve situations, then we need to do it in a way that doesn't break the use case in #243. I'm definitely willing to hear ideas.

[klis87]

Thanks for the explanation and workaround example.

I will think about possible solution without breaking #243 , maybe over the weekend. I will keep you posted.

[kentcdodds]

Thanks! :D

[klis87]

@kentcdodds I spent quite big amount of time on this today, and really I cannot find a way to fix this not to break #243 . Actually I think that #243 solution is kinda hacky and won't work always anyway, because in:

inputValue: this.isControlledProp('selectedItem')
          ? this.props.defaultInputValue
          : this.props.itemToString(item),

the assumption is that selectedItem was never changed since mounting the component, which is not necessary true - then resetting it to this.props.defaultInputValue won't be correct anymore.

I really admire this library and how flexible it is, and that it allows to use mixture of controlled and not controlled props, depending on requirement. But this flexibility has a costs and this is one of them, it is too magical in this case.

So, the only thing that comes to my mind is to revert to inputValue: this.props.itemToString(item) and to use controlled inputValue like you initially suggested in #243 or to use an alternative solution in argument 2).

The arguments I have:
1) see updated official redux example to latest Downshift - https://codesandbox.io/s/vqx63mo6x3 You can see this is one of the most basic examples and it is broken when you select already selected item
2) #243 case seems to me really rare and using controlled inputValue is quite ok there I think. Also, even without controlling it is previous behaviour really an issue? Someone typed apple and it is still there. And if it really is, the alternative solution would be just to provide key={items.length} - see it in action - https://codesandbox.io/s/xom626kkmp - yeeah I know, maybe it is not super optimal to destroy and mount component this way, but this is only 1 input, not a big deal :)
3) Your workaround for my case unfortunately doesnt work. I needed to add changes.type !== Downshift.stateChangeTypes.keyDownEnter case but it is still broken. It would work without autocomplete, but for autocomplete there is problem with cursor - if you select input at the beginning and write sth or click delete, cursor will always move to the end of input - see https://codesandbox.io/s/qqwlw21xk9

[kentcdodds]

I think I agree with you and I'd like to consider a 3.0.0 to change this behavior. This will require a bit of coordination with one or two other changes I'd like to make. Hopefully I can dedicate some time soon to make these changes in the next few weeks.

Until then, you could probably make a form of the project and ship that knowing that you'd be able to upgrade without much problem when this is dealt with.

[alexandernanberg]

Wouldn't a quick and dirty fix be to just compare the current selectedItem and item? This works when I test locally and all tests are passing as well. Could a possible solution be to add this fix to v2 and refactor it in v3?

  selectItem = (item, otherStateToSet, cb) => {
    otherStateToSet = pickState(otherStateToSet)

    const {selectedItem} = this.getState()
    const inputValue =
      selectedItem !== item && this.isControlledProp('selectedItem')
        ? this.props.defaultInputValue
        : this.props.itemToString(item)

    this.internalSetState(
      {
        isOpen: false,
        highlightedIndex: this.props.defaultHighlightedIndex,
        selectedItem: item,
        inputValue,
        ...otherStateToSet,
      },
      cb,
    )
  }

[klis87]

@kentcdodds Thanks! Probably I will subclass Downshift class to override selectItem for now (I know, subclassing in React?! but it should work :))

@alexandernanberg Interesting idea :+1:

[kentcdodds]

I'd rather just fix it properly. I'm hoping to be able to do that this week.

[klis87]

If anyone would like to have it know, I recommend doing:

import BrokenDownshift from 'downshift';

const stateKeys = [
  'highlightedIndex',
  'inputValue',
  'isOpen',
  'selectedItem',
  'type',
];

const pickState = (state = {}) => {
  const result = {};
  stateKeys.forEach(k => {
    if (state.hasOwnProperty(k)) {
      result[k] = state[k];
    }
  });
  return result;
};

class Downshift extends BrokenDownshift {
  selectItem = (item, otherStateToSet, cb) => {
    otherStateToSet = pickState(otherStateToSet);
    this.internalSetState(
      {
        isOpen: false,
        highlightedIndex: this.props.defaultHighlightedIndex,
        selectedItem: item,
        inputValue: this.props.itemToString(item),
        ...otherStateToSet,
      },
      cb,
    );
  };
}

[dronsfield]

Spent hours earlier this month trying to debug this thinking I was doing something wrong / going crazy. Thanks for the workaround @klis87, works perfectly

[franklixuefei]

In my case, when I first clicked on the selected item, the input was not flushed. But for subsequent clicks, the input value would always become empty. Is this also the case for you guys?

[manuelgerstner]

Yes, the initial select works and the subsequent ones don't (for the already selected item).

[IvanCoronado]

I have the same problem, unfortunately, @klis87 solution doesn't work with TS (or I am not able to make it work).

Anyway, We can live with this bug by now because we are not still in production.

[creatyvtype]

What's the status of this? Currently looking at this bug as it causes problems for options derived from input via async search. Making a selection triggers empty onInputValueChange which causes options to return empty.

[kentcdodds]

Sorry, I'll try to get to it as soon as I'm able. It's something I really want to get right and therefore need to consider carefully.

[Kevin-K]

Walked into the same issue.

Was able to get around it by forcing the inputValue in the state reducer if the selectedItem doesn't change by calling my itemToString implementation with the current selectedItem.

const itemToString = (item) => ((item && item.label) ? item.label : '');

downshiftStateReducer = (state, changes) => {
      switch (changes.type) {
        case Downshift.stateChangeTypes.keyDownEnter:
        case Downshift.stateChangeTypes.clickItem:

          return {
            ...changes,
            inputValue: (state.selectedItem === changes.selectedItem) ?
              itemToString(state.selectedItem): changes.inputValue,
          };
        default:
          return changes;
      }
    }

[trepidacious]

I've also run into this issue and spent a few hours trying to work out what I was doing wrong ;)

If this can't be fixed soon, would it make sense to at least add a property to select the old (in my opinion correct) behaviour (as described by @klis87 on Jul 21)? I've tried using props.breakingChanges.resetInputOnSelection as found here https://github.com/paypal/downshift/commit/831aeaba329dee811cbf09e88252d31b5ee15773 but this doesn't seem to help. Or perhaps an optional prop to control behaviour on selection - this could default to the old behaviour of setting the inputValue to the selection, and have another behaviour to clear it as in #243?

At the moment it seems like a very common use case (having a controlled selectedItem) is broken by a complicated and unusual one (#243). I may be atypical, but to me using a controlled selectedItem is actually the most common use case, assuming you don't want to have state scattered around in Downshift components.

On the subject of #243 (I think this might have been said before) it seems like there is a very simple solution for that case - just use sensible keys. I think the problem with https://codesandbox.io/s/k5px1z0075 is really just that the keys used are not valid (I'm sure this is just to produce a simple example) :

1. On line 64, the key for the Autocomplete array is the selected value - try adding multiple "apple" entries - you end up with a data list full of apples but only two autocompletes displayed (see image later).
2. The following Autocomplete on line 65 is in the same array as the Autocompletes produced by the map, but has no key, which is invalid.

These can both be fixed by using a sensible key - as @klis87 remarked above you can use the index within the data array for the mapped Autocompletes, then length of data for the final Autocomplete - again a better key would probably be available in a real application. This fixes both the problem when adding multiple copies of the same fruit, and the "problem" with the new fruit autocomplete not being reset. When a new data item is added to the array, it will keep the same Autocomplete that was used to add it (which to me is completely correct), so this isn't even less efficient - we create one new Autocomplete when a new item is added, and that's it.

Edit: Forgot the image!

Edit: #243 example fixed with index-based keys: https://codesandbox.io/s/8z4znxz2kl

[kentcdodds]

Thanks @trepidacious. I'm planning on getting this worked on and maybe even released today!

[trepidacious]

That's great, by the way thanks for the library - I'm working on a facade for using material-ui in scalajs and downshift looked like by far the easiest way to get autoselect working.

[kentcdodds]

Dang, something came up and I had to stop my progress. Hopefully soon though!

[binarybaba]

Stumbled upon the same issue while doing async search. Subscribing!

[RaulTsc]

Any updates on this? :D

[joshdcuneo]

One workaround that feels ok to me is to close the dropdown if the entered
value matches one in the dropdown. It feels like fairly natural auto
complete behaviour and prevents the user selecting the matching one.

On Wed., 26 Sep. 2018, 5:31 pm Raul Tomescu, [e-mail]
wrote:

Any updates on this? :D

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/paypal/downshift/issues/512#issuecomment-424614868,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AjUuU5IJUDExEGS4YSlWwUl301Xe8KqKks5uey1JgaJpZM4VTfho
.

[binarybaba]

I don't think that's a very good way to go about it. This is more of a core
issue. Let's hope this gets closed soon because downshift has a great
support for asyc loading of items. You will find using it some day or the
other.
The workaround i resorted to was storing the selected item in my
localstorage and then making another component which uses that. Then I made
a service to crud to the localstorage.

On Wed, Sep 26, 2018, 15:29 Josh Cuneo [e-mail] wrote:

One workaround that feels ok to me is to close the dropdown if the entered
value matches one in the dropdown. It feels like fairly natural auto
complete behaviour and prevents the user selecting the matching one.

On Wed., 26 Sep. 2018, 5:31 pm Raul Tomescu, [e-mail]
wrote:

Any updates on this? :D

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/paypal/downshift/issues/512#issuecomment-424614868,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/AjUuU5IJUDExEGS4YSlWwUl301Xe8KqKks5uey1JgaJpZM4VTfho

.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/paypal/downshift/issues/512#issuecomment-424657743,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGFff1J9h9zRPg5zGdg1MpApoaQ7vkrvks5ue0_7gaJpZM4VTfho
.

[joshdcuneo]

Certainly depends on the use case.

On Wed, Sep 26, 2018 at 9:38 PM Amin Mohamed Ajani [e-mail]
wrote:

I don't think that's a very good way to go about it. This is more of a core
issue. Let's hope this gets closed soon because downshift has a great
support for asyc loading of items. You will find using it some day or the
other.
The workaround i resorted to was storing the selected item in my
localstorage and then making another component which uses that. Then I made
a service to crud to the localstorage.

On Wed, Sep 26, 2018, 15:29 Josh Cuneo [e-mail] wrote:

One workaround that feels ok to me is to close the dropdown if the
entered
value matches one in the dropdown. It feels like fairly natural auto
complete behaviour and prevents the user selecting the matching one.

On Wed., 26 Sep. 2018, 5:31 pm Raul Tomescu, [e-mail]
wrote:

Any updates on this? :D

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<https://github.com/paypal/downshift/issues/512#issuecomment-424614868
,
or mute the thread
<

https://github.com/notifications/unsubscribe-auth/AjUuU5IJUDExEGS4YSlWwUl301Xe8KqKks5uey1JgaJpZM4VTfho
>

.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/paypal/downshift/issues/512#issuecomment-424657743,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/AGFff1J9h9zRPg5zGdg1MpApoaQ7vkrvks5ue0_7gaJpZM4VTfho

.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/paypal/downshift/issues/512#issuecomment-424683423,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AjUuU51ATGdjNbk6sM7uKCBh3gki4i6Eks5ue2cdgaJpZM4VTfho
.

[franklixuefei]

@kentcdodds Any updates or plans on this? 😄

[kentcdodds]

Ok, I'm for real working on this today.

[RaulTsc]

:)

[kentcdodds]

Alrighty, this one's resolved in the next branch.

See #591 to keep up with what's left for 3.0.0

[kentcdodds]

:tada: This issue has been resolved in version 3.0.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[RaulTsc]

Nice Kent! Thank you

On Mon, 8 Oct 2018 at 23:29, Kent C. Dodds [e-mail] wrote:

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release
  https://github.com/paypal/downshift/releases/tag/v3.0.0

Your semantic-release
https://github.com/semantic-release/semantic-release bot 📦🚀

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/paypal/downshift/issues/512#issuecomment-427985198,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AM6wibLuDl41xpl0jqjZYTkmUJbsCtymks5ui8O1gaJpZM4VTfho
.

[trepidacious]

Thanks 👍 :)