carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.75k stars 1.8k forks source link

[Bug]: Multiselect / FilterableMultiSelect selectionFeedback not working correctly with dynamic initialSelectedItems #12788

Open motherchucker opened 1 year ago

motherchucker commented 1 year ago

Package

carbon-components-react

Browser

Chrome

Package version

v7.52.0

React version

v17.0.2

Description

Configuration for MultiSelect:

<MultiSelect
  id='carbon-multiselect-example-2'
  titleText='Multiselect title'
  helperText='This is helper text'
  items={items}
  itemToString={(item) => (item ? item.text : '')}
  initialSelectedItems={selectedItems}
  selectionFeedback='top-after-reopen'
/>

items:

const items = [
  {
    id: 0,
    text: 'Test 1',
  },
  {
    id: 1,
    text: 'Test 2',
  },
  {
    id: 2,
    text: 'Test 3',
  },
  {
    id: 3,
    text: 'Test 4',
  },
];

selectedItems:

const selectedItems = [
  {
    id: 1,
    text: 'Test 2',
  },
  {
    id: 3,
    text: 'Test 4',
  },
];

Expected behaviour:

Initial selected items should be on the top when opening the selection.

Actual behaviour:

They stay on the same index. The items are checked, but they aren't on the top, ever:

image

If we pass initialSelectedItems like this, everything works ok, but that is not suitable for dynamic values:

image

Suggested Severity

Severity 2 = User cannot complete task, and/or no workaround within the user experience of a given component.

Reproduction/example

https://codesandbox.io/s/selectionfeedback-bug-dcq7gy

Steps to reproduce

  1. Create a multiselect component and pass dynamical selectedItems to initialSelectedItems
  2. Open multiselect dropdown

The examples can be tested out in the sandbox by commenting/uncommenting the provided initialSelectedItems examples:

image

Code of Conduct

tay1orjones commented 1 year ago

Thanks for the report! This might be some kind of referential equality issue present in internal/Selection.js, but I'm not totally sure.

I'm going to mark this as a severity 3 since the user still has the ability to view the selected items, and devs have a workaround by explicitly referring to items in the existing array as you demonstrate. Also I think you could use selectedItems to filter down items and get the matching indicies to support dynamic values as you describe.

TannerS commented 1 year ago

@tay1orjones can i give this a go?