equinor / design-system

The Equinor design system
MIT License
119 stars 63 forks source link

Autocomplete: Duplicate entries in selectedItems for controlled Autocomplete #3259

Open tomktjemsland opened 7 months ago

tomktjemsland commented 7 months ago

Describe the bug

I am trying to replace an old custom drop-down solution with Autocomplete. The Autocomplete component is Controlled with multiple selections enabled. Options are a list of objects on the format: { id: 'Aker Barents', selected: true, title: 'Aker Barents' }. Enabling options works as intended, but when attempting to disable options selectedItems given onOptionsChange has duplicate entries instead!

Steps to reproduce the bug

  1. Create a controlled Autocomplete component with multiple enabled.
  2. Set the options property to a list of objects with a selected flag available.
  3. Filter objects on selected === true and use this as data for the selectedOptions property.
  4. Log selectedItems available in onOptionsChange.

Expected behavior

I expect objects to disappear from selectedItems when I try to disable them in the drop-down.

Specifications

Additional context

image

oddvernes commented 7 months ago

Thanks for the report! We will investigate this issue further

BirteThornquist commented 7 months ago

I understand you are already looking into this @yusijs

yusijs commented 7 months ago

I understand you are already looking into this @yusijs

Perhaps - if my thinking is not to optimistic. :)

oddvernes commented 4 months ago

A possible clue https://github.com/downshift-js/downshift/blob/master/src/hooks/useCombobox/README.md#itemtokey

oddvernes commented 3 months ago

This should be fixed by #3455, by using the new itemCompare prop. Please try it out in the new release (0.39.0) and let us know if it works!

mhwaage commented 3 weeks ago

The intended functionality is still broken in 0.41.0.

Using itemCompare causes the duplicate to disappear, but that is fixing the wrong issue, since it does not remove the de-selected item. e: and yet, I am not able to reproduce that when trying to make a unit test 😢

yusijs commented 3 weeks ago

The intended functionality is still broken in 0.41.0.

Just want to point to the title of my commit/pr 😅: Possible solution for object-checking

e: and yet, I am not able to reproduce that when trying to make a unit test 😢 I know the feeling, it was a pain to nail the behaviour down..

mhwaage commented 3 weeks ago

OK, I am able to reproduce this in a development situation, but not usre the best way of exposing it for a unit test. To expose it, select some items in an autocomplete, then trigger a hot-reload (which will keep the selected items unaltered, but seems to trigger some reference changes?)

The issue is most likely in the block

} else if (multiple) {
    selectedItems.includes(selectedItem)
    ? removeSelectedItem(selectedItem)
    : addSelectedItem(selectedItem)
    } else {

where the condition on removing/adding is placed on the reference equalitry instead of the itemCompare.

oddvernes commented 3 weeks ago

I have published a new release with the new fix included https://github.com/equinor/design-system/releases/tag/eds-core-react%400.41.3