Skip to content

Commit

Permalink
Default keys for multi-text children (deephaven#293)
Browse files Browse the repository at this point in the history
  • Loading branch information
bmingles committed Mar 6, 2024
1 parent 3882a01 commit a709176
Showing 1 changed file with 21 additions and 9 deletions.
30 changes: 21 additions & 9 deletions packages/components/src/spectrum/picker/PickerItemContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,29 @@ export function PickerItemContent({
// Boolean values need to be stringified to render
content = String(content);
} else if (Array.isArray(content)) {
// For cases where there are multiple `Text` children, add css class to
// handle overflow
content = content.map(item =>
// For cases where there are multiple `Text` children, add a css class to
// handle overflow. The primary use case for multiple text nodes is when a
// description is provided for an item. e.g.
// <Item textValue="Some Text">
// <SomeIcon />
// <Text>Some Label</Text>
// <Text slot="description">Some Description</Text>
// </Item>
content = content.map((item, i) =>
isElementOfType(item, Text)
? // Cloning elements has the side effect of resetting React's internal
// `_store.validated` value to `false on each item. This causes them
// to be re-validated when they are rendered. Since they are inside of
// an array, React will show devtools warnings if they are missing `key`
// props.
cloneElement(item, {
? cloneElement(item, {
...item.props,
// `cloneElement` has the side effect of resetting React's internal
// `_store.validated` value to `false on the item. This causes it
// to be re-validated as a child in an array when is is rendered,
// even if the item was originally provided as an inline child.
// Since React expects array children to have explicit keys, this
// will show devtools warnings for items that wouldn't usually
// require explicit keys. Since we are only cloning `Text` nodes, it
// should be reasonable to fallback to a key matching the stringified
// content. The index suffix is an extra precation for when 2 <Text>
// nodes have the same value.
key: item.key ?? `${item.props.children}_${i}`,
UNSAFE_className: cl(
item.props.UNSAFE_className,
stylesCommon.spectrumEllipsis
Expand Down

0 comments on commit a709176

Please sign in to comment.