SelectPanel should compare Objects based on key or id instead of the Objects themselves

[ajhenry]

Description

When passing in objects to the SelectPanel component, the selected items may not render correctly since they are doing direct object comparison:

react/packages/react/src/SelectPanel/SelectPanel.tsx

Lines 135 to 155 in ea44386

	onAction: (itemFromAction, event) => {
	item.onAction?.(itemFromAction, event)
	if (event.defaultPrevented) {
	return
	}
	if (isMultiSelectVariant(selected)) {
	const otherSelectedItems = selected.filter(selectedItem => selectedItem !== item)
	const newSelectedItems = selected.includes(item) ? otherSelectedItems : [...otherSelectedItems, item]
	const multiSelectOnChange = onSelectedChange as SelectPanelMultiSelection['onSelectedChange']
	multiSelectOnChange(newSelectedItems)
	return
	}
	// single select
	const singleSelectOnChange = onSelectedChange as SelectPanelSingleSelection['onSelectedChange']
	singleSelectOnChange(item === selected ? undefined : item)
	onClose('selection')
	},

If the items are defined in the component, they could rerender and create new object references. Then this comparison check will always return false as the original objects are held in useState.

Instead, we should rely on comparing the key or id props as these are guaranteed* to be unique per object.

So our comparison function would instead look something like the following:

onAction: (itemFromAction, event) => {
          item.onAction?.(itemFromAction, event)

          if (event.defaultPrevented) {
            return
          }

          if (isMultiSelectVariant(selected)) {
            const otherSelectedItems = selected.filter(selectedItem => selectedItem.key !== item.key)
            const newSelectedItems = selected.find(selectedItem => selectedItem.key === item.key) ? otherSelectedItems : [...otherSelectedItems, item]

            const multiSelectOnChange = onSelectedChange as SelectPanelMultiSelection['onSelectedChange']
            multiSelectOnChange(newSelectedItems)
            return
          }

          // single select
          const singleSelectOnChange = onSelectedChange as SelectPanelSingleSelection['onSelectedChange']
          singleSelectOnChange(item.key === selected.key ? undefined : item)
          onClose('selection')
        },

cc: @ipc103

Steps to reproduce

Minimal Repo
https://codesandbox.io/p/sandbox/dreamy-cache-msfqyq

Version

v35.26.0

Browser

Chrome

[ajhenry]

@ipc103 and I are happy to take this on 😄

[ipc103]

Still WIP, but we started working on a fix here: ipc103@f472da2 Next steps:

• Refactor the duplicated logic
• Add a couple of additional test cases
• Maybe support other properties besides key as a fallback?

[github-actions]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.