Skip to content

Commit

Permalink
fix(ComboBox): ensure unique keys (#4999)
Browse files Browse the repository at this point in the history 
This change swiches React `key`s of the items of `<Dropdown>` as well
as the one of `<ComboBox>` from their text content to provided keys in
data.

This is applying the corresponding logic of `<MultiSelect>`.
In this way duplicate item text won't yeild duplicate React keys.

Fixes #4969.
  • Loading branch information
@asudoh
asudoh authored Jan 11, 2020
1 parent c1b2bf7 commit b92e240
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 44 deletions.
37 changes: 20 additions & 17 deletions packages/react/src/components/ComboBox/ComboBox.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,23 +390,26 @@ export default class ComboBox extends React.Component {
{isOpen && (
<ListBox.Menu aria-label={ariaLabel} id={id}>
{this.filterItems(items, itemToString, inputValue).map(
(item, index) => (
<ListBox.MenuItem
key={itemToString(item)}
isActive={selectedItem === item}
isHighlighted={
highlightedIndex === index ||
(selectedItem && selectedItem.id === item.id) ||
false
}
{...getItemProps({ item, index })}>
{itemToElement ? (
<ItemToElement key={itemToString(item)} {...item} />
) : (
itemToString(item)
)}
</ListBox.MenuItem>
)
(item, index) => {
const itemProps = getItemProps({ item, index });
return (
<ListBox.MenuItem
key={itemProps.id}
isActive={selectedItem === item}
isHighlighted={
highlightedIndex === index ||
(selectedItem && selectedItem.id === item.id) ||
false
}
{...itemProps}>
{itemToElement ? (
<ItemToElement key={itemProps.id} {...item} />
) : (
itemToString(item)
)}
</ListBox.MenuItem>
);
}
)}
</ListBox.Menu>
)}
Expand Down
34 changes: 19 additions & 15 deletions packages/react/src/components/Dropdown/Dropdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ export default class Dropdown extends React.Component {
[`${prefix}--dropdown--inline`]: inline,
[`${prefix}--dropdown--disabled`]: disabled,
[`${prefix}--dropdown--light`]: light,
[`${prefix}--dropdown--${size}`]: size,
});
const titleClasses = cx(`${prefix}--label`, {
[`${prefix}--label--disabled`]: disabled,
Expand Down Expand Up @@ -294,21 +295,24 @@ export default class Dropdown extends React.Component {
</ListBox.Field>
{isOpen && (
<ListBox.Menu aria-labelledby={id} id={id}>
{items.map((item, index) => (
<ListBox.MenuItem
key={itemToString(item)}
isActive={selectedItem === item}
isHighlighted={
highlightedIndex === index || selectedItem === item
}
{...getItemProps({ item, index })}>
{itemToElement ? (
<ItemToElement key={itemToString(item)} {...item} />
) : (
itemToString(item)
)}
</ListBox.MenuItem>
))}
{items.map((item, index) => {
const itemProps = getItemProps({ item, index });
return (
<ListBox.MenuItem
key={itemProps.id}
isActive={selectedItem === item}
isHighlighted={
highlightedIndex === index || selectedItem === item
}
{...itemProps}>
{itemToElement ? (
<ItemToElement key={itemProps.id} {...item} />
) : (
itemToString(item)
)}
</ListBox.MenuItem>
);
})}
</ListBox.Menu>
)}
</ListBox>
Expand Down
24 changes: 12 additions & 12 deletions packages/react/src/components/Dropdown/__snapshots__/Dropdown-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ exports[`Dropdown should render custom item components 1`] = `
id="downshift-4-item-0"
isActive={false}
isHighlighted={false}
key="Item 0"
key="downshift-4-item-0"
onClick={[Function]}
onMouseDown={[Function]}
onMouseMove={[Function]}
Expand All @@ -370,7 +370,7 @@ exports[`Dropdown should render custom item components 1`] = `
>
<itemToElement
id="id-0"
key="Item 0"
key="downshift-4-item-0"
label="Item 0"
value={0}
>
Expand All @@ -387,7 +387,7 @@ exports[`Dropdown should render custom item components 1`] = `
id="downshift-4-item-1"
isActive={false}
isHighlighted={false}
key="Item 1"
key="downshift-4-item-1"
onClick={[Function]}
onMouseDown={[Function]}
onMouseMove={[Function]}
Expand All @@ -404,7 +404,7 @@ exports[`Dropdown should render custom item components 1`] = `
>
<itemToElement
id="id-1"
key="Item 1"
key="downshift-4-item-1"
label="Item 1"
value={1}
>
Expand All @@ -421,7 +421,7 @@ exports[`Dropdown should render custom item components 1`] = `
id="downshift-4-item-2"
isActive={false}
isHighlighted={false}
key="Item 2"
key="downshift-4-item-2"
onClick={[Function]}
onMouseDown={[Function]}
onMouseMove={[Function]}
Expand All @@ -438,7 +438,7 @@ exports[`Dropdown should render custom item components 1`] = `
>
<itemToElement
id="id-2"
key="Item 2"
key="downshift-4-item-2"
label="Item 2"
value={2}
>
Expand All @@ -455,7 +455,7 @@ exports[`Dropdown should render custom item components 1`] = `
id="downshift-4-item-3"
isActive={false}
isHighlighted={false}
key="Item 3"
key="downshift-4-item-3"
onClick={[Function]}
onMouseDown={[Function]}
onMouseMove={[Function]}
Expand All @@ -472,7 +472,7 @@ exports[`Dropdown should render custom item components 1`] = `
>
<itemToElement
id="id-3"
key="Item 3"
key="downshift-4-item-3"
label="Item 3"
value={3}
>
Expand All @@ -489,7 +489,7 @@ exports[`Dropdown should render custom item components 1`] = `
id="downshift-4-item-4"
isActive={false}
isHighlighted={false}
key="Item 4"
key="downshift-4-item-4"
onClick={[Function]}
onMouseDown={[Function]}
onMouseMove={[Function]}
Expand All @@ -506,7 +506,7 @@ exports[`Dropdown should render custom item components 1`] = `
>
<itemToElement
id="id-4"
key="Item 4"
key="downshift-4-item-4"
label="Item 4"
value={4}
>
Expand Down Expand Up @@ -685,7 +685,7 @@ exports[`Dropdown should render with strings as items 1`] = `
id="downshift-3-item-0"
isActive={false}
isHighlighted={false}
key="zar"
key="downshift-3-item-0"
onClick={[Function]}
onMouseDown={[Function]}
onMouseMove={[Function]}
Expand All @@ -708,7 +708,7 @@ exports[`Dropdown should render with strings as items 1`] = `
id="downshift-3-item-1"
isActive={false}
isHighlighted={false}
key="doz"
key="downshift-3-item-1"
onClick={[Function]}
onMouseDown={[Function]}
onMouseMove={[Function]}
Expand Down

0 comments on commit b92e240

Please sign in to comment.