diff --git a/modules/react/collection/lib/useOverflowListModel.tsx b/modules/react/collection/lib/useOverflowListModel.tsx index ba4a7ecb25..643d2221dd 100644 --- a/modules/react/collection/lib/useOverflowListModel.tsx +++ b/modules/react/collection/lib/useOverflowListModel.tsx @@ -25,9 +25,16 @@ export function getHiddenIds( let shouldAddGap = false; if (selectedIds !== 'all' && selectedIds.length) { - if (items.length) { - // If selectedIds[0] is not in items, use the first id from items - selectedKey = items.find(item => item.id === selectedIds[0]) ? selectedIds[0] : items[0].id; + const candidate = selectedIds[0]; + const fallbackId = items?.[0]?.id; + // Only pin own cached sizes so unmeasured ids and inherited names cannot yield NaN. + if (Object.prototype.hasOwnProperty.call(itemSizeCache, candidate)) { + selectedKey = candidate; + } else if ( + fallbackId !== undefined && + Object.prototype.hasOwnProperty.call(itemSizeCache, fallbackId) + ) { + selectedKey = fallbackId; } } diff --git a/modules/react/collection/spec/useOverflowModel.spec.tsx b/modules/react/collection/spec/useOverflowModel.spec.tsx index e90071b250..0cbe550faf 100644 --- a/modules/react/collection/spec/useOverflowModel.spec.tsx +++ b/modules/react/collection/spec/useOverflowModel.spec.tsx @@ -90,5 +90,50 @@ describe('useOverflowModel', () => { ).toEqual(hiddenIds); }); }); + + it('should keep the selected item visible when items use a custom id field instead of `id`', () => { + expect( + getHiddenIds(250, 0, overflowTargeSize, itemSizeCache, ['second'], [ + {index: 0, value: {contextId: 'first'}, textValue: 'First'}, + {index: 1, value: {contextId: 'second'}, textValue: 'Second'}, + {index: 2, value: {contextId: 'third'}, textValue: 'Third'}, + {index: 3, value: {contextId: 'fourth'}, textValue: 'Fourth'}, + ] as any) + ).toEqual(['first', 'third', 'fourth']); + }); + + const items = [ + {id: 'first', value: 'first', index: 0, textValue: 'first'}, + {id: 'second', value: 'second', index: 1, textValue: 'second'}, + {id: 'third', value: 'third', index: 2, textValue: 'third'}, + {id: 'fourth', value: 'fourth', index: 3, textValue: 'fourth'}, + ]; + + it('should not pin an unmeasured selected id when the first item is cached', () => { + expect( + getHiddenIds( + 250, + 0, + overflowTargeSize, + {first: 100, second: 150, third: 200}, + ['fourth'], + items + ) + ).toEqual(['second', 'third']); + }); + + it('should leave the selected item unpinned when neither the selected id nor the first item is cached', () => { + expect( + getHiddenIds(250, 0, overflowTargeSize, {second: 150, third: 200}, ['fourth'], items) + ).toEqual(['third']); + }); + + it('should ignore inherited prototype identifiers such as toString', () => { + expect(getHiddenIds(250, 0, overflowTargeSize, itemSizeCache, ['toString'], items)).toEqual([ + 'second', + 'third', + 'fourth', + ]); + }); }); }); diff --git a/modules/react/tabs/lib/useTabsModel.tsx b/modules/react/tabs/lib/useTabsModel.tsx index 5ec6318e93..7453338305 100644 --- a/modules/react/tabs/lib/useTabsModel.tsx +++ b/modules/react/tabs/lib/useTabsModel.tsx @@ -110,21 +110,26 @@ export const useTabsModel = createModelHook({ unregisterPanel: panels.events.unregisterItem, }; - const menu = useMenuModel( - useMenuModel.mergeConfig(config.menuConfig, { - id: `menu-${model.state.id}`, - items: overflowItems, - nonInteractiveIds: state.nonInteractiveIds.filter(key => !state.hiddenIds.includes(key)), - onSelect(data) { - menu.events.hide(); - events.select(data); - }, - onShow() { - // Always select the first item when the menu is opened - menu.events.goToFirst(); - }, - }) - ); + const mergedMenuConfig = useMenuModel.mergeConfig(config.menuConfig, { + id: `menu-${model.state.id}`, + items: overflowItems, + // `getId`/`getTextValue` aren't callbacks or guards, so `mergeConfig` would let these + // unconditionally clobber a `menuConfig.getId`/`getTextValue` override. Fall back to the + // top-level Tabs config only when the caller hasn't set one on `menuConfig` directly. + getId: config.menuConfig?.getId || getId, + getTextValue: config.menuConfig?.getTextValue || config.getTextValue, + nonInteractiveIds: state.nonInteractiveIds.filter(key => !state.hiddenIds.includes(key)), + onSelect(data) { + menu.events.hide(); + events.select(data); + }, + onShow() { + // Always select the first item when the menu is opened + menu.events.goToFirst(); + }, + }); + + const menu = useMenuModel(mergedMenuConfig); return { ...model, diff --git a/modules/react/tabs/stories/examples/OverflowTabs.tsx b/modules/react/tabs/stories/examples/OverflowTabs.tsx index 143872a5fd..f9d10dd728 100644 --- a/modules/react/tabs/stories/examples/OverflowTabs.tsx +++ b/modules/react/tabs/stories/examples/OverflowTabs.tsx @@ -7,23 +7,24 @@ import {px2rem} from '@workday/canvas-kit-styling'; import {system} from '@workday/canvas-tokens-web'; type MyTabItem = { - id: string; + contextId: string; text: React.ReactNode; contents: string; }; export const OverflowTabs = () => { const [items] = React.useState([ - {id: 'first', text: 'First Tab', contents: 'Contents of First Tab'}, - {id: 'second', text: 'Second Tab', contents: 'Contents of Second Tab'}, - {id: 'third', text: 'Third Tab', contents: 'Contents of Third Tab'}, - {id: 'fourth', text: 'Fourth Tab', contents: 'Contents of Fourth Tab'}, - {id: 'fifth', text: 'Fifth Tab', contents: 'Contents of Fifth Tab'}, - {id: 'sixth', text: 'Sixth Tab', contents: 'Contents of Sixth Tab'}, - {id: 'seventh', text: 'Seventh Tab', contents: 'Contents of Seventh Tab'}, + {contextId: 'first', text: 'First Tab', contents: 'Contents of First Tab'}, + {contextId: 'second', text: 'Second Tab', contents: 'Contents of Second Tab'}, + {contextId: 'third', text: 'Third Tab', contents: 'Contents of Third Tab'}, + {contextId: 'fourth', text: 'Fourth Tab', contents: 'Contents of Fourth Tab'}, + {contextId: 'fifth', text: 'Fifth Tab', contents: 'Contents of Fifth Tab'}, + {contextId: 'sixth', text: 'Sixth Tab', contents: 'Contents of Sixth Tab'}, + {contextId: 'seventh', text: 'Seventh Tab', contents: 'Contents of Seventh Tab'}, ]); const model = useTabsModel({ items, + getId: (item: MyTabItem) => item.contextId, }); const [containerWidth, setContainerWidth] = React.useState('100%'); return (