From 89d0a9f52b08d24f840e751613838f4e1ef5208d Mon Sep 17 00:00:00 2001 From: Akshat Jawne Date: Thu, 27 Jun 2024 10:52:48 -0600 Subject: [PATCH 1/5] fix: make textValue default to key for Normalized Item --- packages/components/src/spectrum/utils/itemUtils.ts | 4 +--- .../src/spectrum/utils/useRenderNormalizedItem.tsx | 5 +++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index 49895787c..c8d58159d 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -137,9 +137,7 @@ export function getItemKey< */ export function getItemTextValue(item: ItemElement): string | undefined { if (item.props.textValue == null) { - return ['string', 'boolean', 'number'].includes(typeof item.props.children) - ? String(item.props.children) - : undefined; + return item.key == null ? ITEM_EMPTY_STRING_TEXT_VALUE : String(item.key); } return item.props.textValue === '' diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx index 74258085c..109f72214 100644 --- a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx +++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx @@ -8,8 +8,8 @@ import { ListActionMenu, ListActionMenuProps } from '../ListActionMenu'; import { Item } from '../shared'; import { getItemKey, - ItemIconSlot, ITEM_EMPTY_STRING_TEXT_VALUE, + ItemIconSlot, NormalizedItem, TooltipOptions, } from './itemUtils'; @@ -50,7 +50,8 @@ export function useRenderNormalizedItem({ (normalizedItem: NormalizedItem) => { const itemKey = getItemKey(normalizedItem); const content = wrapPrimitiveWithText(normalizedItem.item?.content); - const textValue = normalizedItem.item?.textValue ?? ''; + const textValue = + normalizedItem.item?.textValue ?? (normalizedItem.item?.key as string); const description = showItemDescriptions ? wrapPrimitiveWithText(normalizedItem.item?.description, 'description') From bc94b9d564eeebe4204770ce942ac5659e772cc0 Mon Sep 17 00:00:00 2001 From: Akshat Jawne Date: Thu, 27 Jun 2024 11:03:34 -0600 Subject: [PATCH 2/5] revert getItemTextValue logic --- packages/components/src/spectrum/utils/itemUtils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index c8d58159d..d5683c57c 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -137,7 +137,10 @@ export function getItemKey< */ export function getItemTextValue(item: ItemElement): string | undefined { if (item.props.textValue == null) { - return item.key == null ? ITEM_EMPTY_STRING_TEXT_VALUE : String(item.key); + const itemKey = item.key !== undefined ? String(item.key) : undefined; + return ['string', 'boolean', 'number'].includes(typeof item.props.children) + ? String(item.props.children) + : itemKey; } return item.props.textValue === '' From 74483cbb503cf65c8828d8ea2246f5e399b71167 Mon Sep 17 00:00:00 2001 From: Akshat Jawne Date: Thu, 27 Jun 2024 12:27:03 -0600 Subject: [PATCH 3/5] cleanup --- packages/components/src/spectrum/utils/itemUtils.test.tsx | 2 +- packages/components/src/spectrum/utils/itemUtils.ts | 4 ++-- .../src/spectrum/utils/useRenderNormalizedItem.test.tsx | 4 ++-- .../components/src/spectrum/utils/useRenderNormalizedItem.tsx | 3 ++- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/components/src/spectrum/utils/itemUtils.test.tsx b/packages/components/src/spectrum/utils/itemUtils.test.tsx index 51c6932a9..a09c8fc02 100644 --- a/packages/components/src/spectrum/utils/itemUtils.test.tsx +++ b/packages/components/src/spectrum/utils/itemUtils.test.tsx @@ -61,7 +61,7 @@ describe('getItemTextValue', () => { object , - undefined, + '', ], ])( 'should return the expected `textValue`: %s, %s', diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index d5683c57c..a0e15bb07 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -137,10 +137,10 @@ export function getItemKey< */ export function getItemTextValue(item: ItemElement): string | undefined { if (item.props.textValue == null) { - const itemKey = item.key !== undefined ? String(item.key) : undefined; + const itemKeyStr = item.key == null ? undefined : String(item.key); return ['string', 'boolean', 'number'].includes(typeof item.props.children) ? String(item.props.children) - : itemKey; + : itemKeyStr; } return item.props.textValue === '' diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx index c4a2c7b4c..cbeac3b47 100644 --- a/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx +++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx @@ -105,7 +105,7 @@ describe.each([ it.each([ [ { key: 'mock.key', textValue: undefined }, - 'Empty', + 'mock.key', 'wrapIcon(undefined, illustration)', 'wrapPrimitiveWithText(undefined, undefined)', 'wrapPrimitiveWithText(undefined, description)', @@ -115,7 +115,7 @@ describe.each([ key: 'mock.key', item: { content: 'mock.content', textValue: undefined }, }, - 'Empty', + 'mock.key', 'wrapIcon(undefined, illustration)', 'wrapPrimitiveWithText(mock.content, undefined)', 'wrapPrimitiveWithText(undefined, description)', diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx index 109f72214..1a14cc6fa 100644 --- a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx +++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx @@ -51,7 +51,8 @@ export function useRenderNormalizedItem({ const itemKey = getItemKey(normalizedItem); const content = wrapPrimitiveWithText(normalizedItem.item?.content); const textValue = - normalizedItem.item?.textValue ?? (normalizedItem.item?.key as string); + normalizedItem.item?.textValue ?? + (itemKey == null ? undefined : String(itemKey)); const description = showItemDescriptions ? wrapPrimitiveWithText(normalizedItem.item?.description, 'description') From 5c9f28222e4c119b6339be716e1666a5c22ff19c Mon Sep 17 00:00:00 2001 From: Akshat Jawne Date: Thu, 27 Jun 2024 15:12:20 -0600 Subject: [PATCH 4/5] increase coverage --- .../utils/useRenderNormalizedItem.test.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx index cbeac3b47..7ab514c68 100644 --- a/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx +++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx @@ -120,6 +120,16 @@ describe.each([ 'wrapPrimitiveWithText(mock.content, undefined)', 'wrapPrimitiveWithText(undefined, description)', ], + [ + { + key: 'mock.key', + item: { content: 'mock.content', textValue: '' }, + }, + 'Empty', + 'wrapIcon(undefined, illustration)', + 'wrapPrimitiveWithText(mock.content, undefined)', + 'wrapPrimitiveWithText(undefined, description)', + ], [ { key: 'mock.key', @@ -132,15 +142,15 @@ describe.each([ ], [ { - key: 'mock.key', + key: undefined, item: { - textValue: 'mock.textValue', + textValue: undefined, icon: 'mock.icon', content: 'mock.content', description: 'mock.description', }, }, - 'mock.textValue', + undefined, 'wrapIcon(mock.icon, illustration)', 'wrapPrimitiveWithText(mock.content, undefined)', 'wrapPrimitiveWithText(mock.description, description)', From 41fb5cbfe93d474f734cd33d2017a5852fb8dc5e Mon Sep 17 00:00:00 2001 From: Akshat Jawne Date: Thu, 27 Jun 2024 16:01:45 -0600 Subject: [PATCH 5/5] add additional test case --- .../utils/useRenderNormalizedItem.test.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx index 7ab514c68..18e7433af 100644 --- a/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx +++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.test.tsx @@ -140,6 +140,21 @@ describe.each([ 'wrapPrimitiveWithText(mock.content, undefined)', 'wrapPrimitiveWithText(undefined, description)', ], + [ + { + key: '', + item: { + textValue: undefined, + icon: 'mock.icon', + content: 'mock.content', + description: 'mock.description', + }, + }, + 'Empty', + 'wrapIcon(mock.icon, illustration)', + 'wrapPrimitiveWithText(mock.content, undefined)', + 'wrapPrimitiveWithText(mock.description, description)', + ], [ { key: undefined,