chore: ListView - refactor Picker utils to be more generic (#1910)

Renamed "picker" utils + modules to more generic "item" names for re-use
with other components.

packages/code-studio/src/styleguide/Pickers.tsx

@@ -1,13 +1,14 @@
 import React, { useCallback, useState } from 'react';
 import {
   Flex,
+  Item,
   Picker,
-  PickerItemKey,
+  ItemKey,
   Section,
   Text,
 } from '@deephaven/components';
 import { vsPerson } from '@deephaven/icons';
-import { Icon, Item } from '@adobe/react-spectrum';
+import { Icon } from '@adobe/react-spectrum';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { sampleSectionIdAndClasses } from './utils';
 
@@ -28,9 +29,9 @@ function PersonIcon(): JSX.Element {
 }
 
 export function Pickers(): JSX.Element {
-  const [selectedKey, setSelectedKey] = useState<PickerItemKey>();
+  const [selectedKey, setSelectedKey] = useState<ItemKey>();
 
-  const onChange = useCallback((key: PickerItemKey): void => {
+  const onChange = useCallback((key: ItemKey): void => {
     setSelectedKey(key);
   }, []);
 

packages/components/src/index.ts

@@ -49,7 +49,6 @@ export * from './SelectValueList';
 export * from './shortcuts';
 export { default as SocketedButton } from './SocketedButton';
 export * from './spectrum';
-export * from './spectrum/utils';
 export * from './TableViewEmptyState';
 export * from './TextWithTooltip';
 export * from './theme';

packages/components/src/spectrum/picker/Picker.tsx

@@ -1,6 +1,6 @@
 import { Key, ReactNode, useCallback, useMemo } from 'react';
 import { DOMRef } from '@react-types/shared';
-import { Flex, Picker as SpectrumPicker, Text } from '@adobe/react-spectrum';
+import { Flex, Picker as SpectrumPicker } from '@adobe/react-spectrum';
 import {
   getPositionOfSelectedItem,
   findSpectrumPickerScrollArea,
@@ -15,30 +15,28 @@ import {
 import cl from 'classnames';
 import { Tooltip } from '../../popper';
 import {
-  isNormalizedPickerSection,
+  isNormalizedSection,
   NormalizedSpectrumPickerProps,
-  normalizePickerItemList,
+  normalizeItemList,
   normalizeTooltipOptions,
-  NormalizedPickerItem,
-  PickerItemOrSection,
+  NormalizedItem,
+  ItemOrSection,
   TooltipOptions,
-  PickerItemKey,
-  getPickerItemKey,
-} from './PickerUtils';
+  ItemKey,
+  getItemKey,
+} from '../utils/itemUtils';
 import { PickerItemContent } from './PickerItemContent';
 import { Item, Section } from '../shared';
+import { Text } from '../Text';
 
 export type PickerProps = {
-  children:
-    | PickerItemOrSection
-    | PickerItemOrSection[]
-    | NormalizedPickerItem[];
+  children: ItemOrSection | ItemOrSection[] | NormalizedItem[];
   /** Can be set to true or a TooltipOptions to enable item tooltips */
   tooltip?: boolean | TooltipOptions;
   /** The currently selected key in the collection (controlled). */
-  selectedKey?: PickerItemKey | null;
+  selectedKey?: ItemKey | null;
   /** The initial selected key in the collection (uncontrolled). */
-  defaultSelectedKey?: PickerItemKey;
+  defaultSelectedKey?: ItemKey;
   /** Function to retrieve initial scroll position when opening the picker */
   getInitialScrollPosition?: () => Promise<number | null>;
   /**
@@ -47,7 +45,7 @@ export type PickerProps = {
    * `onSelectionChange`. We are renaming for better consistency with other
    * components.
    */
-  onChange?: (key: PickerItemKey) => void;
+  onChange?: (key: ItemKey) => void;
 
   /** Handler that is called when the picker is scrolled. */
   onScroll?: (event: Event) => void;
@@ -56,7 +54,7 @@ export type PickerProps = {
    * Handler that is called when the selection changes.
    * @deprecated Use `onChange` instead
    */
-  onSelectionChange?: (key: PickerItemKey) => void;
+  onSelectionChange?: (key: ItemKey) => void;
 } /*
  * Support remaining SpectrumPickerProps.
  * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
@@ -113,7 +111,7 @@ export function Picker({
   ...spectrumPickerProps
 }: PickerProps): JSX.Element {
   const normalizedItems = useMemo(
-    () => normalizePickerItemList(children),
+    () => normalizeItemList(children),
     [children]
   );
 
@@ -123,8 +121,8 @@ export function Picker({
   );
 
   const renderItem = useCallback(
-    (normalizedItem: NormalizedPickerItem) => {
-      const key = getPickerItemKey(normalizedItem);
+    (normalizedItem: NormalizedItem) => {
+      const key = getItemKey(normalizedItem);
       const content = normalizedItem.item?.content ?? '';
       const textValue = normalizedItem.item?.textValue ?? '';
 
@@ -191,15 +189,15 @@ export function Picker({
   );
 
   const onSelectionChangeInternal = useCallback(
-    (key: PickerItemKey): void => {
+    (key: ItemKey): void => {
       // The `key` arg will always be a string due to us setting the `Item` key
       // prop in `renderItem`. We need to find the matching item to determine
       // the actual key.
       const selectedItem = normalizedItems.find(
-        item => String(getPickerItemKey(item)) === key
+        item => String(getItemKey(item)) === key
       );
 
-      const actualKey = getPickerItemKey(selectedItem) ?? key;
+      const actualKey = getItemKey(selectedItem) ?? key;
 
       (onChange ?? onSelectionChange)?.(actualKey);
     },
@@ -227,10 +225,10 @@ export function Picker({
       }
     >
       {itemOrSection => {
-        if (isNormalizedPickerSection(itemOrSection)) {
+        if (isNormalizedSection(itemOrSection)) {
           return (
             <Section
-              key={getPickerItemKey(itemOrSection)}
+              key={getItemKey(itemOrSection)}
               title={itemOrSection.item?.title}
               items={itemOrSection.item?.items}
             >

packages/components/src/spectrum/picker/index.ts

@@ -1,2 +1,2 @@
 export * from './Picker';
-export * from './PickerUtils';
+export * from './PickerItemContent';

packages/components/src/spectrum/utils/index.ts

@@ -0,0 +1,2 @@
+export * from './itemUtils';
+export * from './themeUtils';

packages/components/src/spectrum/picker/PickerUtils.test.tsx → packages/components/src/spectrum/utils/itemUtils.test.tsx

@@ -1,21 +1,21 @@
 import React, { createElement } from 'react';
 import {
-  getPickerItemKey,
-  INVALID_PICKER_ITEM_ERROR_MESSAGE,
+  getItemKey,
+  INVALID_ITEM_ERROR_MESSAGE,
   isItemElement,
   isNormalizedItemsWithKeysList,
-  isNormalizedPickerSection,
-  isPickerItemOrSection,
+  isNormalizedSection,
+  isItemOrSection,
   isSectionElement,
-  NormalizedPickerItem,
-  NormalizedPickerSection,
-  normalizePickerItemList,
+  NormalizedItem,
+  NormalizedSection,
+  normalizeItemList,
   normalizeTooltipOptions,
-  PickerItem,
-  PickerItemOrSection,
-  PickerSection,
-} from './PickerUtils';
-import type { PickerProps } from './Picker';
+  ItemElementOrPrimitive,
+  ItemOrSection,
+  SectionElement,
+} from './itemUtils';
+import type { PickerProps } from '../picker/Picker';
 import { Item, Section } from '../shared';
 import { Text } from '../Text';
 
@@ -103,7 +103,7 @@ const expectedItems = {
       },
     },
   ],
-} satisfies Record<string, [PickerItem, NormalizedPickerItem]>;
+} satisfies Record<string, [ItemElementOrPrimitive, NormalizedItem]>;
 /* eslint-enable react/jsx-key */
 
 const nonItemElement = <span>Non-item element</span>;
@@ -138,12 +138,12 @@ const expectedSections = {
       },
     },
   ],
-} satisfies Record<string, [PickerItem, NormalizedPickerSection]>;
+} satisfies Record<string, [ItemElementOrPrimitive, NormalizedSection]>;
 /* eslint-enable react/jsx-key */
 
 const expectedNormalizations = new Map<
-  PickerItem,
-  NormalizedPickerItem | NormalizedPickerSection
+  ItemElementOrPrimitive,
+  NormalizedItem | NormalizedSection
 >([...Object.values(expectedItems), ...Object.values(expectedSections)]);
 
 const mixedItems = [...expectedNormalizations.keys()];
@@ -154,17 +154,17 @@ const children = {
   mixed: mixedItems as PickerProps['children'],
 };
 
-describe('getPickerItemKey', () => {
+describe('getItemKey', () => {
   it.each([
     [{ key: 'top-level.key', item: { key: 'item.key' } }, 'item.key'],
     [{ key: 'top-level.key', item: {} }, 'top-level.key'],
     [{ key: 'top-level.key' }, 'top-level.key'],
     [{ item: { key: 'item.key' } }, 'item.key'],
     [{}, undefined],
-  ] as NormalizedPickerItem[])(
+  ] as NormalizedItem[])(
     'should return the item.key or fallback to the top-level key: %s, %s',
     (given, expected) => {
-      const actual = getPickerItemKey(given);
+      const actual = getItemKey(given);
       expect(actual).toBe(expected);
     }
   );
@@ -175,17 +175,17 @@ describe('isNormalizedItemsWithKeysList', () => {
     normalizedItemWithKey: {
       key: 'some.key',
       item: { content: '' },
-    } as NormalizedPickerItem,
+    } as NormalizedItem,
     normalizedSectionWithKey: {
       key: 'some.key',
       item: { items: [] },
-    } as NormalizedPickerSection,
-    item: (<Item>Item</Item>) as PickerItem,
+    } as NormalizedSection,
+    item: (<Item>Item</Item>) as ItemElementOrPrimitive,
     section: (
       <Section>
         <Item>Item</Item>
       </Section>
-    ) as PickerSection,
+    ) as SectionElement,
   } as const;
 
   it.each([
@@ -202,8 +202,8 @@ describe('isNormalizedItemsWithKeysList', () => {
     'should return true for a normalized items with keys list: %s, %s',
     (givenKeys, expected) => {
       const given = givenKeys.map(key => mock[key]) as
-        | PickerItemOrSection[]
-        | (NormalizedPickerItem | NormalizedPickerSection)[];
+        | ItemOrSection[]
+        | (NormalizedItem | NormalizedSection)[];
 
       expect(isNormalizedItemsWithKeysList(given)).toBe(expected);
     }
@@ -230,7 +230,7 @@ describe('isItemElement', () => {
   });
 });
 
-describe('isPickerItemOrSection', () => {
+describe('isItemOrSection', () => {
   it.each([
     [createElement(Item), true],
     [createElement(Section), true],
@@ -242,41 +242,38 @@ describe('isPickerItemOrSection', () => {
   ])(
     'should return true for a Item or Section element: %s, %s',
     (element, expected) => {
-      expect(isPickerItemOrSection(element)).toBe(expected);
+      expect(isItemOrSection(element)).toBe(expected);
     }
   );
 });
 
-describe('isNormalizedPickerSection', () => {
+describe('isNormalizedSection', () => {
   it.each([
-    [{ item: {} } as NormalizedPickerItem, false],
-    [{ item: { items: [] } } as NormalizedPickerSection, true],
-  ])(
-    'should return true for a normalized Picker section: %s',
-    (obj, expected) => {
-      expect(isNormalizedPickerSection(obj)).toBe(expected);
-    }
-  );
+    [{ item: {} } as NormalizedItem, false],
+    [{ item: { items: [] } } as NormalizedSection, true],
+  ])('should return true for a normalized section: %s', (obj, expected) => {
+    expect(isNormalizedSection(obj)).toBe(expected);
+  });
 });
 
-describe('normalizePickerItemList', () => {
+describe('normalizeItemList', () => {
   it.each([children.empty, children.single, children.mixed])(
-    'should return normalized picker items: %#: %s',
+    'should return normalized items: %#: %s',
     given => {
       const childrenArray = Array.isArray(given) ? given : [given];
 
       const expected = childrenArray.map(item =>
         expectedNormalizations.get(item)
       );
 
-      const actual = normalizePickerItemList(given);
+      const actual = normalizeItemList(given);
       expect(actual).toEqual(expected);
     }
   );
 
   it(`should throw for invalid items: %#: %s`, () => {
-    expect(() => normalizePickerItemList(nonItemElement)).toThrow(
-      INVALID_PICKER_ITEM_ERROR_MESSAGE
+    expect(() => normalizeItemList(nonItemElement)).toThrow(
+      INVALID_ITEM_ERROR_MESSAGE
     );
   });
 });