From a6f38fd6bb159649563b005efc2009fcf800363b Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Mon, 1 Apr 2024 17:17:58 -0500 Subject: [PATCH] Renamed picker symbols to more generic ones (#1909) --- .../code-studio/src/styleguide/Pickers.tsx | 6 +- .../components/src/spectrum/picker/Picker.tsx | 41 ++--- .../src/spectrum/utils/itemUtils.test.tsx | 75 ++++---- .../src/spectrum/utils/itemUtils.ts | 168 ++++++++---------- .../src/spectrum/Picker/Picker.tsx | 4 +- .../Picker/usePickerItemRowDeserializer.ts | 6 +- 6 files changed, 141 insertions(+), 159 deletions(-) diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx index 98ede5eed3..18dc678c10 100644 --- a/packages/code-studio/src/styleguide/Pickers.tsx +++ b/packages/code-studio/src/styleguide/Pickers.tsx @@ -3,7 +3,7 @@ import { Flex, Item, Picker, - PickerItemKey, + ItemKey, Section, Text, } from '@deephaven/components'; @@ -29,9 +29,9 @@ function PersonIcon(): JSX.Element { } export function Pickers(): JSX.Element { - const [selectedKey, setSelectedKey] = useState(); + const [selectedKey, setSelectedKey] = useState(); - const onChange = useCallback((key: PickerItemKey): void => { + const onChange = useCallback((key: ItemKey): void => { setSelectedKey(key); }, []); diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx index c22a130099..e706ede9d8 100644 --- a/packages/components/src/spectrum/picker/Picker.tsx +++ b/packages/components/src/spectrum/picker/Picker.tsx @@ -15,31 +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, + 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; /** @@ -48,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; @@ -57,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 @@ -114,7 +111,7 @@ export function Picker({ ...spectrumPickerProps }: PickerProps): JSX.Element { const normalizedItems = useMemo( - () => normalizePickerItemList(children), + () => normalizeItemList(children), [children] ); @@ -124,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 ?? ''; @@ -192,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); }, @@ -228,10 +225,10 @@ export function Picker({ } > {itemOrSection => { - if (isNormalizedPickerSection(itemOrSection)) { + if (isNormalizedSection(itemOrSection)) { return (
diff --git a/packages/components/src/spectrum/utils/itemUtils.test.tsx b/packages/components/src/spectrum/utils/itemUtils.test.tsx index 18eec1e7d2..e2cdad44c7 100644 --- a/packages/components/src/spectrum/utils/itemUtils.test.tsx +++ b/packages/components/src/spectrum/utils/itemUtils.test.tsx @@ -1,19 +1,19 @@ 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, + ItemElementOrPrimitive, + ItemOrSection, + SectionElement, } from './itemUtils'; import type { PickerProps } from '../picker/Picker'; import { Item, Section } from '../shared'; @@ -103,7 +103,7 @@ const expectedItems = { }, }, ], -} satisfies Record; +} satisfies Record; /* eslint-enable react/jsx-key */ const nonItemElement = Non-item element; @@ -138,12 +138,12 @@ const expectedSections = { }, }, ], -} satisfies Record; +} satisfies Record; /* 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) as PickerItem, + } as NormalizedSection, + item: (Item) as ItemElementOrPrimitive, section: (
Item
- ) 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,26 +242,23 @@ 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]; @@ -269,14 +266,14 @@ describe('normalizePickerItemList', () => { 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 ); }); }); diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index 0c3b9bbb96..06f5e852dc 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -6,10 +6,10 @@ import { KeyedItem } from '@deephaven/utils'; import { Item, ItemProps, Section, SectionProps } from '../shared'; import { PopperOptions } from '../../popper'; -const log = Log.module('PickerUtils'); +const log = Log.module('itemUtils'); -export const INVALID_PICKER_ITEM_ERROR_MESSAGE = - 'Picker items must be strings, numbers, booleans, or
elements:'; +export const INVALID_ITEM_ERROR_MESSAGE = + 'Items must be strings, numbers, booleans, or
elements:'; /** * React Spectrum
supports an `ItemRenderer` function as a child. The @@ -21,74 +21,70 @@ type SectionPropsNoItemRenderer = Omit, 'children'> & { }; type ItemElement = ReactElement>; -type SectionElement = ReactElement>; +export type SectionElement = ReactElement>; -export type PickerItem = number | string | boolean | ItemElement; -export type PickerSection = SectionElement; -export type PickerItemOrSection = PickerItem | PickerSection; +export type ItemElementOrPrimitive = number | string | boolean | ItemElement; +export type ItemOrSection = ItemElementOrPrimitive | SectionElement; /** * Augment the Spectrum selection key type to include boolean values. - * The Spectrum Picker already supports this, but the built in types don't - * reflect it. + * Spectrum collection components already supports this, but the built in types + * don't reflect it. */ -export type PickerItemKey = Key | boolean; +export type ItemKey = Key | boolean; /** * Augment the Spectrum selection change handler type to include boolean keys. - * The Spectrum Picker already supports this, but the built in types don't + * Spectrum components already supports this, but the built in types don't * reflect it. */ -export type PickerSelectionChangeHandler = (key: PickerItemKey) => void; +export type ItemSelectionChangeHandler = (key: ItemKey) => void; -export interface NormalizedPickerItemData { - key?: PickerItemKey; +export interface NormalizedItemData { + key?: ItemKey; content: ReactNode; textValue?: string; } -export interface NormalizedPickerSectionData { +export interface NormalizedSectionData { key?: Key; title?: ReactNode; - items: NormalizedPickerItem[]; + items: NormalizedItem[]; } /** - * The Picker supports a variety of item types, including strings, numbers, - * booleans, and more complex React elements. This type represents a normalized - * form to make rendering items simpler and keep the logic of transformation - * in separate util methods. It also adheres to the `KeyedItem` interface to - * be compatible with Windowed data utils (e.g. `useViewportData`). + * Spectrum collection components support a variety of item types, including + * strings, numbers, booleans, and more complex React elements. This type + * represents a normalized form to make rendering items simpler and keep the + * logic of transformation in separate util methods. It also adheres to the + * `KeyedItem` interface to be compatible with Windowed data utils + * (e.g. `useViewportData`). */ -export type NormalizedPickerItem = KeyedItem< - NormalizedPickerItemData, - PickerItemKey | undefined ->; +export type NormalizedItem = KeyedItem; -export type NormalizedPickerSection = KeyedItem< - NormalizedPickerSectionData, +export type NormalizedSection = KeyedItem< + NormalizedSectionData, Key | undefined >; -export type NormalizedSpectrumPickerProps = - SpectrumPickerProps; +export type NormalizedSpectrumPickerProps = SpectrumPickerProps; export type TooltipOptions = { placement: PopperOptions['placement'] }; /** - * Picker uses a normalized item that includes a `key` prop and an optional - * `item` prop. This is mostly to support Windowed data where items are created - * before their data has been loaded (data gets set in the `item` prop). If - * data has loaded, return its `key`. If not, return the top-level `key` on the - * normalized item. - * @param item The normalized picker item or section + * DH wrappers of Spectrum collection components use a normalized item that + * includes a `key` prop and an optional `item` prop. This is mostly to support + * Windowed data where items are created before their data has been loaded (data + * gets set in the `item` prop). If data has loaded, return its `key`. If not, + * return the top-level `key` on the normalized item. + * @param item The normalized item or section * @returns The `key` of the item or section */ -export function getPickerItemKey< - TItem extends NormalizedPickerItem | NormalizedPickerSection, - TKey extends TItem extends NormalizedPickerItem - ? PickerItemKey | undefined - : TItem extends NormalizedPickerSection +export function getItemKey< + TItem extends NormalizedItem | NormalizedSection, + TKey extends TItem extends NormalizedItem + ? ItemKey | undefined + : TItem extends NormalizedSection ? Key | undefined : undefined, >(item: TItem | null | undefined): TKey { @@ -124,11 +120,8 @@ export function isItemElement( * @returns True if the node is a normalized item with keys array */ export function isNormalizedItemsWithKeysList( - node: - | PickerItemOrSection - | PickerItemOrSection[] - | (NormalizedPickerItem | NormalizedPickerSection)[] -): node is (NormalizedPickerItem | NormalizedPickerSection)[] { + node: ItemOrSection | ItemOrSection[] | (NormalizedItem | NormalizedSection)[] +): node is (NormalizedItem | NormalizedSection)[] { if (!Array.isArray(node)) { return false; } @@ -137,32 +130,30 @@ export function isNormalizedItemsWithKeysList( return true; } - return !isPickerItemOrSection(node[0]) && 'key' in node[0]; + return !isItemOrSection(node[0]) && 'key' in node[0]; } /** - * Determine if an object is a normalized Picker section. - * @param maybeNormalizedPickerSection The object to check - * @returns True if the object is a normalized Picker section + * Determine if an object is a normalized section. + * @param maybeNormalizedSection The object to check + * @returns True if the object is a normalized section */ -export function isNormalizedPickerSection( - maybeNormalizedPickerSection: NormalizedPickerItem | NormalizedPickerSection -): maybeNormalizedPickerSection is NormalizedPickerSection { +export function isNormalizedSection( + maybeNormalizedSection: NormalizedItem | NormalizedSection +): maybeNormalizedSection is NormalizedSection { return ( - maybeNormalizedPickerSection.item != null && - 'items' in maybeNormalizedPickerSection.item + maybeNormalizedSection.item != null && + 'items' in maybeNormalizedSection.item ); } /** - * Determine if a node is a Picker item or section. Valid types include strings, + * Determine if a node is an item or section. Valid types include strings, * numbers, booleans, Item elements, and Section elements. * @param node The node to check - * @returns True if the node is a Picker item or section + * @returns True if the node is an item or section */ -export function isPickerItemOrSection( - node: ReactNode -): node is PickerItemOrSection { +export function isItemOrSection(node: ReactNode): node is ItemOrSection { return ( typeof node === 'string' || typeof node === 'number' || @@ -173,15 +164,15 @@ export function isPickerItemOrSection( } /** - * Determine the `key` of a picker item or section. - * @param itemOrSection The picker item or section - * @returns A `PickerItemKey` for the picker item or undefined if a key can't be determined + * Determine the `key` of an item or section. + * @param itemOrSection The item or section + * @returns A `ItemKey` for the item or undefined if a key can't be determined */ -function normalizeItemKey(item: PickerItem): PickerItemKey | undefined; -function normalizeItemKey(section: PickerSection): Key | undefined; +function normalizeItemKey(item: ItemElementOrPrimitive): ItemKey | undefined; +function normalizeItemKey(section: SectionElement): Key | undefined; function normalizeItemKey( - itemOrSection: PickerItem | PickerSection -): Key | PickerItemKey | undefined { + itemOrSection: ItemElementOrPrimitive | SectionElement +): Key | ItemKey | undefined { // string, number, or boolean if (typeof itemOrSection !== 'object') { return itemOrSection; @@ -209,11 +200,11 @@ function normalizeItemKey( } /** - * Get a normalized `textValue` for a picker item ensuring it is a string. - * @param item The picker item - * @returns A string `textValue` for the picker item + * Get a normalized `textValue` for an item ensuring it is a string. + * @param item The item + * @returns A string `textValue` for the item */ -function normalizeTextValue(item: PickerItem): string | undefined { +function normalizeTextValue(item: ItemElementOrPrimitive): string | undefined { if (typeof item !== 'object') { return String(item); } @@ -230,26 +221,26 @@ function normalizeTextValue(item: PickerItem): string | undefined { } /** - * Normalize a picker item to an object form. + * Normalize an item or section to an object form. * @param itemOrSection item to normalize - * @returns NormalizedPickerItem object + * @returns NormalizedItem or NormalizedSection object */ -function normalizePickerItem( - itemOrSection: PickerItemOrSection -): NormalizedPickerItem | NormalizedPickerSection { - if (!isPickerItemOrSection(itemOrSection)) { - log.debug(INVALID_PICKER_ITEM_ERROR_MESSAGE, itemOrSection); - throw new Error(INVALID_PICKER_ITEM_ERROR_MESSAGE); +function normalizeItem( + itemOrSection: ItemOrSection +): NormalizedItem | NormalizedSection { + if (!isItemOrSection(itemOrSection)) { + log.debug(INVALID_ITEM_ERROR_MESSAGE, itemOrSection); + throw new Error(INVALID_ITEM_ERROR_MESSAGE); } if (isSectionElement(itemOrSection)) { const key = normalizeItemKey(itemOrSection); const { title } = itemOrSection.props; - const items = normalizePickerItemList(itemOrSection.props.children).filter( + const items = normalizeItemList(itemOrSection.props.children).filter( // We don't support nested section elements childItem => !isSectionElement(childItem) - ) as NormalizedPickerItem[]; + ) as NormalizedItem[]; return { item: { key, title, items }, @@ -268,16 +259,13 @@ function normalizePickerItem( } /** - * Get normalized picker items from a picker item or array of picker items. - * @param itemsOrSections A picker item or array of picker items - * @returns An array of normalized picker items + * Get normalized items from an item or array of items. + * @param itemsOrSections An item or array of items + * @returns An array of normalized items */ -export function normalizePickerItemList( - itemsOrSections: - | PickerItemOrSection - | PickerItemOrSection[] - | NormalizedPickerItem[] -): (NormalizedPickerItem | NormalizedPickerSection)[] { +export function normalizeItemList( + itemsOrSections: ItemOrSection | ItemOrSection[] | NormalizedItem[] +): (NormalizedItem | NormalizedSection)[] { // If already normalized, just return as-is if (isNormalizedItemsWithKeysList(itemsOrSections)) { return itemsOrSections; @@ -287,7 +275,7 @@ export function normalizePickerItemList( ? itemsOrSections : [itemsOrSections]; - return itemsArray.map(normalizePickerItem); + return itemsArray.map(normalizeItem); } /** diff --git a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx index 704dfc84d9..45fbf52291 100644 --- a/packages/jsapi-components/src/spectrum/Picker/Picker.tsx +++ b/packages/jsapi-components/src/spectrum/Picker/Picker.tsx @@ -1,5 +1,5 @@ import { - NormalizedPickerItemData, + NormalizedItemData, Picker as PickerBase, PickerProps as PickerPropsBase, } from '@deephaven/components'; @@ -67,7 +67,7 @@ export function Picker({ }, [getItemIndexByValue]); const { viewportData, onScroll, setViewport } = useViewportData< - NormalizedPickerItemData, + NormalizedItemData, DhType.Table >({ reuseItemsOnTableResize: true, diff --git a/packages/jsapi-components/src/spectrum/Picker/usePickerItemRowDeserializer.ts b/packages/jsapi-components/src/spectrum/Picker/usePickerItemRowDeserializer.ts index 017e1a5304..28c5822a32 100644 --- a/packages/jsapi-components/src/spectrum/Picker/usePickerItemRowDeserializer.ts +++ b/packages/jsapi-components/src/spectrum/Picker/usePickerItemRowDeserializer.ts @@ -1,5 +1,5 @@ import { useCallback, useMemo } from 'react'; -import { NormalizedPickerItemData } from '@deephaven/components'; +import { NormalizedItemData } from '@deephaven/components'; import { dh } from '@deephaven/jsapi-types'; import { getPickerKeyColumn, getPickerLabelColumn } from './PickerUtils'; @@ -39,7 +39,7 @@ export function usePickerItemRowDeserializer({ keyColumnName?: string; labelColumnName?: string; formatValue?: (value: unknown, columnType: string) => string; -}): (row: dh.Row) => NormalizedPickerItemData { +}): (row: dh.Row) => NormalizedItemData { const keyColumn = useMemo( () => getPickerKeyColumn(table, keyColumnName), [keyColumnName, table] @@ -51,7 +51,7 @@ export function usePickerItemRowDeserializer({ ); const deserializeRow = useCallback( - (row: dh.Row): NormalizedPickerItemData => { + (row: dh.Row): NormalizedItemData => { const key = defaultFormatKey(row.get(keyColumn)); const content = formatValue(row.get(labelColumn), labelColumn.type);