From 977ca8a8cdd17fea8f35b8bee419381b558b3765 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Tue, 16 Apr 2024 11:52:37 -0500
Subject: [PATCH 01/30] Find element scroll position (#1935)

---
 .../src/spectrum/utils/itemUtils.ts           | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index 351d13f94d..fb20fdc0b3 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -96,6 +96,42 @@ export function getItemKey<
   return (item?.item?.key ?? item?.key) as TKey;
 }
 
+export async function getPositionOfSelectedItem<
+  TKey extends string | number | boolean | undefined,
+>({
+  scrollAreaEl,
+  selectedKey,
+}: {
+  scrollAreaEl: HTMLElement;
+  selectedKey: TKey | null | undefined;
+}): Promise<number> {
+  const el = scrollAreaEl.querySelector<HTMLElement>(
+    `[data-key="${selectedKey}"]`
+  );
+  console.log(
+    '[TESTING] el:',
+    el,
+    selectedKey,
+    scrollAreaEl.firstChild?.childNodes.length
+  );
+  if (el == null) {
+    return 0;
+  }
+
+  const containerRect = scrollAreaEl.getBoundingClientRect();
+  const elRect = el.getBoundingClientRect();
+
+  // Account for scrolling by subtracting the scroll position of the container
+  const scrollOffset = scrollAreaEl.scrollTop;
+
+  // Calculate the position of the element relative to the container, accounting for scrolling
+  const relativeTopPosition = elRect.top - containerRect.top - scrollOffset;
+
+  console.log('[TESTING] position:', relativeTopPosition, scrollOffset);
+
+  return relativeTopPosition;
+}
+
 /**
  * Determine if a node is a Section element.
  * @param node The node to check

From cea664223d6643e924f8b6717f3bb0990aeadede Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 09:40:52 -0500
Subject: [PATCH 02/30] Find scroll position in elements (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx |  34 ++++--
 .../src/spectrum/utils/itemUtils.ts           |  89 ++++++++++------
 .../src/spectrum/utils/itemWrapperUtils.tsx   | 100 ++++++++++++++++++
 .../jsapi-components/src/spectrum/Picker.tsx  |   6 +-
 packages/utils/src/TextUtils.ts               |   4 +-
 packages/utils/src/UIConstants.ts             |   5 +-
 packages/utils/src/index.ts                   |   2 +-
 7 files changed, 194 insertions(+), 46 deletions(-)
 create mode 100644 packages/components/src/spectrum/utils/itemWrapperUtils.tsx

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index d4bba67aa8..bc0b0e4742 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -1,14 +1,13 @@
-import { useCallback, useMemo } from 'react';
+import { useCallback, useMemo, useState } from 'react';
 import { DOMRef } from '@react-types/shared';
 import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
 import {
-  getPositionOfSelectedItem,
   findSpectrumPickerScrollArea,
   usePopoverOnScrollRef,
 } from '@deephaven/react-hooks';
 import {
   EMPTY_FUNCTION,
-  PICKER_ITEM_HEIGHT,
+  PICKER_ITEM_HEIGHTS,
   PICKER_TOP_OFFSET,
 } from '@deephaven/utils';
 import cl from 'classnames';
@@ -22,9 +21,11 @@ import {
   TooltipOptions,
   ItemKey,
   getItemKey,
+  getPositionOfSelectedItem,
 } from '../utils/itemUtils';
 import { Section } from '../shared';
 import { useRenderNormalizedItem } from '../utils';
+import { wrapItemChildren } from '../utils/itemWrapperUtils';
 
 export type PickerProps = {
   children: ItemOrSection | ItemOrSection[] | NormalizedItem[];
@@ -97,21 +98,29 @@ export function Picker({
     [tooltip]
   );
 
+  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
+    useState(defaultSelectedKey);
+
   const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
 
   const getInitialScrollPositionInternal = useCallback(
     () =>
       getInitialScrollPosition == null
         ? getPositionOfSelectedItem({
-            keyedItems: normalizedItems,
-            // TODO: #1890 & deephaven-plugins#371 add support for sections and
-            // items with descriptions since they impact the height calculations
-            itemHeight: PICKER_ITEM_HEIGHT,
-            selectedKey,
+            children: wrapItemChildren(children, tooltipOptions),
+            itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+            itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+            selectedKey: selectedKey ?? uncontrolledSelectedKey,
             topOffset: PICKER_TOP_OFFSET,
           })
         : getInitialScrollPosition(),
-    [getInitialScrollPosition, normalizedItems, selectedKey]
+    [
+      children,
+      getInitialScrollPosition,
+      selectedKey,
+      tooltipOptions,
+      uncontrolledSelectedKey,
+    ]
   );
 
   const { ref: scrollRef, onOpenChange: popoverOnOpenChange } =
@@ -142,9 +151,14 @@ export function Picker({
 
       const actualKey = getItemKey(selectedItem) ?? key;
 
+      // If our component is uncontrolled, track the selected key internally
+      if (selectedKey == null) {
+        setUncontrolledSelectedKey(actualKey);
+      }
+
       (onChange ?? onSelectionChange)?.(actualKey);
     },
-    [normalizedItems, onChange, onSelectionChange]
+    [normalizedItems, onChange, onSelectionChange, selectedKey]
   );
 
   return (
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index fb20fdc0b3..165e7dc32d 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -1,10 +1,13 @@
-import { isValidElement, Key, ReactElement, ReactNode } from 'react';
+import { Key, ReactElement, ReactNode } from 'react';
 import { SpectrumPickerProps } from '@adobe/react-spectrum';
 import type { ItemRenderer } from '@react-types/shared';
 import Log from '@deephaven/log';
-import { KeyedItem, SelectionT } from '@deephaven/utils';
+import { isElementOfType } from '@deephaven/react-hooks';
+import { KeyedItem, PICKER_ITEM_HEIGHTS, SelectionT } from '@deephaven/utils';
 import { Item, ItemProps, Section, SectionProps } from '../shared';
 import { PopperOptions } from '../../popper';
+import { Text } from '../Text';
+import ItemContent from '../ItemContent';
 
 const log = Log.module('itemUtils');
 
@@ -20,7 +23,7 @@ type SectionPropsNoItemRenderer<T> = Omit<SectionProps<T>, 'children'> & {
   children: Exclude<SectionProps<T>['children'], ItemRenderer<T>>;
 };
 
-type ItemElement = ReactElement<ItemProps<unknown>>;
+export type ItemElement = ReactElement<ItemProps<unknown>>;
 export type SectionElement = ReactElement<SectionPropsNoItemRenderer<unknown>>;
 
 export type ItemElementOrPrimitive = number | string | boolean | ItemElement;
@@ -99,37 +102,34 @@ export function getItemKey<
 export async function getPositionOfSelectedItem<
   TKey extends string | number | boolean | undefined,
 >({
-  scrollAreaEl,
+  children,
+  itemHeight,
+  itemHeightWithDescription,
   selectedKey,
+  topOffset,
 }: {
-  scrollAreaEl: HTMLElement;
+  children: (ItemElement | SectionElement)[];
   selectedKey: TKey | null | undefined;
+  itemHeight: number;
+  itemHeightWithDescription: number;
+  topOffset: number;
 }): Promise<number> {
-  const el = scrollAreaEl.querySelector<HTMLElement>(
-    `[data-key="${selectedKey}"]`
-  );
-  console.log(
-    '[TESTING] el:',
-    el,
-    selectedKey,
-    scrollAreaEl.firstChild?.childNodes.length
-  );
-  if (el == null) {
-    return 0;
+  let position = 0;
+
+  // eslint-disable-next-line no-restricted-syntax
+  for (const child of children) {
+    if (child.key === selectedKey) {
+      break;
+    }
+
+    if (isItemElementWithDescription(child)) {
+      position += PICKER_ITEM_HEIGHTS.withDescription;
+    } else {
+      position += PICKER_ITEM_HEIGHTS.noDescription;
+    }
   }
 
-  const containerRect = scrollAreaEl.getBoundingClientRect();
-  const elRect = el.getBoundingClientRect();
-
-  // Account for scrolling by subtracting the scroll position of the container
-  const scrollOffset = scrollAreaEl.scrollTop;
-
-  // Calculate the position of the element relative to the container, accounting for scrolling
-  const relativeTopPosition = elRect.top - containerRect.top - scrollOffset;
-
-  console.log('[TESTING] position:', relativeTopPosition, scrollOffset);
-
-  return relativeTopPosition;
+  return position + topOffset;
 }
 
 /**
@@ -140,7 +140,7 @@ export async function getPositionOfSelectedItem<
 export function isSectionElement<T>(
   node: ReactNode
 ): node is ReactElement<SectionProps<T>> {
-  return isValidElement<SectionProps<T>>(node) && node.type === Section;
+  return isElementOfType(node, Section);
 }
 
 /**
@@ -151,7 +151,36 @@ export function isSectionElement<T>(
 export function isItemElement<T>(
   node: ReactNode
 ): node is ReactElement<ItemProps<T>> {
-  return isValidElement<ItemProps<T>>(node) && node.type === Item;
+  return isElementOfType(node, Item);
+}
+
+export function isItemElementWithDescription<T>(
+  node: ReactNode
+): node is ReactElement<ItemProps<T>> {
+  if (!isItemElement(node)) {
+    return false;
+  }
+
+  const children = isElementOfType(node.props.children, ItemContent)
+    ? node.props.children.props.children
+    : node.props.children;
+
+  const childrenArray = Array.isArray(children) ? children : [children];
+
+  const result = childrenArray.some(
+    child => child.props?.slot === 'description' && isElementOfType(child, Text)
+    // (isElementOfType(child, Text) || child.props.children?.[0].type === Text)
+  );
+
+  // console.log(
+  //   '[TESTING] result:',
+  //   result,
+  //   node.key,
+  //   childrenArray,
+  //   childrenArray.map(child => [child, Text])
+  // );
+
+  return result;
 }
 
 /**
diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
new file mode 100644
index 0000000000..d8f8087613
--- /dev/null
+++ b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
@@ -0,0 +1,100 @@
+import { cloneElement, ReactElement, ReactNode } from 'react';
+import { Item } from '@adobe/react-spectrum';
+import { isElementOfType } from '@deephaven/react-hooks';
+import { NON_BREAKING_SPACE } from '@deephaven/utils';
+import { Text } from '../Text';
+import {
+  isItemElement,
+  isSectionElement,
+  ItemElement,
+  ItemOrSection,
+  SectionElement,
+  TooltipOptions,
+} from './itemUtils';
+import { ItemProps } from '../shared';
+import ItemContent from '../ItemContent';
+
+/**
+ * If the given content is a primitive type, wrap it in a Text component.
+ * @param content The content to wrap
+ * @param slot The slot to use for the Text component
+ * @returns The wrapped content or original content if not a primitive type
+ */
+export function wrapPrimitiveWithText(
+  content?: ReactNode,
+  slot?: string
+): ReactNode {
+  // eslint-disable-next-line no-param-reassign
+  content = content ?? '';
+
+  if (['string', 'boolean', 'number'].includes(typeof content)) {
+    return (
+      <Text slot={slot}>
+        {content === '' ? NON_BREAKING_SPACE : String(content)}
+      </Text>
+    );
+  }
+
+  return content;
+}
+
+/**
+ * Ensure all primitive children are wrapped in `Item` elements and that all
+ * `Item` element content is wrapped in `ItemContent` elements to handle text
+ * overflow consistently and to support tooltips.
+ * @param itemsOrSections The items or sections to wrap
+ * @param tooltipOptions The tooltip options to use when wrapping items
+ * @returns The wrapped items or sections
+ */
+export function wrapItemChildren(
+  itemsOrSections: ItemOrSection | ItemOrSection[],
+  tooltipOptions: TooltipOptions | null
+): (ItemElement | SectionElement)[] {
+  const itemsOrSectionsArray = Array.isArray(itemsOrSections)
+    ? itemsOrSections
+    : [itemsOrSections];
+
+  return itemsOrSectionsArray.map((item: ItemOrSection) => {
+    if (isItemElement(item)) {
+      if (isElementOfType(item.props.children, ItemContent)) {
+        return item;
+      }
+
+      // Wrap in `ItemContent` so we can support tooltips and handle text
+      // overflow
+      return cloneElement(item, {
+        ...item.props,
+        children: (
+          <ItemContent tooltipOptions={tooltipOptions}>
+            {wrapPrimitiveWithText(item.props.children)}
+          </ItemContent>
+        ),
+      });
+    }
+
+    if (isSectionElement(item)) {
+      return cloneElement(item, {
+        ...item.props,
+        children: wrapItemChildren(
+          item.props.children,
+          tooltipOptions
+        ) as ReactElement<ItemProps<unknown>>[],
+      });
+    }
+
+    if (
+      typeof item === 'string' ||
+      typeof item === 'number' ||
+      typeof item === 'boolean'
+    ) {
+      const text = String(item);
+      return (
+        <Item key={text} textValue={text}>
+          <ItemContent tooltipOptions={tooltipOptions}>{text}</ItemContent>
+        </Item>
+      );
+    }
+
+    return item;
+  });
+}
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index 370c4b77cc..43f741870c 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -6,7 +6,7 @@ import {
 import { dh as DhType } from '@deephaven/jsapi-types';
 import { Settings } from '@deephaven/jsapi-utils';
 import Log from '@deephaven/log';
-import { PICKER_ITEM_HEIGHT, PICKER_TOP_OFFSET } from '@deephaven/utils';
+import { PICKER_ITEM_HEIGHTS, PICKER_TOP_OFFSET } from '@deephaven/utils';
 import { useCallback, useEffect, useMemo } from 'react';
 import useFormatter from '../useFormatter';
 import useGetItemIndexByValue from '../useGetItemIndexByValue';
@@ -63,7 +63,7 @@ export function Picker({
       return null;
     }
 
-    return index * PICKER_ITEM_HEIGHT + PICKER_TOP_OFFSET;
+    return index * PICKER_ITEM_HEIGHTS.noDescription + PICKER_TOP_OFFSET;
   }, [getItemIndexByValue]);
 
   const { viewportData, onScroll, setViewport } = useViewportData<
@@ -72,7 +72,7 @@ export function Picker({
   >({
     reuseItemsOnTableResize: true,
     table,
-    itemHeight: PICKER_ITEM_HEIGHT,
+    itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
     deserializeRow,
   });
 
diff --git a/packages/utils/src/TextUtils.ts b/packages/utils/src/TextUtils.ts
index e75fbf63fc..fe372e9f3a 100644
--- a/packages/utils/src/TextUtils.ts
+++ b/packages/utils/src/TextUtils.ts
@@ -1,4 +1,6 @@
-class TextUtils {
+export const NON_BREAKING_SPACE = '\xa0';
+
+export class TextUtils {
   /**
    * Joins a list of strings with a comma, keeping the oxford comma and adding "and" as appropriate.
    * Eg.
diff --git a/packages/utils/src/UIConstants.ts b/packages/utils/src/UIConstants.ts
index a7cf22fabf..4286849996 100644
--- a/packages/utils/src/UIConstants.ts
+++ b/packages/utils/src/UIConstants.ts
@@ -2,7 +2,10 @@ export const ACTION_ICON_HEIGHT = 24;
 export const COMBO_BOX_ITEM_HEIGHT = 32;
 export const COMBO_BOX_TOP_OFFSET = 4;
 export const ITEM_KEY_PREFIX = 'DH_ITEM_KEY';
-export const PICKER_ITEM_HEIGHT = 32;
+export const PICKER_ITEM_HEIGHTS = {
+  noDescription: 32,
+  withDescription: 48,
+} as const;
 export const PICKER_TOP_OFFSET = 4;
 export const TABLE_ROW_HEIGHT = 33;
 export const SCROLL_DEBOUNCE_MS = 150;
diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts
index 0a1e112cb0..dc0afb0ada 100644
--- a/packages/utils/src/index.ts
+++ b/packages/utils/src/index.ts
@@ -18,7 +18,7 @@ export * from './ErrorUtils';
 export * from './ObjectUtils';
 export { default as RangeUtils, generateRange } from './RangeUtils';
 export type { Range } from './RangeUtils';
-export { default as TextUtils } from './TextUtils';
+export * from './TextUtils';
 export { default as TimeoutError } from './TimeoutError';
 export { default as TimeUtils } from './TimeUtils';
 export * from './TypeUtils';

From e8f1a3bc55ab64162461a3fef61dc4ae53618466 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 10:25:26 -0500
Subject: [PATCH 03/30] Cleanup (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx | 43 +++++++++++--------
 .../src/spectrum/utils/itemUtils.ts           |  4 +-
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index bc0b0e4742..5e1258e85e 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -22,6 +22,7 @@ import {
   ItemKey,
   getItemKey,
   getPositionOfSelectedItem,
+  isNormalizedItemsWithKeysList,
 } from '../utils/itemUtils';
 import { Section } from '../shared';
 import { useRenderNormalizedItem } from '../utils';
@@ -103,25 +104,29 @@ export function Picker({
 
   const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
 
-  const getInitialScrollPositionInternal = useCallback(
-    () =>
-      getInitialScrollPosition == null
-        ? getPositionOfSelectedItem({
-            children: wrapItemChildren(children, tooltipOptions),
-            itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
-            itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-            selectedKey: selectedKey ?? uncontrolledSelectedKey,
-            topOffset: PICKER_TOP_OFFSET,
-          })
-        : getInitialScrollPosition(),
-    [
-      children,
-      getInitialScrollPosition,
-      selectedKey,
-      tooltipOptions,
-      uncontrolledSelectedKey,
-    ]
-  );
+  const getInitialScrollPositionInternal = useCallback(async () => {
+    if (getInitialScrollPosition != null) {
+      return getInitialScrollPosition();
+    }
+
+    if (isNormalizedItemsWithKeysList(children)) {
+      return null;
+    }
+
+    return getPositionOfSelectedItem({
+      children: wrapItemChildren(children, tooltipOptions),
+      itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+      itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+      selectedKey: selectedKey ?? uncontrolledSelectedKey,
+      topOffset: PICKER_TOP_OFFSET,
+    });
+  }, [
+    children,
+    getInitialScrollPosition,
+    selectedKey,
+    tooltipOptions,
+    uncontrolledSelectedKey,
+  ]);
 
   const { ref: scrollRef, onOpenChange: popoverOnOpenChange } =
     usePopoverOnScrollRef(
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index 165e7dc32d..e59e77f75e 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -123,9 +123,9 @@ export async function getPositionOfSelectedItem<
     }
 
     if (isItemElementWithDescription(child)) {
-      position += PICKER_ITEM_HEIGHTS.withDescription;
+      position += itemHeightWithDescription;
     } else {
-      position += PICKER_ITEM_HEIGHTS.noDescription;
+      position += itemHeight;
     }
   }
 

From 73d29b0766964e5d97af497fd44764f14c9fddbc Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 12:24:02 -0500
Subject: [PATCH 04/30] Started splitting Picker component (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx |   4 +-
 .../spectrum/picker/PickerFromChildren.tsx    | 137 ++++++++++++++++
 .../src/spectrum/picker/PickerFromItems.tsx   | 146 ++++++++++++++++++
 .../components/src/spectrum/picker/index.ts   |   2 +
 .../spectrum/picker/usePickerScrollOnOpen.ts  |  46 ++++++
 .../src/spectrum/utils/itemUtils.ts           |   4 +-
 .../jsapi-components/src/spectrum/Picker.tsx  |   6 +-
 .../react-hooks/src/usePopoverOnScrollRef.ts  |   2 +-
 8 files changed, 339 insertions(+), 8 deletions(-)
 create mode 100644 packages/components/src/spectrum/picker/PickerFromChildren.tsx
 create mode 100644 packages/components/src/spectrum/picker/PickerFromItems.tsx
 create mode 100644 packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index 5e1258e85e..d556c8c0c1 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -21,7 +21,7 @@ import {
   TooltipOptions,
   ItemKey,
   getItemKey,
-  getPositionOfSelectedItem,
+  getPositionOfSelectedItemElement,
   isNormalizedItemsWithKeysList,
 } from '../utils/itemUtils';
 import { Section } from '../shared';
@@ -113,7 +113,7 @@ export function Picker({
       return null;
     }
 
-    return getPositionOfSelectedItem({
+    return getPositionOfSelectedItemElement({
       children: wrapItemChildren(children, tooltipOptions),
       itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
       itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
diff --git a/packages/components/src/spectrum/picker/PickerFromChildren.tsx b/packages/components/src/spectrum/picker/PickerFromChildren.tsx
new file mode 100644
index 0000000000..499d031c66
--- /dev/null
+++ b/packages/components/src/spectrum/picker/PickerFromChildren.tsx
@@ -0,0 +1,137 @@
+import { useCallback, useMemo, useState } from 'react';
+import { DOMRef } from '@react-types/shared';
+import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
+
+import {
+  EMPTY_FUNCTION,
+  PICKER_ITEM_HEIGHTS,
+  PICKER_TOP_OFFSET,
+} from '@deephaven/utils';
+import cl from 'classnames';
+import {
+  NormalizedSpectrumPickerProps,
+  normalizeTooltipOptions,
+  ItemOrSection,
+  TooltipOptions,
+  ItemKey,
+  getPositionOfSelectedItemElement,
+} from '../utils/itemUtils';
+import { wrapItemChildren } from '../utils/itemWrapperUtils';
+import usePickerScrollOnOpen from './usePickerScrollOnOpen';
+
+export type PickerFromChildrenProps = {
+  children: ItemOrSection | ItemOrSection[];
+  /** Can be set to true or a TooltipOptions to enable item tooltips */
+  tooltip?: boolean | TooltipOptions;
+  /** The currently selected key in the collection (controlled). */
+  selectedKey?: ItemKey | null;
+  /** The initial selected key in the collection (uncontrolled). */
+  defaultSelectedKey?: ItemKey;
+  /**
+   * Handler that is called when the selection change.
+   * Note that under the hood, this is just an alias for Spectrum's
+   * `onSelectionChange`. We are renaming for better consistency with other
+   * components.
+   */
+  onChange?: (key: ItemKey) => void;
+
+  /** Handler that is called when the picker is scrolled. */
+  onScroll?: (event: Event) => void;
+
+  /**
+   * Handler that is called when the selection changes.
+   * @deprecated Use `onChange` instead
+   */
+  onSelectionChange?: (key: ItemKey) => void;
+} /*
+ * Support remaining SpectrumPickerProps.
+ * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
+ * re-defined above to account for boolean types which aren't included in the
+ * React `Key` type, but are actually supported by the Spectrum Picker component.
+ */ & Omit<
+  NormalizedSpectrumPickerProps,
+  | 'children'
+  | 'items'
+  | 'onSelectionChange'
+  | 'selectedKey'
+  | 'defaultSelectedKey'
+>;
+
+export function PickerFromChildren({
+  children,
+  tooltip = true,
+  defaultSelectedKey,
+  selectedKey,
+  onChange,
+  onOpenChange,
+  onScroll = EMPTY_FUNCTION,
+  onSelectionChange,
+  // eslint-disable-next-line camelcase
+  UNSAFE_className,
+  ...spectrumPickerProps
+}: PickerFromChildrenProps): JSX.Element {
+  const tooltipOptions = useMemo(
+    () => normalizeTooltipOptions(tooltip),
+    [tooltip]
+  );
+
+  const wrappedItems = useMemo(
+    () => wrapItemChildren(children, tooltipOptions),
+    [children, tooltipOptions]
+  );
+
+  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
+    useState(defaultSelectedKey);
+
+  const getInitialScrollPosition = useCallback(
+    async () =>
+      getPositionOfSelectedItemElement({
+        children: wrappedItems,
+        itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+        itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+        selectedKey: selectedKey ?? uncontrolledSelectedKey,
+        topOffset: PICKER_TOP_OFFSET,
+      }),
+    [selectedKey, uncontrolledSelectedKey, wrappedItems]
+  );
+
+  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+    usePickerScrollOnOpen({
+      getInitialScrollPosition,
+      onScroll,
+      onOpenChange,
+    });
+
+  const onSelectionChangeInternal = useCallback(
+    (key: ItemKey): void => {
+      // If our component is uncontrolled, track the selected key internally
+      if (selectedKey == null) {
+        setUncontrolledSelectedKey(key);
+      }
+
+      (onChange ?? onSelectionChange)?.(key);
+    },
+    [onChange, onSelectionChange, selectedKey]
+  );
+
+  return (
+    <SpectrumPicker
+      // eslint-disable-next-line react/jsx-props-no-spreading
+      {...spectrumPickerProps}
+      ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
+      onOpenChange={onOpenChangeInternal}
+      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+      selectedKey={selectedKey as NormalizedSpectrumPickerProps['selectedKey']}
+      defaultSelectedKey={
+        defaultSelectedKey as NormalizedSpectrumPickerProps['defaultSelectedKey']
+      }
+      onSelectionChange={
+        onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
+      }
+    >
+      {wrappedItems}
+    </SpectrumPicker>
+  );
+}
+
+export default PickerFromChildren;
diff --git a/packages/components/src/spectrum/picker/PickerFromItems.tsx b/packages/components/src/spectrum/picker/PickerFromItems.tsx
new file mode 100644
index 0000000000..7ccfb9f9de
--- /dev/null
+++ b/packages/components/src/spectrum/picker/PickerFromItems.tsx
@@ -0,0 +1,146 @@
+import { useCallback, useMemo } from 'react';
+import { DOMRef } from '@react-types/shared';
+import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
+import { EMPTY_FUNCTION } from '@deephaven/utils';
+import cl from 'classnames';
+import {
+  isNormalizedSection,
+  NormalizedSpectrumPickerProps,
+  normalizeTooltipOptions,
+  NormalizedItem,
+  TooltipOptions,
+  ItemKey,
+  getItemKey,
+  NormalizedSection,
+} from '../utils/itemUtils';
+import { Section } from '../shared';
+import { useRenderNormalizedItem } from '../utils';
+import usePickerScrollOnOpen from './usePickerScrollOnOpen';
+
+export type PickerFromItemsProps = {
+  children: (normalizedItem: NormalizedItem) => JSX.Element;
+  items: (NormalizedItem | NormalizedSection)[];
+  /** Can be set to true or a TooltipOptions to enable item tooltips */
+  tooltip?: boolean | TooltipOptions;
+  /** The currently selected key in the collection (controlled). */
+  selectedKey?: ItemKey | null;
+  /** The initial selected key in the collection (uncontrolled). */
+  defaultSelectedKey?: ItemKey;
+  /** Function to retrieve initial scroll position when opening the picker */
+  getInitialScrollPosition?: () => Promise<number | null>;
+  /**
+   * Handler that is called when the selection change.
+   * Note that under the hood, this is just an alias for Spectrum's
+   * `onSelectionChange`. We are renaming for better consistency with other
+   * components.
+   */
+  onChange?: (key: ItemKey) => void;
+
+  /** Handler that is called when the picker is scrolled. */
+  onScroll?: (event: Event) => void;
+
+  /**
+   * Handler that is called when the selection changes.
+   * @deprecated Use `onChange` instead
+   */
+  onSelectionChange?: (key: ItemKey) => void;
+} /*
+ * Support remaining SpectrumPickerProps.
+ * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
+ * re-defined above to account for boolean types which aren't included in the
+ * React `Key` type, but are actually supported by the Spectrum Picker component.
+ */ & Omit<
+  NormalizedSpectrumPickerProps,
+  | 'children'
+  | 'items'
+  | 'onSelectionChange'
+  | 'selectedKey'
+  | 'defaultSelectedKey'
+>;
+
+export function PickerFromItems({
+  children,
+  items,
+  tooltip = true,
+  defaultSelectedKey,
+  selectedKey,
+  getInitialScrollPosition,
+  onChange,
+  onOpenChange,
+  onScroll = EMPTY_FUNCTION,
+  onSelectionChange,
+  // eslint-disable-next-line camelcase
+  UNSAFE_className,
+  ...spectrumPickerProps
+}: PickerFromItemsProps): JSX.Element {
+  const tooltipOptions = useMemo(
+    () => normalizeTooltipOptions(tooltip),
+    [tooltip]
+  );
+
+  const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
+
+  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+    usePickerScrollOnOpen({
+      getInitialScrollPosition,
+      onScroll,
+      onOpenChange,
+    });
+
+  const onSelectionChangeInternal = useCallback(
+    (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 = items.find(item => String(getItemKey(item)) === key);
+
+      const actualKey = getItemKey(selectedItem) ?? key;
+
+      (onChange ?? onSelectionChange)?.(actualKey);
+    },
+    [items, onChange, onSelectionChange]
+  );
+
+  return (
+    <SpectrumPicker
+      // eslint-disable-next-line react/jsx-props-no-spreading
+      {...spectrumPickerProps}
+      ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
+      onOpenChange={onOpenChangeInternal}
+      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+      items={items}
+      // Spectrum Picker treats keys as strings if the `key` prop is explicitly
+      // set on `Item` elements. Since we do this in `renderItem`, we need to
+      // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
+      // for selection to work.
+      selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
+      defaultSelectedKey={
+        defaultSelectedKey == null
+          ? defaultSelectedKey
+          : defaultSelectedKey.toString()
+      }
+      // `onChange` is just an alias for `onSelectionChange`
+      onSelectionChange={
+        onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
+      }
+    >
+      {itemOrSection => {
+        if (isNormalizedSection(itemOrSection)) {
+          return (
+            <Section
+              key={getItemKey(itemOrSection)}
+              title={itemOrSection.item?.title}
+              items={itemOrSection.item?.items}
+            >
+              {renderNormalizedItem}
+            </Section>
+          );
+        }
+
+        return renderNormalizedItem(itemOrSection);
+      }}
+    </SpectrumPicker>
+  );
+}
+
+export default PickerFromItems;
diff --git a/packages/components/src/spectrum/picker/index.ts b/packages/components/src/spectrum/picker/index.ts
index c434d5d810..b3fa6ecaed 100644
--- a/packages/components/src/spectrum/picker/index.ts
+++ b/packages/components/src/spectrum/picker/index.ts
@@ -1 +1,3 @@
 export * from './Picker';
+export * from './PickerFromChildren';
+export * from './PickerFromItems';
diff --git a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
new file mode 100644
index 0000000000..635b4adec6
--- /dev/null
+++ b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
@@ -0,0 +1,46 @@
+import { useCallback } from 'react';
+import type { DOMRefValue } from '@react-types/shared';
+import {
+  findSpectrumPickerScrollArea,
+  usePopoverOnScrollRef,
+} from '@deephaven/react-hooks';
+
+export interface UsePickerScrollOnOpenOptions {
+  getInitialScrollPosition?: () => Promise<number | null | undefined>;
+  onScroll: (event: Event) => void;
+  onOpenChange?: (isOpen: boolean) => void;
+}
+
+export interface UsePickerScrollOnOpenResult {
+  ref: React.RefObject<DOMRefValue<HTMLElement>>;
+  onOpenChange: (isOpen: boolean) => void;
+}
+
+export function usePickerScrollOnOpen({
+  getInitialScrollPosition,
+  onScroll,
+  onOpenChange,
+}: UsePickerScrollOnOpenOptions): UsePickerScrollOnOpenResult {
+  const { ref, onOpenChange: popoverOnOpenChange } = usePopoverOnScrollRef(
+    findSpectrumPickerScrollArea,
+    onScroll,
+    getInitialScrollPosition
+  );
+
+  const onOpenChangeInternal = useCallback(
+    (isOpen: boolean): void => {
+      // Attach scroll event handling
+      popoverOnOpenChange(isOpen);
+
+      onOpenChange?.(isOpen);
+    },
+    [onOpenChange, popoverOnOpenChange]
+  );
+
+  return {
+    ref,
+    onOpenChange: onOpenChangeInternal,
+  };
+}
+
+export default usePickerScrollOnOpen;
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index e59e77f75e..04160a86aa 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -3,7 +3,7 @@ import { SpectrumPickerProps } from '@adobe/react-spectrum';
 import type { ItemRenderer } from '@react-types/shared';
 import Log from '@deephaven/log';
 import { isElementOfType } from '@deephaven/react-hooks';
-import { KeyedItem, PICKER_ITEM_HEIGHTS, SelectionT } from '@deephaven/utils';
+import { KeyedItem, SelectionT } from '@deephaven/utils';
 import { Item, ItemProps, Section, SectionProps } from '../shared';
 import { PopperOptions } from '../../popper';
 import { Text } from '../Text';
@@ -99,7 +99,7 @@ export function getItemKey<
   return (item?.item?.key ?? item?.key) as TKey;
 }
 
-export async function getPositionOfSelectedItem<
+export async function getPositionOfSelectedItemElement<
   TKey extends string | number | boolean | undefined,
 >({
   children,
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index 43f741870c..e43d8e27f7 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -1,7 +1,7 @@
 import {
   NormalizedItemData,
-  Picker as PickerBase,
-  PickerProps as PickerPropsBase,
+  PickerBase,
+  PickerBaseProps,
 } from '@deephaven/components';
 import { dh as DhType } from '@deephaven/jsapi-types';
 import { Settings } from '@deephaven/jsapi-utils';
@@ -16,7 +16,7 @@ import { useItemRowDeserializer } from './utils/useItemRowDeserializer';
 
 const log = Log.module('jsapi-components.Picker');
 
-export interface PickerProps extends Omit<PickerPropsBase, 'children'> {
+export interface PickerProps extends Omit<PickerBaseProps, 'children'> {
   table: DhType.Table;
   /* The column of values to use as item keys. Defaults to the first column. */
   keyColumn?: string;
diff --git a/packages/react-hooks/src/usePopoverOnScrollRef.ts b/packages/react-hooks/src/usePopoverOnScrollRef.ts
index 688eac9145..60d58501e8 100644
--- a/packages/react-hooks/src/usePopoverOnScrollRef.ts
+++ b/packages/react-hooks/src/usePopoverOnScrollRef.ts
@@ -21,7 +21,7 @@ export interface UsePopoverOnScrollRefResult<T> {
 export function usePopoverOnScrollRef<T>(
   findScrollArea: (ref: T | null) => HTMLElement | null,
   onScroll: (event: Event) => void,
-  getInitialScrollPosition?: () => Promise<number | null>
+  getInitialScrollPosition?: () => Promise<number | null | undefined>
 ): UsePopoverOnScrollRefResult<T> {
   const ref = useRef<T>(null);
   const isScrollOnOpenEnabledRef = useRef(false);

From 7664794df58c4635ec339dc6c22daec03c28b79f Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 14:19:17 -0500
Subject: [PATCH 05/30] WIP (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx | 300 ++++++++----------
 .../spectrum/picker/PickerFromChildren.tsx    | 192 +++++------
 .../src/spectrum/picker/PickerFromItems.tsx   | 209 ++++++------
 .../src/spectrum/picker/PickerModel.ts        |  37 +++
 .../components/src/spectrum/picker/index.ts   |   1 +
 .../jsapi-components/src/spectrum/Picker.tsx  |  10 +-
 6 files changed, 341 insertions(+), 408 deletions(-)
 create mode 100644 packages/components/src/spectrum/picker/PickerModel.ts

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index d556c8c0c1..e0f3bcb073 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -1,72 +1,31 @@
-import { useCallback, useMemo, useState } from 'react';
-import { DOMRef } from '@react-types/shared';
-import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
-import {
-  findSpectrumPickerScrollArea,
-  usePopoverOnScrollRef,
-} from '@deephaven/react-hooks';
-import {
-  EMPTY_FUNCTION,
-  PICKER_ITEM_HEIGHTS,
-  PICKER_TOP_OFFSET,
-} from '@deephaven/utils';
-import cl from 'classnames';
+import { useMemo } from 'react';
+
 import {
-  isNormalizedSection,
-  NormalizedSpectrumPickerProps,
-  normalizeItemList,
   normalizeTooltipOptions,
-  NormalizedItem,
-  ItemOrSection,
   TooltipOptions,
-  ItemKey,
-  getItemKey,
-  getPositionOfSelectedItemElement,
   isNormalizedItemsWithKeysList,
 } from '../utils/itemUtils';
-import { Section } from '../shared';
-import { useRenderNormalizedItem } from '../utils';
-import { wrapItemChildren } from '../utils/itemWrapperUtils';
+import { PickerCommonProps } from './PickerModel';
+import {
+  PickerFromChildrenProps,
+  PickerFromChildren,
+} from './PickerFromChildren';
+import PickerFromItems, { PickerFromItemsProps } from './PickerFromItems';
 
 export type PickerProps = {
-  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?: ItemKey | null;
-  /** The initial selected key in the collection (uncontrolled). */
-  defaultSelectedKey?: ItemKey;
-  /** Function to retrieve initial scroll position when opening the picker */
-  getInitialScrollPosition?: () => Promise<number | null>;
-  /**
-   * Handler that is called when the selection change.
-   * Note that under the hood, this is just an alias for Spectrum's
-   * `onSelectionChange`. We are renaming for better consistency with other
-   * components.
-   */
-  onChange?: (key: ItemKey) => void;
-
-  /** Handler that is called when the picker is scrolled. */
-  onScroll?: (event: Event) => void;
 
-  /**
-   * Handler that is called when the selection changes.
-   * @deprecated Use `onChange` instead
-   */
-  onSelectionChange?: (key: ItemKey) => void;
-} /*
- * Support remaining SpectrumPickerProps.
- * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
- * re-defined above to account for boolean types which aren't included in the
- * React `Key` type, but are actually supported by the Spectrum Picker component.
- */ & Omit<
-  NormalizedSpectrumPickerProps,
-  | 'children'
-  | 'items'
-  | 'onSelectionChange'
-  | 'selectedKey'
-  | 'defaultSelectedKey'
->;
+  /** Function to retrieve initial scroll position when opening the picker */
+  // getInitialScrollPosition?: () => Promise<number | null>;
+} & PickerCommonProps &
+  (
+    | { children: PickerFromChildrenProps['children'] }
+    | {
+        children: PickerFromItemsProps['items'];
+        getInitialScrollPosition: () => Promise<number | null | undefined>;
+      }
+  );
 
 /**
  * Picker component for selecting items from a list of items. Items can be
@@ -78,134 +37,123 @@ export type PickerProps = {
 export function Picker({
   children,
   tooltip = true,
-  defaultSelectedKey,
-  selectedKey,
-  getInitialScrollPosition,
-  onChange,
-  onOpenChange,
-  onScroll = EMPTY_FUNCTION,
-  onSelectionChange,
+  // defaultSelectedKey,
+  // selectedKey,
+  // onChange,
+  // onOpenChange,
+  // onSelectionChange,
   // eslint-disable-next-line camelcase
   UNSAFE_className,
-  ...spectrumPickerProps
+  ...props
 }: PickerProps): JSX.Element {
-  const normalizedItems = useMemo(
-    () => normalizeItemList(children),
-    [children]
-  );
-
   const tooltipOptions = useMemo(
     () => normalizeTooltipOptions(tooltip),
     [tooltip]
   );
 
-  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
-    useState(defaultSelectedKey);
-
-  const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
-
-  const getInitialScrollPositionInternal = useCallback(async () => {
-    if (getInitialScrollPosition != null) {
-      return getInitialScrollPosition();
-    }
-
-    if (isNormalizedItemsWithKeysList(children)) {
-      return null;
-    }
-
-    return getPositionOfSelectedItemElement({
-      children: wrapItemChildren(children, tooltipOptions),
-      itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
-      itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-      selectedKey: selectedKey ?? uncontrolledSelectedKey,
-      topOffset: PICKER_TOP_OFFSET,
-    });
-  }, [
-    children,
-    getInitialScrollPosition,
-    selectedKey,
-    tooltipOptions,
-    uncontrolledSelectedKey,
-  ]);
-
-  const { ref: scrollRef, onOpenChange: popoverOnOpenChange } =
-    usePopoverOnScrollRef(
-      findSpectrumPickerScrollArea,
-      onScroll,
-      getInitialScrollPositionInternal
-    );
-
-  const onOpenChangeInternal = useCallback(
-    (isOpen: boolean): void => {
-      // Attach scroll event handling
-      popoverOnOpenChange(isOpen);
-
-      onOpenChange?.(isOpen);
-    },
-    [onOpenChange, popoverOnOpenChange]
-  );
-
-  const onSelectionChangeInternal = useCallback(
-    (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(getItemKey(item)) === key
-      );
-
-      const actualKey = getItemKey(selectedItem) ?? key;
-
-      // If our component is uncontrolled, track the selected key internally
-      if (selectedKey == null) {
-        setUncontrolledSelectedKey(actualKey);
-      }
-
-      (onChange ?? onSelectionChange)?.(actualKey);
-    },
-    [normalizedItems, onChange, onSelectionChange, selectedKey]
-  );
-
-  return (
-    <SpectrumPicker
+  // const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
+  //   useState(defaultSelectedKey);
+
+  // const getInitialScrollPositionInternal = useCallback(async () => {
+  //   if (getInitialScrollPosition != null) {
+  //     return getInitialScrollPosition();
+  //   }
+
+  //   if (isNormalizedItemsWithKeysList(children)) {
+  //     return null;
+  //   }
+
+  //   return getPositionOfSelectedItemElement({
+  //     children: wrapItemChildren(children, tooltipOptions),
+  //     itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+  //     itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+  //     selectedKey: selectedKey ?? uncontrolledSelectedKey,
+  //     topOffset: PICKER_TOP_OFFSET,
+  //   });
+  // }, [
+  //   children,
+  //   getInitialScrollPosition,
+  //   selectedKey,
+  //   tooltipOptions,
+  //   uncontrolledSelectedKey,
+  // ]);
+
+  // const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+  //   usePickerScrollOnOpen({
+  //     getInitialScrollPosition: getInitialScrollPositionInternal,
+  //     onScroll,
+  //     onOpenChange,
+  //   });
+
+  // const onChangeInternal = useCallback(
+  //   (key: ItemKey): void => {
+  //     // If our component is uncontrolled, track the selected key internally
+  //     if (selectedKey == null) {
+  //       setUncontrolledSelectedKey(key);
+  //     }
+
+  //     (onChange ?? onSelectionChange)?.(key);
+  //   },
+  //   [onChange, onSelectionChange, selectedKey]
+  // );
+
+  return isNormalizedItemsWithKeysList(children) ? (
+    <PickerFromItems
       // eslint-disable-next-line react/jsx-props-no-spreading
-      {...spectrumPickerProps}
-      ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
-      onOpenChange={onOpenChangeInternal}
-      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-      items={normalizedItems}
-      // Spectrum Picker treats keys as strings if the `key` prop is explicitly
-      // set on `Item` elements. Since we do this in `renderItem`, we need to
-      // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
-      // for selection to work.
-      selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
-      defaultSelectedKey={
-        defaultSelectedKey == null
-          ? defaultSelectedKey
-          : defaultSelectedKey.toString()
-      }
-      // `onChange` is just an alias for `onSelectionChange`
-      onSelectionChange={
-        onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
-      }
+      {...(props as PickerFromItemsProps)}
+      items={children}
+      tooltipOptions={tooltipOptions}
+    />
+  ) : (
+    <PickerFromChildren
+      // eslint-disable-next-line react/jsx-props-no-spreading
+      {...props}
+      tooltipOptions={tooltipOptions}
     >
-      {itemOrSection => {
-        if (isNormalizedSection(itemOrSection)) {
-          return (
-            <Section
-              key={getItemKey(itemOrSection)}
-              title={itemOrSection.item?.title}
-              items={itemOrSection.item?.items}
-            >
-              {renderNormalizedItem}
-            </Section>
-          );
-        }
-
-        return renderNormalizedItem(itemOrSection);
-      }}
-    </SpectrumPicker>
+      {children}
+    </PickerFromChildren>
   );
+
+  // return (
+  //   <SpectrumPicker
+  //     // eslint-disable-next-line react/jsx-props-no-spreading
+  //     {...spectrumPickerProps}
+  //     ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
+  //     onOpenChange={onOpenChangeInternal}
+  //     UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+  //     items={normalizedItems}
+  //     // Spectrum Picker treats keys as strings if the `key` prop is explicitly
+  //     // set on `Item` elements. Since we do this in `renderItem`, we need to
+  //     // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
+  //     // for selection to work.
+  //     selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
+  //     defaultSelectedKey={
+  //       defaultSelectedKey == null
+  //         ? defaultSelectedKey
+  //         : defaultSelectedKey.toString()
+  //     }
+  //     // `onChange` is just an alias for `onSelectionChange`
+  //     onSelectionChange={
+  //       onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
+  //     }
+  //   >
+  //     {itemOrSection => {
+  //       if (isNormalizedSection(itemOrSection)) {
+  //         return (
+  //           <Section
+  //             key={getItemKey(itemOrSection)}
+  //             title={itemOrSection.item?.title}
+  //             items={itemOrSection.item?.items}
+  //           >
+  //             {renderNormalizedItem}
+  //           </Section>
+  //         );
+  //       }
+
+  //       return renderNormalizedItem(itemOrSection);
+  //     }}
+  //   </SpectrumPicker>
+  // );
 }
 
 export default Picker;
diff --git a/packages/components/src/spectrum/picker/PickerFromChildren.tsx b/packages/components/src/spectrum/picker/PickerFromChildren.tsx
index 499d031c66..5cac902ca0 100644
--- a/packages/components/src/spectrum/picker/PickerFromChildren.tsx
+++ b/packages/components/src/spectrum/picker/PickerFromChildren.tsx
@@ -1,137 +1,109 @@
-import { useCallback, useMemo, useState } from 'react';
-import { DOMRef } from '@react-types/shared';
+import { forwardRef, useCallback, useMemo, useState } from 'react';
+import type { DOMRef, DOMRefValue } from '@react-types/shared';
 import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
-
+import cl from 'classnames';
 import {
   EMPTY_FUNCTION,
   PICKER_ITEM_HEIGHTS,
   PICKER_TOP_OFFSET,
 } from '@deephaven/utils';
-import cl from 'classnames';
 import {
   NormalizedSpectrumPickerProps,
-  normalizeTooltipOptions,
   ItemOrSection,
   TooltipOptions,
-  ItemKey,
   getPositionOfSelectedItemElement,
+  ItemKey,
 } from '../utils/itemUtils';
 import { wrapItemChildren } from '../utils/itemWrapperUtils';
+import { PickerCommonProps } from './PickerModel';
 import usePickerScrollOnOpen from './usePickerScrollOnOpen';
 
-export type PickerFromChildrenProps = {
+export interface PickerFromChildrenProps extends PickerCommonProps {
   children: ItemOrSection | ItemOrSection[];
-  /** Can be set to true or a TooltipOptions to enable item tooltips */
-  tooltip?: boolean | TooltipOptions;
-  /** The currently selected key in the collection (controlled). */
-  selectedKey?: ItemKey | null;
-  /** The initial selected key in the collection (uncontrolled). */
-  defaultSelectedKey?: ItemKey;
-  /**
-   * Handler that is called when the selection change.
-   * Note that under the hood, this is just an alias for Spectrum's
-   * `onSelectionChange`. We are renaming for better consistency with other
-   * components.
-   */
-  onChange?: (key: ItemKey) => void;
-
-  /** Handler that is called when the picker is scrolled. */
-  onScroll?: (event: Event) => void;
 
-  /**
-   * Handler that is called when the selection changes.
-   * @deprecated Use `onChange` instead
-   */
-  onSelectionChange?: (key: ItemKey) => void;
-} /*
- * Support remaining SpectrumPickerProps.
- * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
- * re-defined above to account for boolean types which aren't included in the
- * React `Key` type, but are actually supported by the Spectrum Picker component.
- */ & Omit<
-  NormalizedSpectrumPickerProps,
-  | 'children'
-  | 'items'
-  | 'onSelectionChange'
-  | 'selectedKey'
-  | 'defaultSelectedKey'
->;
+  tooltipOptions: TooltipOptions | null;
+}
 
-export function PickerFromChildren({
-  children,
-  tooltip = true,
-  defaultSelectedKey,
-  selectedKey,
-  onChange,
-  onOpenChange,
-  onScroll = EMPTY_FUNCTION,
-  onSelectionChange,
-  // eslint-disable-next-line camelcase
-  UNSAFE_className,
-  ...spectrumPickerProps
-}: PickerFromChildrenProps): JSX.Element {
-  const tooltipOptions = useMemo(
-    () => normalizeTooltipOptions(tooltip),
-    [tooltip]
-  );
+export const PickerFromChildren = forwardRef<
+  DOMRefValue<HTMLElement>,
+  PickerFromChildrenProps
+>(
+  (
+    {
+      children,
+      tooltipOptions,
+      defaultSelectedKey,
+      selectedKey,
+      onChange,
+      onOpenChange,
+      onScroll = EMPTY_FUNCTION,
+      onSelectionChange,
+      // eslint-disable-next-line camelcase
+      UNSAFE_className,
+      ...spectrumPickerProps
+    }: PickerFromChildrenProps,
+    forwardedRef
+  ): JSX.Element => {
+    const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
+      useState(defaultSelectedKey);
 
-  const wrappedItems = useMemo(
-    () => wrapItemChildren(children, tooltipOptions),
-    [children, tooltipOptions]
-  );
+    const wrappedItems = useMemo(
+      () => wrapItemChildren(children, tooltipOptions),
+      [children, tooltipOptions]
+    );
 
-  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
-    useState(defaultSelectedKey);
+    const getInitialScrollPosition = useCallback(
+      async () =>
+        getPositionOfSelectedItemElement({
+          children: wrappedItems,
+          itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+          itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+          selectedKey: selectedKey ?? uncontrolledSelectedKey,
+          topOffset: PICKER_TOP_OFFSET,
+        }),
+      [selectedKey, uncontrolledSelectedKey, wrappedItems]
+    );
 
-  const getInitialScrollPosition = useCallback(
-    async () =>
-      getPositionOfSelectedItemElement({
-        children: wrappedItems,
-        itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
-        itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-        selectedKey: selectedKey ?? uncontrolledSelectedKey,
-        topOffset: PICKER_TOP_OFFSET,
-      }),
-    [selectedKey, uncontrolledSelectedKey, wrappedItems]
-  );
+    const onSelectionChangeInternal = useCallback(
+      (key: ItemKey): void => {
+        // If our component is uncontrolled, track the selected key internally
+        if (selectedKey == null) {
+          setUncontrolledSelectedKey(key);
+        }
 
-  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
-    usePickerScrollOnOpen({
-      getInitialScrollPosition,
-      onScroll,
-      onOpenChange,
-    });
+        (onChange ?? onSelectionChange)?.(key);
+      },
+      [onChange, onSelectionChange, selectedKey]
+    );
 
-  const onSelectionChangeInternal = useCallback(
-    (key: ItemKey): void => {
-      // If our component is uncontrolled, track the selected key internally
-      if (selectedKey == null) {
-        setUncontrolledSelectedKey(key);
-      }
+    const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+      usePickerScrollOnOpen({
+        getInitialScrollPosition,
+        onScroll,
+        onOpenChange,
+      });
 
-      (onChange ?? onSelectionChange)?.(key);
-    },
-    [onChange, onSelectionChange, selectedKey]
-  );
+    return (
+      <SpectrumPicker
+        // eslint-disable-next-line react/jsx-props-no-spreading
+        {...spectrumPickerProps}
+        ref={scrollRef as DOMRef<HTMLDivElement>}
+        UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+        selectedKey={
+          selectedKey as NormalizedSpectrumPickerProps['selectedKey']
+        }
+        defaultSelectedKey={
+          defaultSelectedKey as NormalizedSpectrumPickerProps['defaultSelectedKey']
+        }
+        onSelectionChange={onSelectionChangeInternal}
+        onOpenChange={onOpenChangeInternal}
+      >
+        {wrappedItems}
+      </SpectrumPicker>
+    );
+  }
+);
 
-  return (
-    <SpectrumPicker
-      // eslint-disable-next-line react/jsx-props-no-spreading
-      {...spectrumPickerProps}
-      ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
-      onOpenChange={onOpenChangeInternal}
-      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-      selectedKey={selectedKey as NormalizedSpectrumPickerProps['selectedKey']}
-      defaultSelectedKey={
-        defaultSelectedKey as NormalizedSpectrumPickerProps['defaultSelectedKey']
-      }
-      onSelectionChange={
-        onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
-      }
-    >
-      {wrappedItems}
-    </SpectrumPicker>
-  );
-}
+PickerFromChildren.displayName = 'PickerFromChildren';
 
 export default PickerFromChildren;
diff --git a/packages/components/src/spectrum/picker/PickerFromItems.tsx b/packages/components/src/spectrum/picker/PickerFromItems.tsx
index 7ccfb9f9de..e4d8c8d09f 100644
--- a/packages/components/src/spectrum/picker/PickerFromItems.tsx
+++ b/packages/components/src/spectrum/picker/PickerFromItems.tsx
@@ -1,12 +1,11 @@
-import { useCallback, useMemo } from 'react';
-import { DOMRef } from '@react-types/shared';
+import { forwardRef, useCallback } from 'react';
+import type { DOMRef, DOMRefValue } from '@react-types/shared';
 import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
-import { EMPTY_FUNCTION } from '@deephaven/utils';
 import cl from 'classnames';
+import { EMPTY_FUNCTION } from '@deephaven/utils';
 import {
   isNormalizedSection,
   NormalizedSpectrumPickerProps,
-  normalizeTooltipOptions,
   NormalizedItem,
   TooltipOptions,
   ItemKey,
@@ -15,132 +14,104 @@ import {
 } from '../utils/itemUtils';
 import { Section } from '../shared';
 import { useRenderNormalizedItem } from '../utils';
+import { PickerCommonProps } from './PickerModel';
 import usePickerScrollOnOpen from './usePickerScrollOnOpen';
 
-export type PickerFromItemsProps = {
-  children: (normalizedItem: NormalizedItem) => JSX.Element;
+export interface PickerFromItemsProps extends PickerCommonProps {
   items: (NormalizedItem | NormalizedSection)[];
-  /** Can be set to true or a TooltipOptions to enable item tooltips */
-  tooltip?: boolean | TooltipOptions;
-  /** The currently selected key in the collection (controlled). */
-  selectedKey?: ItemKey | null;
-  /** The initial selected key in the collection (uncontrolled). */
-  defaultSelectedKey?: ItemKey;
-  /** Function to retrieve initial scroll position when opening the picker */
-  getInitialScrollPosition?: () => Promise<number | null>;
-  /**
-   * Handler that is called when the selection change.
-   * Note that under the hood, this is just an alias for Spectrum's
-   * `onSelectionChange`. We are renaming for better consistency with other
-   * components.
-   */
-  onChange?: (key: ItemKey) => void;
-
-  /** Handler that is called when the picker is scrolled. */
-  onScroll?: (event: Event) => void;
-
-  /**
-   * Handler that is called when the selection changes.
-   * @deprecated Use `onChange` instead
-   */
-  onSelectionChange?: (key: ItemKey) => void;
-} /*
- * Support remaining SpectrumPickerProps.
- * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
- * re-defined above to account for boolean types which aren't included in the
- * React `Key` type, but are actually supported by the Spectrum Picker component.
- */ & Omit<
-  NormalizedSpectrumPickerProps,
-  | 'children'
-  | 'items'
-  | 'onSelectionChange'
-  | 'selectedKey'
-  | 'defaultSelectedKey'
->;
-
-export function PickerFromItems({
-  children,
-  items,
-  tooltip = true,
-  defaultSelectedKey,
-  selectedKey,
-  getInitialScrollPosition,
-  onChange,
-  onOpenChange,
-  onScroll = EMPTY_FUNCTION,
-  onSelectionChange,
-  // eslint-disable-next-line camelcase
-  UNSAFE_className,
-  ...spectrumPickerProps
-}: PickerFromItemsProps): JSX.Element {
-  const tooltipOptions = useMemo(
-    () => normalizeTooltipOptions(tooltip),
-    [tooltip]
-  );
-
-  const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
+  tooltipOptions: TooltipOptions | null;
+  getInitialScrollPosition: () => Promise<number | null | undefined>;
+}
 
-  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
-    usePickerScrollOnOpen({
+export const PickerFromItems = forwardRef<
+  DOMRefValue<HTMLElement>,
+  PickerFromItemsProps
+>(
+  (
+    {
+      items,
+      tooltipOptions,
+      defaultSelectedKey,
+      selectedKey,
       getInitialScrollPosition,
-      onScroll,
+      onChange,
       onOpenChange,
-    });
+      onScroll = EMPTY_FUNCTION,
+      onSelectionChange,
+      // eslint-disable-next-line camelcase
+      UNSAFE_className,
+      ...spectrumPickerProps
+    }: PickerFromItemsProps,
+    forwardedRef
+  ): JSX.Element => {
+    const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
+
+    const onSelectionChangeInternal = useCallback(
+      (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 = items.find(
+          item => String(getItemKey(item)) === key
+        );
 
-  const onSelectionChangeInternal = useCallback(
-    (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 = items.find(item => String(getItemKey(item)) === key);
+        const actualKey = getItemKey(selectedItem) ?? key;
 
-      const actualKey = getItemKey(selectedItem) ?? key;
+        (onChange ?? onSelectionChange)?.(actualKey);
+      },
+      [items, onChange, onSelectionChange]
+    );
 
-      (onChange ?? onSelectionChange)?.(actualKey);
-    },
-    [items, onChange, onSelectionChange]
-  );
+    const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+      usePickerScrollOnOpen({
+        getInitialScrollPosition,
+        onScroll,
+        onOpenChange,
+      });
 
-  return (
-    <SpectrumPicker
-      // eslint-disable-next-line react/jsx-props-no-spreading
-      {...spectrumPickerProps}
-      ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
-      onOpenChange={onOpenChangeInternal}
-      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-      items={items}
-      // Spectrum Picker treats keys as strings if the `key` prop is explicitly
-      // set on `Item` elements. Since we do this in `renderItem`, we need to
-      // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
-      // for selection to work.
-      selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
-      defaultSelectedKey={
-        defaultSelectedKey == null
-          ? defaultSelectedKey
-          : defaultSelectedKey.toString()
-      }
-      // `onChange` is just an alias for `onSelectionChange`
-      onSelectionChange={
-        onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
-      }
-    >
-      {itemOrSection => {
-        if (isNormalizedSection(itemOrSection)) {
-          return (
-            <Section
-              key={getItemKey(itemOrSection)}
-              title={itemOrSection.item?.title}
-              items={itemOrSection.item?.items}
-            >
-              {renderNormalizedItem}
-            </Section>
-          );
+    return (
+      <SpectrumPicker
+        // eslint-disable-next-line react/jsx-props-no-spreading
+        {...spectrumPickerProps}
+        ref={scrollRef as DOMRef<HTMLDivElement>}
+        UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+        items={items}
+        // Spectrum Picker treats keys as strings if the `key` prop is explicitly
+        // set on `Item` elements. Since we do this in `renderItem`, we need to
+        // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
+        // for selection to work.
+        selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
+        defaultSelectedKey={
+          defaultSelectedKey == null
+            ? defaultSelectedKey
+            : defaultSelectedKey.toString()
         }
+        // `onChange` is just an alias for `onSelectionChange`
+        onSelectionChange={
+          onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
+        }
+        onOpenChange={onOpenChangeInternal}
+      >
+        {itemOrSection => {
+          if (isNormalizedSection(itemOrSection)) {
+            return (
+              <Section
+                key={getItemKey(itemOrSection)}
+                title={itemOrSection.item?.title}
+                items={itemOrSection.item?.items}
+              >
+                {renderNormalizedItem}
+              </Section>
+            );
+          }
 
-        return renderNormalizedItem(itemOrSection);
-      }}
-    </SpectrumPicker>
-  );
-}
+          return renderNormalizedItem(itemOrSection);
+        }}
+      </SpectrumPicker>
+    );
+  }
+);
+
+PickerFromItems.displayName = 'PickerFromItems';
 
 export default PickerFromItems;
diff --git a/packages/components/src/spectrum/picker/PickerModel.ts b/packages/components/src/spectrum/picker/PickerModel.ts
new file mode 100644
index 0000000000..9121e2ad03
--- /dev/null
+++ b/packages/components/src/spectrum/picker/PickerModel.ts
@@ -0,0 +1,37 @@
+import type { ItemKey, NormalizedSpectrumPickerProps } from '../utils';
+
+export type PickerCommonProps = {
+  /** The currently selected key in the collection (controlled). */
+  selectedKey?: ItemKey | null;
+  /** The initial selected key in the collection (uncontrolled). */
+  defaultSelectedKey?: ItemKey;
+
+  /** Handler that is called when the picker is scrolled. */
+  onScroll?: (event: Event) => void;
+
+  /**
+   * Handler that is called when the selection change.
+   * Note that under the hood, this is just an alias for Spectrum's
+   * `onSelectionChange`. We are renaming for better consistency with other
+   * components.
+   */
+  onChange?: (key: ItemKey) => void;
+
+  /**
+   * Handler that is called when the selection changes.
+   * @deprecated Use `onChange` instead
+   */
+  onSelectionChange?: (key: ItemKey) => void;
+} & /*
+ * Support remaining SpectrumPickerProps.
+ * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
+ * re-defined above to account for boolean types which aren't included in the
+ * React `Key` type, but are actually supported by the Spectrum Picker component.
+ */ Omit<
+  NormalizedSpectrumPickerProps,
+  | 'children'
+  | 'items'
+  | 'onSelectionChange'
+  | 'selectedKey'
+  | 'defaultSelectedKey'
+>;
diff --git a/packages/components/src/spectrum/picker/index.ts b/packages/components/src/spectrum/picker/index.ts
index b3fa6ecaed..ab610984fc 100644
--- a/packages/components/src/spectrum/picker/index.ts
+++ b/packages/components/src/spectrum/picker/index.ts
@@ -1,3 +1,4 @@
 export * from './Picker';
 export * from './PickerFromChildren';
 export * from './PickerFromItems';
+export * from './PickerModel';
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index e43d8e27f7..0222c3e863 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -1,13 +1,13 @@
 import {
   NormalizedItemData,
-  PickerBase,
-  PickerBaseProps,
+  Picker as PickerBase,
+  PickerProps as PickerBaseProps,
 } from '@deephaven/components';
 import { dh as DhType } from '@deephaven/jsapi-types';
 import { Settings } from '@deephaven/jsapi-utils';
 import Log from '@deephaven/log';
 import { PICKER_ITEM_HEIGHTS, PICKER_TOP_OFFSET } from '@deephaven/utils';
-import { useCallback, useEffect, useMemo } from 'react';
+import { useCallback, useEffect, useMemo, useState } from 'react';
 import useFormatter from '../useFormatter';
 import useGetItemIndexByValue from '../useGetItemIndexByValue';
 import { useViewportData } from '../useViewportData';
@@ -38,6 +38,10 @@ export function Picker({
 }: PickerProps): JSX.Element {
   const { getFormattedString: formatValue } = useFormatter(settings);
 
+  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = useState(
+    props.defaultSelectedKey
+  );
+
   const keyColumn = useMemo(
     () => getItemKeyColumn(table, keyColumnName),
     [keyColumnName, table]

From 3fa1fc5436e21c70614af2b0f3c741b046a6d037 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 15:16:56 -0500
Subject: [PATCH 06/30] Cleanup (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx | 201 +++++++-----------
 .../spectrum/picker/PickerFromChildren.tsx    | 109 ----------
 .../src/spectrum/picker/PickerFromItems.tsx   | 117 ----------
 .../src/spectrum/picker/PickerModel.ts        |  10 +-
 .../src/spectrum/picker/Picker_old.tsx        | 156 ++++++++++++++
 .../components/src/spectrum/picker/index.ts   |   3 +-
 .../jsapi-components/src/spectrum/Picker.tsx  | 107 ++++++++--
 7 files changed, 331 insertions(+), 372 deletions(-)
 delete mode 100644 packages/components/src/spectrum/picker/PickerFromChildren.tsx
 delete mode 100644 packages/components/src/spectrum/picker/PickerFromItems.tsx
 create mode 100644 packages/components/src/spectrum/picker/Picker_old.tsx

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index e0f3bcb073..4c3492b2fc 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -1,159 +1,100 @@
-import { useMemo } from 'react';
-
+import { useCallback, useMemo, useState } from 'react';
+import type { DOMRef } from '@react-types/shared';
+import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
+import cl from 'classnames';
+import {
+  EMPTY_FUNCTION,
+  PICKER_ITEM_HEIGHTS,
+  PICKER_TOP_OFFSET,
+} from '@deephaven/utils';
 import {
+  NormalizedSpectrumPickerProps,
+  ItemOrSection,
+  getPositionOfSelectedItemElement,
+  ItemKey,
   normalizeTooltipOptions,
-  TooltipOptions,
-  isNormalizedItemsWithKeysList,
 } from '../utils/itemUtils';
+import { wrapItemChildren } from '../utils/itemWrapperUtils';
 import { PickerCommonProps } from './PickerModel';
-import {
-  PickerFromChildrenProps,
-  PickerFromChildren,
-} from './PickerFromChildren';
-import PickerFromItems, { PickerFromItemsProps } from './PickerFromItems';
-
-export type PickerProps = {
-  /** Can be set to true or a TooltipOptions to enable item tooltips */
-  tooltip?: boolean | TooltipOptions;
+import usePickerScrollOnOpen from './usePickerScrollOnOpen';
 
-  /** Function to retrieve initial scroll position when opening the picker */
-  // getInitialScrollPosition?: () => Promise<number | null>;
-} & PickerCommonProps &
-  (
-    | { children: PickerFromChildrenProps['children'] }
-    | {
-        children: PickerFromItemsProps['items'];
-        getInitialScrollPosition: () => Promise<number | null | undefined>;
-      }
-  );
+export interface PickerProps extends PickerCommonProps {
+  children: ItemOrSection | ItemOrSection[];
+}
 
-/**
- * Picker component for selecting items from a list of items. Items can be
- * provided via the `items` prop or as children. Each item can be a string,
- * number, boolean, or a Spectrum <Item> element. The remaining props are just
- * pass through props for the Spectrum Picker component.
- * See https://react-spectrum.adobe.com/react-spectrum/Picker.html
- */
 export function Picker({
   children,
   tooltip = true,
-  // defaultSelectedKey,
-  // selectedKey,
-  // onChange,
-  // onOpenChange,
-  // onSelectionChange,
+  defaultSelectedKey,
+  selectedKey,
+  onChange,
+  onOpenChange,
+  onScroll = EMPTY_FUNCTION,
+  onSelectionChange,
   // eslint-disable-next-line camelcase
   UNSAFE_className,
-  ...props
+  ...spectrumPickerProps
 }: PickerProps): JSX.Element {
   const tooltipOptions = useMemo(
     () => normalizeTooltipOptions(tooltip),
     [tooltip]
   );
 
-  // const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
-  //   useState(defaultSelectedKey);
-
-  // const getInitialScrollPositionInternal = useCallback(async () => {
-  //   if (getInitialScrollPosition != null) {
-  //     return getInitialScrollPosition();
-  //   }
+  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
+    useState(defaultSelectedKey);
 
-  //   if (isNormalizedItemsWithKeysList(children)) {
-  //     return null;
-  //   }
+  const wrappedItems = useMemo(
+    () => wrapItemChildren(children, tooltipOptions),
+    [children, tooltipOptions]
+  );
 
-  //   return getPositionOfSelectedItemElement({
-  //     children: wrapItemChildren(children, tooltipOptions),
-  //     itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
-  //     itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-  //     selectedKey: selectedKey ?? uncontrolledSelectedKey,
-  //     topOffset: PICKER_TOP_OFFSET,
-  //   });
-  // }, [
-  //   children,
-  //   getInitialScrollPosition,
-  //   selectedKey,
-  //   tooltipOptions,
-  //   uncontrolledSelectedKey,
-  // ]);
+  const getInitialScrollPosition = useCallback(
+    async () =>
+      getPositionOfSelectedItemElement({
+        children: wrappedItems,
+        itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+        itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+        selectedKey: selectedKey ?? uncontrolledSelectedKey,
+        topOffset: PICKER_TOP_OFFSET,
+      }),
+    [selectedKey, uncontrolledSelectedKey, wrappedItems]
+  );
 
-  // const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
-  //   usePickerScrollOnOpen({
-  //     getInitialScrollPosition: getInitialScrollPositionInternal,
-  //     onScroll,
-  //     onOpenChange,
-  //   });
+  const onSelectionChangeInternal = useCallback(
+    (key: ItemKey): void => {
+      // If our component is uncontrolled, track the selected key internally
+      if (selectedKey == null) {
+        setUncontrolledSelectedKey(key);
+      }
 
-  // const onChangeInternal = useCallback(
-  //   (key: ItemKey): void => {
-  //     // If our component is uncontrolled, track the selected key internally
-  //     if (selectedKey == null) {
-  //       setUncontrolledSelectedKey(key);
-  //     }
+      (onChange ?? onSelectionChange)?.(key);
+    },
+    [onChange, onSelectionChange, selectedKey]
+  );
 
-  //     (onChange ?? onSelectionChange)?.(key);
-  //   },
-  //   [onChange, onSelectionChange, selectedKey]
-  // );
+  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+    usePickerScrollOnOpen({
+      getInitialScrollPosition,
+      onScroll,
+      onOpenChange,
+    });
 
-  return isNormalizedItemsWithKeysList(children) ? (
-    <PickerFromItems
+  return (
+    <SpectrumPicker
       // eslint-disable-next-line react/jsx-props-no-spreading
-      {...(props as PickerFromItemsProps)}
-      items={children}
-      tooltipOptions={tooltipOptions}
-    />
-  ) : (
-    <PickerFromChildren
-      // eslint-disable-next-line react/jsx-props-no-spreading
-      {...props}
-      tooltipOptions={tooltipOptions}
+      {...spectrumPickerProps}
+      ref={scrollRef as DOMRef<HTMLDivElement>}
+      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+      selectedKey={selectedKey as NormalizedSpectrumPickerProps['selectedKey']}
+      defaultSelectedKey={
+        defaultSelectedKey as NormalizedSpectrumPickerProps['defaultSelectedKey']
+      }
+      onSelectionChange={onSelectionChangeInternal}
+      onOpenChange={onOpenChangeInternal}
     >
-      {children}
-    </PickerFromChildren>
+      {wrappedItems}
+    </SpectrumPicker>
   );
-
-  // return (
-  //   <SpectrumPicker
-  //     // eslint-disable-next-line react/jsx-props-no-spreading
-  //     {...spectrumPickerProps}
-  //     ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
-  //     onOpenChange={onOpenChangeInternal}
-  //     UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-  //     items={normalizedItems}
-  //     // Spectrum Picker treats keys as strings if the `key` prop is explicitly
-  //     // set on `Item` elements. Since we do this in `renderItem`, we need to
-  //     // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
-  //     // for selection to work.
-  //     selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
-  //     defaultSelectedKey={
-  //       defaultSelectedKey == null
-  //         ? defaultSelectedKey
-  //         : defaultSelectedKey.toString()
-  //     }
-  //     // `onChange` is just an alias for `onSelectionChange`
-  //     onSelectionChange={
-  //       onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
-  //     }
-  //   >
-  //     {itemOrSection => {
-  //       if (isNormalizedSection(itemOrSection)) {
-  //         return (
-  //           <Section
-  //             key={getItemKey(itemOrSection)}
-  //             title={itemOrSection.item?.title}
-  //             items={itemOrSection.item?.items}
-  //           >
-  //             {renderNormalizedItem}
-  //           </Section>
-  //         );
-  //       }
-
-  //       return renderNormalizedItem(itemOrSection);
-  //     }}
-  //   </SpectrumPicker>
-  // );
 }
 
 export default Picker;
diff --git a/packages/components/src/spectrum/picker/PickerFromChildren.tsx b/packages/components/src/spectrum/picker/PickerFromChildren.tsx
deleted file mode 100644
index 5cac902ca0..0000000000
--- a/packages/components/src/spectrum/picker/PickerFromChildren.tsx
+++ /dev/null
@@ -1,109 +0,0 @@
-import { forwardRef, useCallback, useMemo, useState } from 'react';
-import type { DOMRef, DOMRefValue } from '@react-types/shared';
-import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
-import cl from 'classnames';
-import {
-  EMPTY_FUNCTION,
-  PICKER_ITEM_HEIGHTS,
-  PICKER_TOP_OFFSET,
-} from '@deephaven/utils';
-import {
-  NormalizedSpectrumPickerProps,
-  ItemOrSection,
-  TooltipOptions,
-  getPositionOfSelectedItemElement,
-  ItemKey,
-} from '../utils/itemUtils';
-import { wrapItemChildren } from '../utils/itemWrapperUtils';
-import { PickerCommonProps } from './PickerModel';
-import usePickerScrollOnOpen from './usePickerScrollOnOpen';
-
-export interface PickerFromChildrenProps extends PickerCommonProps {
-  children: ItemOrSection | ItemOrSection[];
-
-  tooltipOptions: TooltipOptions | null;
-}
-
-export const PickerFromChildren = forwardRef<
-  DOMRefValue<HTMLElement>,
-  PickerFromChildrenProps
->(
-  (
-    {
-      children,
-      tooltipOptions,
-      defaultSelectedKey,
-      selectedKey,
-      onChange,
-      onOpenChange,
-      onScroll = EMPTY_FUNCTION,
-      onSelectionChange,
-      // eslint-disable-next-line camelcase
-      UNSAFE_className,
-      ...spectrumPickerProps
-    }: PickerFromChildrenProps,
-    forwardedRef
-  ): JSX.Element => {
-    const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
-      useState(defaultSelectedKey);
-
-    const wrappedItems = useMemo(
-      () => wrapItemChildren(children, tooltipOptions),
-      [children, tooltipOptions]
-    );
-
-    const getInitialScrollPosition = useCallback(
-      async () =>
-        getPositionOfSelectedItemElement({
-          children: wrappedItems,
-          itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
-          itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-          selectedKey: selectedKey ?? uncontrolledSelectedKey,
-          topOffset: PICKER_TOP_OFFSET,
-        }),
-      [selectedKey, uncontrolledSelectedKey, wrappedItems]
-    );
-
-    const onSelectionChangeInternal = useCallback(
-      (key: ItemKey): void => {
-        // If our component is uncontrolled, track the selected key internally
-        if (selectedKey == null) {
-          setUncontrolledSelectedKey(key);
-        }
-
-        (onChange ?? onSelectionChange)?.(key);
-      },
-      [onChange, onSelectionChange, selectedKey]
-    );
-
-    const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
-      usePickerScrollOnOpen({
-        getInitialScrollPosition,
-        onScroll,
-        onOpenChange,
-      });
-
-    return (
-      <SpectrumPicker
-        // eslint-disable-next-line react/jsx-props-no-spreading
-        {...spectrumPickerProps}
-        ref={scrollRef as DOMRef<HTMLDivElement>}
-        UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-        selectedKey={
-          selectedKey as NormalizedSpectrumPickerProps['selectedKey']
-        }
-        defaultSelectedKey={
-          defaultSelectedKey as NormalizedSpectrumPickerProps['defaultSelectedKey']
-        }
-        onSelectionChange={onSelectionChangeInternal}
-        onOpenChange={onOpenChangeInternal}
-      >
-        {wrappedItems}
-      </SpectrumPicker>
-    );
-  }
-);
-
-PickerFromChildren.displayName = 'PickerFromChildren';
-
-export default PickerFromChildren;
diff --git a/packages/components/src/spectrum/picker/PickerFromItems.tsx b/packages/components/src/spectrum/picker/PickerFromItems.tsx
deleted file mode 100644
index e4d8c8d09f..0000000000
--- a/packages/components/src/spectrum/picker/PickerFromItems.tsx
+++ /dev/null
@@ -1,117 +0,0 @@
-import { forwardRef, useCallback } from 'react';
-import type { DOMRef, DOMRefValue } from '@react-types/shared';
-import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
-import cl from 'classnames';
-import { EMPTY_FUNCTION } from '@deephaven/utils';
-import {
-  isNormalizedSection,
-  NormalizedSpectrumPickerProps,
-  NormalizedItem,
-  TooltipOptions,
-  ItemKey,
-  getItemKey,
-  NormalizedSection,
-} from '../utils/itemUtils';
-import { Section } from '../shared';
-import { useRenderNormalizedItem } from '../utils';
-import { PickerCommonProps } from './PickerModel';
-import usePickerScrollOnOpen from './usePickerScrollOnOpen';
-
-export interface PickerFromItemsProps extends PickerCommonProps {
-  items: (NormalizedItem | NormalizedSection)[];
-  tooltipOptions: TooltipOptions | null;
-  getInitialScrollPosition: () => Promise<number | null | undefined>;
-}
-
-export const PickerFromItems = forwardRef<
-  DOMRefValue<HTMLElement>,
-  PickerFromItemsProps
->(
-  (
-    {
-      items,
-      tooltipOptions,
-      defaultSelectedKey,
-      selectedKey,
-      getInitialScrollPosition,
-      onChange,
-      onOpenChange,
-      onScroll = EMPTY_FUNCTION,
-      onSelectionChange,
-      // eslint-disable-next-line camelcase
-      UNSAFE_className,
-      ...spectrumPickerProps
-    }: PickerFromItemsProps,
-    forwardedRef
-  ): JSX.Element => {
-    const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
-
-    const onSelectionChangeInternal = useCallback(
-      (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 = items.find(
-          item => String(getItemKey(item)) === key
-        );
-
-        const actualKey = getItemKey(selectedItem) ?? key;
-
-        (onChange ?? onSelectionChange)?.(actualKey);
-      },
-      [items, onChange, onSelectionChange]
-    );
-
-    const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
-      usePickerScrollOnOpen({
-        getInitialScrollPosition,
-        onScroll,
-        onOpenChange,
-      });
-
-    return (
-      <SpectrumPicker
-        // eslint-disable-next-line react/jsx-props-no-spreading
-        {...spectrumPickerProps}
-        ref={scrollRef as DOMRef<HTMLDivElement>}
-        UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-        items={items}
-        // Spectrum Picker treats keys as strings if the `key` prop is explicitly
-        // set on `Item` elements. Since we do this in `renderItem`, we need to
-        // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
-        // for selection to work.
-        selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
-        defaultSelectedKey={
-          defaultSelectedKey == null
-            ? defaultSelectedKey
-            : defaultSelectedKey.toString()
-        }
-        // `onChange` is just an alias for `onSelectionChange`
-        onSelectionChange={
-          onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
-        }
-        onOpenChange={onOpenChangeInternal}
-      >
-        {itemOrSection => {
-          if (isNormalizedSection(itemOrSection)) {
-            return (
-              <Section
-                key={getItemKey(itemOrSection)}
-                title={itemOrSection.item?.title}
-                items={itemOrSection.item?.items}
-              >
-                {renderNormalizedItem}
-              </Section>
-            );
-          }
-
-          return renderNormalizedItem(itemOrSection);
-        }}
-      </SpectrumPicker>
-    );
-  }
-);
-
-PickerFromItems.displayName = 'PickerFromItems';
-
-export default PickerFromItems;
diff --git a/packages/components/src/spectrum/picker/PickerModel.ts b/packages/components/src/spectrum/picker/PickerModel.ts
index 9121e2ad03..14db9ff3a9 100644
--- a/packages/components/src/spectrum/picker/PickerModel.ts
+++ b/packages/components/src/spectrum/picker/PickerModel.ts
@@ -1,11 +1,19 @@
-import type { ItemKey, NormalizedSpectrumPickerProps } from '../utils';
+import type {
+  ItemKey,
+  NormalizedSpectrumPickerProps,
+  TooltipOptions,
+} from '../utils';
 
 export type PickerCommonProps = {
   /** The currently selected key in the collection (controlled). */
   selectedKey?: ItemKey | null;
+
   /** The initial selected key in the collection (uncontrolled). */
   defaultSelectedKey?: ItemKey;
 
+  /** Can be set to true or a TooltipOptions to enable item tooltips */
+  tooltip?: boolean | TooltipOptions;
+
   /** Handler that is called when the picker is scrolled. */
   onScroll?: (event: Event) => void;
 
diff --git a/packages/components/src/spectrum/picker/Picker_old.tsx b/packages/components/src/spectrum/picker/Picker_old.tsx
new file mode 100644
index 0000000000..7c7be51845
--- /dev/null
+++ b/packages/components/src/spectrum/picker/Picker_old.tsx
@@ -0,0 +1,156 @@
+import { useMemo } from 'react';
+
+import {
+  normalizeTooltipOptions,
+  TooltipOptions,
+  isNormalizedItemsWithKeysList,
+} from '../utils/itemUtils';
+import { PickerCommonProps } from './PickerModel';
+import { PickerFromChildrenProps, PickerFromChildren } from './Picker';
+import PickerFromItems, { PickerFromItemsProps } from './PickerFromItems';
+
+export type PickerProps = {
+  /** Can be set to true or a TooltipOptions to enable item tooltips */
+  tooltip?: boolean | TooltipOptions;
+
+  /** Function to retrieve initial scroll position when opening the picker */
+  // getInitialScrollPosition?: () => Promise<number | null>;
+} & PickerCommonProps &
+  (
+    | { children: PickerFromChildrenProps['children'] }
+    | {
+        children: PickerFromItemsProps['items'];
+        getInitialScrollPosition: () => Promise<number | null | undefined>;
+      }
+  );
+
+/**
+ * Picker component for selecting items from a list of items. Items can be
+ * provided via the `items` prop or as children. Each item can be a string,
+ * number, boolean, or a Spectrum <Item> element. The remaining props are just
+ * pass through props for the Spectrum Picker component.
+ * See https://react-spectrum.adobe.com/react-spectrum/Picker.html
+ */
+export function Picker({
+  children,
+  tooltip = true,
+  // defaultSelectedKey,
+  // selectedKey,
+  // onChange,
+  // onOpenChange,
+  // onSelectionChange,
+  // eslint-disable-next-line camelcase
+  UNSAFE_className,
+  ...props
+}: PickerProps): JSX.Element {
+  const tooltipOptions = useMemo(
+    () => normalizeTooltipOptions(tooltip),
+    [tooltip]
+  );
+
+  // const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
+  //   useState(defaultSelectedKey);
+
+  // const getInitialScrollPositionInternal = useCallback(async () => {
+  //   if (getInitialScrollPosition != null) {
+  //     return getInitialScrollPosition();
+  //   }
+
+  //   if (isNormalizedItemsWithKeysList(children)) {
+  //     return null;
+  //   }
+
+  //   return getPositionOfSelectedItemElement({
+  //     children: wrapItemChildren(children, tooltipOptions),
+  //     itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+  //     itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+  //     selectedKey: selectedKey ?? uncontrolledSelectedKey,
+  //     topOffset: PICKER_TOP_OFFSET,
+  //   });
+  // }, [
+  //   children,
+  //   getInitialScrollPosition,
+  //   selectedKey,
+  //   tooltipOptions,
+  //   uncontrolledSelectedKey,
+  // ]);
+
+  // const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+  //   usePickerScrollOnOpen({
+  //     getInitialScrollPosition: getInitialScrollPositionInternal,
+  //     onScroll,
+  //     onOpenChange,
+  //   });
+
+  // const onChangeInternal = useCallback(
+  //   (key: ItemKey): void => {
+  //     // If our component is uncontrolled, track the selected key internally
+  //     if (selectedKey == null) {
+  //       setUncontrolledSelectedKey(key);
+  //     }
+
+  //     (onChange ?? onSelectionChange)?.(key);
+  //   },
+  //   [onChange, onSelectionChange, selectedKey]
+  // );
+
+  return isNormalizedItemsWithKeysList(children) ? (
+    <PickerFromItems
+      // eslint-disable-next-line react/jsx-props-no-spreading
+      {...(props as PickerFromItemsProps)}
+      items={children}
+      tooltipOptions={tooltipOptions}
+    />
+  ) : (
+    <PickerFromChildren
+      // eslint-disable-next-line react/jsx-props-no-spreading
+      {...props}
+      tooltipOptions={tooltipOptions}
+    >
+      {children}
+    </PickerFromChildren>
+  );
+
+  // return (
+  //   <SpectrumPicker
+  //     // eslint-disable-next-line react/jsx-props-no-spreading
+  //     {...spectrumPickerProps}
+  //     ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
+  //     onOpenChange={onOpenChangeInternal}
+  //     UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+  //     items={normalizedItems}
+  //     // Spectrum Picker treats keys as strings if the `key` prop is explicitly
+  //     // set on `Item` elements. Since we do this in `renderItem`, we need to
+  //     // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
+  //     // for selection to work.
+  //     selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
+  //     defaultSelectedKey={
+  //       defaultSelectedKey == null
+  //         ? defaultSelectedKey
+  //         : defaultSelectedKey.toString()
+  //     }
+  //     // `onChange` is just an alias for `onSelectionChange`
+  //     onSelectionChange={
+  //       onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
+  //     }
+  //   >
+  //     {itemOrSection => {
+  //       if (isNormalizedSection(itemOrSection)) {
+  //         return (
+  //           <Section
+  //             key={getItemKey(itemOrSection)}
+  //             title={itemOrSection.item?.title}
+  //             items={itemOrSection.item?.items}
+  //           >
+  //             {renderNormalizedItem}
+  //           </Section>
+  //         );
+  //       }
+
+  //       return renderNormalizedItem(itemOrSection);
+  //     }}
+  //   </SpectrumPicker>
+  // );
+}
+
+export default Picker;
diff --git a/packages/components/src/spectrum/picker/index.ts b/packages/components/src/spectrum/picker/index.ts
index ab610984fc..623b4a4835 100644
--- a/packages/components/src/spectrum/picker/index.ts
+++ b/packages/components/src/spectrum/picker/index.ts
@@ -1,4 +1,3 @@
 export * from './Picker';
-export * from './PickerFromChildren';
-export * from './PickerFromItems';
 export * from './PickerModel';
+export * from './usePickerScrollOnOpen';
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index 0222c3e863..e7af339b2f 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -1,13 +1,25 @@
+import { useCallback, useEffect, useMemo, useState } from 'react';
+import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
+import type { DOMRef } from '@react-types/shared';
+import cl from 'classnames';
 import {
+  getItemKey,
+  isNormalizedSection,
+  ItemKey,
+  NormalizedItem,
   NormalizedItemData,
-  Picker as PickerBase,
+  NormalizedSection,
+  NormalizedSectionData,
+  normalizeTooltipOptions,
   PickerProps as PickerBaseProps,
+  Section,
+  usePickerScrollOnOpen,
+  useRenderNormalizedItem,
 } from '@deephaven/components';
 import { dh as DhType } from '@deephaven/jsapi-types';
 import { Settings } from '@deephaven/jsapi-utils';
 import Log from '@deephaven/log';
 import { PICKER_ITEM_HEIGHTS, PICKER_TOP_OFFSET } from '@deephaven/utils';
-import { useCallback, useEffect, useMemo, useState } from 'react';
 import useFormatter from '../useFormatter';
 import useGetItemIndexByValue from '../useGetItemIndexByValue';
 import { useViewportData } from '../useViewportData';
@@ -32,16 +44,26 @@ export function Picker({
   table,
   keyColumn: keyColumnName,
   labelColumn: labelColumnName,
-  selectedKey,
   settings,
+  tooltip = true,
+  selectedKey,
+  defaultSelectedKey,
+  UNSAFE_className,
+  onChange,
+  onOpenChange,
+  onSelectionChange,
   ...props
 }: PickerProps): JSX.Element {
   const { getFormattedString: formatValue } = useFormatter(settings);
 
-  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = useState(
-    props.defaultSelectedKey
+  const tooltipOptions = useMemo(
+    () => normalizeTooltipOptions(tooltip),
+    [tooltip]
   );
 
+  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
+    useState(defaultSelectedKey);
+
   const keyColumn = useMemo(
     () => getItemKeyColumn(table, keyColumnName),
     [keyColumnName, table]
@@ -57,7 +79,7 @@ export function Picker({
   const getItemIndexByValue = useGetItemIndexByValue({
     table,
     columnName: keyColumn.name,
-    value: selectedKey,
+    value: selectedKey ?? uncontrolledSelectedKey,
   });
 
   const getInitialScrollPosition = useCallback(async () => {
@@ -71,7 +93,7 @@ export function Picker({
   }, [getItemIndexByValue]);
 
   const { viewportData, onScroll, setViewport } = useViewportData<
-    NormalizedItemData,
+    NormalizedItemData | NormalizedSectionData,
     DhType.Table
   >({
     reuseItemsOnTableResize: true,
@@ -105,16 +127,75 @@ export function Picker({
     [getItemIndexByValue, settings, setViewport]
   );
 
+  const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
+
+  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+    usePickerScrollOnOpen({
+      getInitialScrollPosition,
+      onScroll,
+      onOpenChange,
+    });
+
+  const onSelectionChangeInternal = useCallback(
+    (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 = (
+        viewportData.items as (NormalizedItem | NormalizedSection)[]
+      ).find(item => String(getItemKey(item)) === key);
+
+      const actualKey = getItemKey(selectedItem) ?? key;
+
+      // If our component is uncontrolled, track the selected key internally
+      // so that we can scroll to the selected item if the user re-opens
+      if (selectedKey == null) {
+        setUncontrolledSelectedKey(key);
+      }
+
+      (onChange ?? onSelectionChange)?.(actualKey);
+    },
+    [viewportData.items, selectedKey, onChange, onSelectionChange]
+  );
+
   return (
-    <PickerBase
+    <SpectrumPicker
       // eslint-disable-next-line react/jsx-props-no-spreading
       {...props}
-      getInitialScrollPosition={getInitialScrollPosition}
-      selectedKey={selectedKey}
-      onScroll={onScroll}
+      ref={scrollRef as DOMRef<HTMLDivElement>}
+      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+      items={viewportData.items as (NormalizedItem | NormalizedSection)[]}
+      // Spectrum Picker treats keys as strings if the `key` prop is explicitly
+      // set on `Item` elements. Since we do this in `renderItem`, we need to
+      // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
+      // for selection to work.
+      selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
+      defaultSelectedKey={
+        defaultSelectedKey == null
+          ? defaultSelectedKey
+          : defaultSelectedKey.toString()
+      }
+      onSelectionChange={
+        onSelectionChangeInternal // as NormalizedSpectrumPickerProps['onSelectionChange']
+      }
+      onOpenChange={onOpenChangeInternal}
     >
-      {viewportData.items}
-    </PickerBase>
+      {itemOrSection => {
+        if (isNormalizedSection(itemOrSection)) {
+          return (
+            <Section
+              key={getItemKey(itemOrSection)}
+              title={itemOrSection.item?.title}
+              items={itemOrSection.item?.items}
+            >
+              {renderNormalizedItem}
+            </Section>
+          );
+        }
+
+        return renderNormalizedItem(itemOrSection);
+      }}
+    </SpectrumPicker>
   );
 }
 

From 0a2a23f19f677fdeb81b3d26c874bce7f75ca7b0 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 15:22:02 -0500
Subject: [PATCH 07/30] Cleanup (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx |  44 ++++-
 .../src/spectrum/picker/PickerModel.ts        |  45 -----
 .../src/spectrum/picker/Picker_old.tsx        | 156 ------------------
 .../components/src/spectrum/picker/index.ts   |   1 -
 .../jsapi-components/src/spectrum/Picker.tsx  |  15 +-
 5 files changed, 51 insertions(+), 210 deletions(-)
 delete mode 100644 packages/components/src/spectrum/picker/PickerModel.ts
 delete mode 100644 packages/components/src/spectrum/picker/Picker_old.tsx

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index 4c3492b2fc..e9ad91c67d 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -13,14 +13,52 @@ import {
   getPositionOfSelectedItemElement,
   ItemKey,
   normalizeTooltipOptions,
+  TooltipOptions,
 } from '../utils/itemUtils';
 import { wrapItemChildren } from '../utils/itemWrapperUtils';
-import { PickerCommonProps } from './PickerModel';
 import usePickerScrollOnOpen from './usePickerScrollOnOpen';
 
-export interface PickerProps extends PickerCommonProps {
+export type PickerProps = {
+  /** The currently selected key in the collection (controlled). */
+  selectedKey?: ItemKey | null;
+
+  /** The initial selected key in the collection (uncontrolled). */
+  defaultSelectedKey?: ItemKey;
+
+  /** Can be set to true or a TooltipOptions to enable item tooltips */
+  tooltip?: boolean | TooltipOptions;
+
+  /** Handler that is called when the picker is scrolled. */
+  onScroll?: (event: Event) => void;
+
+  /**
+   * Handler that is called when the selection change.
+   * Note that under the hood, this is just an alias for Spectrum's
+   * `onSelectionChange`. We are renaming for better consistency with other
+   * components.
+   */
+  onChange?: (key: ItemKey) => void;
+
+  /**
+   * Handler that is called when the selection changes.
+   * @deprecated Use `onChange` instead
+   */
+  onSelectionChange?: (key: ItemKey) => void;
+
   children: ItemOrSection | ItemOrSection[];
-}
+} & /*
+ * Support remaining SpectrumPickerProps.
+ * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
+ * re-defined above to account for boolean types which aren't included in the
+ * React `Key` type, but are actually supported by the Spectrum Picker component.
+ */ Omit<
+  NormalizedSpectrumPickerProps,
+  | 'children'
+  | 'items'
+  | 'onSelectionChange'
+  | 'selectedKey'
+  | 'defaultSelectedKey'
+>;
 
 export function Picker({
   children,
diff --git a/packages/components/src/spectrum/picker/PickerModel.ts b/packages/components/src/spectrum/picker/PickerModel.ts
deleted file mode 100644
index 14db9ff3a9..0000000000
--- a/packages/components/src/spectrum/picker/PickerModel.ts
+++ /dev/null
@@ -1,45 +0,0 @@
-import type {
-  ItemKey,
-  NormalizedSpectrumPickerProps,
-  TooltipOptions,
-} from '../utils';
-
-export type PickerCommonProps = {
-  /** The currently selected key in the collection (controlled). */
-  selectedKey?: ItemKey | null;
-
-  /** The initial selected key in the collection (uncontrolled). */
-  defaultSelectedKey?: ItemKey;
-
-  /** Can be set to true or a TooltipOptions to enable item tooltips */
-  tooltip?: boolean | TooltipOptions;
-
-  /** Handler that is called when the picker is scrolled. */
-  onScroll?: (event: Event) => void;
-
-  /**
-   * Handler that is called when the selection change.
-   * Note that under the hood, this is just an alias for Spectrum's
-   * `onSelectionChange`. We are renaming for better consistency with other
-   * components.
-   */
-  onChange?: (key: ItemKey) => void;
-
-  /**
-   * Handler that is called when the selection changes.
-   * @deprecated Use `onChange` instead
-   */
-  onSelectionChange?: (key: ItemKey) => void;
-} & /*
- * Support remaining SpectrumPickerProps.
- * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
- * re-defined above to account for boolean types which aren't included in the
- * React `Key` type, but are actually supported by the Spectrum Picker component.
- */ Omit<
-  NormalizedSpectrumPickerProps,
-  | 'children'
-  | 'items'
-  | 'onSelectionChange'
-  | 'selectedKey'
-  | 'defaultSelectedKey'
->;
diff --git a/packages/components/src/spectrum/picker/Picker_old.tsx b/packages/components/src/spectrum/picker/Picker_old.tsx
deleted file mode 100644
index 7c7be51845..0000000000
--- a/packages/components/src/spectrum/picker/Picker_old.tsx
+++ /dev/null
@@ -1,156 +0,0 @@
-import { useMemo } from 'react';
-
-import {
-  normalizeTooltipOptions,
-  TooltipOptions,
-  isNormalizedItemsWithKeysList,
-} from '../utils/itemUtils';
-import { PickerCommonProps } from './PickerModel';
-import { PickerFromChildrenProps, PickerFromChildren } from './Picker';
-import PickerFromItems, { PickerFromItemsProps } from './PickerFromItems';
-
-export type PickerProps = {
-  /** Can be set to true or a TooltipOptions to enable item tooltips */
-  tooltip?: boolean | TooltipOptions;
-
-  /** Function to retrieve initial scroll position when opening the picker */
-  // getInitialScrollPosition?: () => Promise<number | null>;
-} & PickerCommonProps &
-  (
-    | { children: PickerFromChildrenProps['children'] }
-    | {
-        children: PickerFromItemsProps['items'];
-        getInitialScrollPosition: () => Promise<number | null | undefined>;
-      }
-  );
-
-/**
- * Picker component for selecting items from a list of items. Items can be
- * provided via the `items` prop or as children. Each item can be a string,
- * number, boolean, or a Spectrum <Item> element. The remaining props are just
- * pass through props for the Spectrum Picker component.
- * See https://react-spectrum.adobe.com/react-spectrum/Picker.html
- */
-export function Picker({
-  children,
-  tooltip = true,
-  // defaultSelectedKey,
-  // selectedKey,
-  // onChange,
-  // onOpenChange,
-  // onSelectionChange,
-  // eslint-disable-next-line camelcase
-  UNSAFE_className,
-  ...props
-}: PickerProps): JSX.Element {
-  const tooltipOptions = useMemo(
-    () => normalizeTooltipOptions(tooltip),
-    [tooltip]
-  );
-
-  // const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
-  //   useState(defaultSelectedKey);
-
-  // const getInitialScrollPositionInternal = useCallback(async () => {
-  //   if (getInitialScrollPosition != null) {
-  //     return getInitialScrollPosition();
-  //   }
-
-  //   if (isNormalizedItemsWithKeysList(children)) {
-  //     return null;
-  //   }
-
-  //   return getPositionOfSelectedItemElement({
-  //     children: wrapItemChildren(children, tooltipOptions),
-  //     itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
-  //     itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-  //     selectedKey: selectedKey ?? uncontrolledSelectedKey,
-  //     topOffset: PICKER_TOP_OFFSET,
-  //   });
-  // }, [
-  //   children,
-  //   getInitialScrollPosition,
-  //   selectedKey,
-  //   tooltipOptions,
-  //   uncontrolledSelectedKey,
-  // ]);
-
-  // const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
-  //   usePickerScrollOnOpen({
-  //     getInitialScrollPosition: getInitialScrollPositionInternal,
-  //     onScroll,
-  //     onOpenChange,
-  //   });
-
-  // const onChangeInternal = useCallback(
-  //   (key: ItemKey): void => {
-  //     // If our component is uncontrolled, track the selected key internally
-  //     if (selectedKey == null) {
-  //       setUncontrolledSelectedKey(key);
-  //     }
-
-  //     (onChange ?? onSelectionChange)?.(key);
-  //   },
-  //   [onChange, onSelectionChange, selectedKey]
-  // );
-
-  return isNormalizedItemsWithKeysList(children) ? (
-    <PickerFromItems
-      // eslint-disable-next-line react/jsx-props-no-spreading
-      {...(props as PickerFromItemsProps)}
-      items={children}
-      tooltipOptions={tooltipOptions}
-    />
-  ) : (
-    <PickerFromChildren
-      // eslint-disable-next-line react/jsx-props-no-spreading
-      {...props}
-      tooltipOptions={tooltipOptions}
-    >
-      {children}
-    </PickerFromChildren>
-  );
-
-  // return (
-  //   <SpectrumPicker
-  //     // eslint-disable-next-line react/jsx-props-no-spreading
-  //     {...spectrumPickerProps}
-  //     ref={scrollRef as unknown as DOMRef<HTMLDivElement>}
-  //     onOpenChange={onOpenChangeInternal}
-  //     UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-  //     items={normalizedItems}
-  //     // Spectrum Picker treats keys as strings if the `key` prop is explicitly
-  //     // set on `Item` elements. Since we do this in `renderItem`, we need to
-  //     // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
-  //     // for selection to work.
-  //     selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
-  //     defaultSelectedKey={
-  //       defaultSelectedKey == null
-  //         ? defaultSelectedKey
-  //         : defaultSelectedKey.toString()
-  //     }
-  //     // `onChange` is just an alias for `onSelectionChange`
-  //     onSelectionChange={
-  //       onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange']
-  //     }
-  //   >
-  //     {itemOrSection => {
-  //       if (isNormalizedSection(itemOrSection)) {
-  //         return (
-  //           <Section
-  //             key={getItemKey(itemOrSection)}
-  //             title={itemOrSection.item?.title}
-  //             items={itemOrSection.item?.items}
-  //           >
-  //             {renderNormalizedItem}
-  //           </Section>
-  //         );
-  //       }
-
-  //       return renderNormalizedItem(itemOrSection);
-  //     }}
-  //   </SpectrumPicker>
-  // );
-}
-
-export default Picker;
diff --git a/packages/components/src/spectrum/picker/index.ts b/packages/components/src/spectrum/picker/index.ts
index 623b4a4835..2ca1d63502 100644
--- a/packages/components/src/spectrum/picker/index.ts
+++ b/packages/components/src/spectrum/picker/index.ts
@@ -1,3 +1,2 @@
 export * from './Picker';
-export * from './PickerModel';
 export * from './usePickerScrollOnOpen';
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index e7af339b2f..7fcea780f7 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -102,6 +102,11 @@ export function Picker({
     deserializeRow,
   });
 
+  const normalizedItems = viewportData.items as (
+    | NormalizedItem
+    | NormalizedSection
+  )[];
+
   useEffect(
     // Set viewport to include the selected item so that its data will load and
     // the real `key` will be available to show the selection in the UI.
@@ -141,9 +146,9 @@ export function Picker({
       // 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 = (
-        viewportData.items as (NormalizedItem | NormalizedSection)[]
-      ).find(item => String(getItemKey(item)) === key);
+      const selectedItem = normalizedItems.find(
+        item => String(getItemKey(item)) === key
+      );
 
       const actualKey = getItemKey(selectedItem) ?? key;
 
@@ -155,7 +160,7 @@ export function Picker({
 
       (onChange ?? onSelectionChange)?.(actualKey);
     },
-    [viewportData.items, selectedKey, onChange, onSelectionChange]
+    [normalizedItems, selectedKey, onChange, onSelectionChange]
   );
 
   return (
@@ -164,7 +169,7 @@ export function Picker({
       {...props}
       ref={scrollRef as DOMRef<HTMLDivElement>}
       UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-      items={viewportData.items as (NormalizedItem | NormalizedSection)[]}
+      items={normalizedItems}
       // Spectrum Picker treats keys as strings if the `key` prop is explicitly
       // set on `Item` elements. Since we do this in `renderItem`, we need to
       // ensure that `selectedKey` and `defaultSelectedKey` are strings in order

From f9e05cedbe641b03954ac5f3eded51aa857ab1e6 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 16:45:13 -0500
Subject: [PATCH 08/30] Cleanup (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx |  6 +-
 .../components/src/spectrum/utils/index.ts    |  1 +
 .../src/spectrum/utils/itemUtils.ts           | 15 ++--
 .../utils/useStringifiedMultiSelection.ts     |  8 +-
 .../spectrum/utils/useStringifiedSelection.ts | 78 +++++++++++++++++++
 .../jsapi-components/src/spectrum/Picker.tsx  | 51 ++++++------
 6 files changed, 125 insertions(+), 34 deletions(-)
 create mode 100644 packages/components/src/spectrum/utils/useStringifiedSelection.ts

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index e9ad91c67d..539a9f44bc 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -78,6 +78,7 @@ export function Picker({
     [tooltip]
   );
 
+  const isUncontrolled = selectedKey === undefined;
   const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
     useState(defaultSelectedKey);
 
@@ -101,13 +102,14 @@ export function Picker({
   const onSelectionChangeInternal = useCallback(
     (key: ItemKey): void => {
       // If our component is uncontrolled, track the selected key internally
-      if (selectedKey == null) {
+      // so that we can scroll to the selected item if the user re-opens
+      if (isUncontrolled) {
         setUncontrolledSelectedKey(key);
       }
 
       (onChange ?? onSelectionChange)?.(key);
     },
-    [onChange, onSelectionChange, selectedKey]
+    [isUncontrolled, onChange, onSelectionChange]
   );
 
   const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
diff --git a/packages/components/src/spectrum/utils/index.ts b/packages/components/src/spectrum/utils/index.ts
index ef406aba98..72e2ecfe4d 100644
--- a/packages/components/src/spectrum/utils/index.ts
+++ b/packages/components/src/spectrum/utils/index.ts
@@ -2,3 +2,4 @@ export * from './itemUtils';
 export * from './themeUtils';
 export * from './useRenderNormalizedItem';
 export * from './useStringifiedMultiSelection';
+export * from './useStringifiedSelection';
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index 04160a86aa..dddd4fca87 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -380,12 +380,17 @@ export function normalizeTooltipOptions(
  * @param itemKeys The selection of `ItemKey`s
  * @returns The selection of strings
  */
-export function itemSelectionToStringSet(
-  itemKeys?: 'all' | Iterable<ItemKey>
-): undefined | 'all' | Set<string> {
+export function itemSelectionToStringSet<
+  TKeys extends 'all' | Iterable<ItemKey> | undefined,
+  TResult extends TKeys extends 'all'
+    ? 'all'
+    : TKeys extends Iterable<ItemKey>
+    ? Set<string>
+    : undefined,
+>(itemKeys: TKeys): TResult {
   if (itemKeys == null || itemKeys === 'all') {
-    return itemKeys as undefined | 'all';
+    return itemKeys as undefined | 'all' as TResult;
   }
 
-  return new Set([...itemKeys].map(String));
+  return new Set([...itemKeys].map(String)) as TResult;
 }
diff --git a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts
index 04ec670168..843d847a92 100644
--- a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts
+++ b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts
@@ -71,8 +71,12 @@ export function useStringifiedMultiSelection({
 
   const onStringSelectionChange = useCallback(
     (keys: 'all' | Set<Key>) => {
+      if (onChange == null) {
+        return;
+      }
+
       if (keys === 'all') {
-        onChange?.('all');
+        onChange('all');
         return;
       }
 
@@ -84,7 +88,7 @@ export function useStringifiedMultiSelection({
         }
       });
 
-      onChange?.(actualKeys);
+      onChange(actualKeys);
     },
     [normalizedItems, onChange]
   );
diff --git a/packages/components/src/spectrum/utils/useStringifiedSelection.ts b/packages/components/src/spectrum/utils/useStringifiedSelection.ts
new file mode 100644
index 0000000000..5ed05a7e42
--- /dev/null
+++ b/packages/components/src/spectrum/utils/useStringifiedSelection.ts
@@ -0,0 +1,78 @@
+import { Key, useCallback, useMemo } from 'react';
+import {
+  getItemKey,
+  ItemKey,
+  itemSelectionToStringSet,
+  NormalizedItem,
+  NormalizedSection,
+} from './itemUtils';
+
+export interface UseStringifiedSelectionOptions {
+  normalizedItems: (NormalizedItem | NormalizedSection)[];
+  selectedKey: ItemKey | null | undefined;
+  defaultSelectedKey: ItemKey | undefined;
+  disabledKeys: Iterable<ItemKey> | undefined;
+  onChange: (key: ItemKey) => void | undefined;
+}
+
+export interface UseStringifiedSelectionResult {
+  defaultSelectedStringKey?: Key;
+  selectedStringKey?: Key | null;
+  disabledStringKeys?: Set<Key>;
+  onStringSelectionChange: (key: Key) => void;
+}
+
+export function useStringifiedSelection({
+  normalizedItems,
+  defaultSelectedKey,
+  selectedKey,
+  disabledKeys,
+  onChange,
+}: UseStringifiedSelectionOptions): UseStringifiedSelectionResult {
+  const selectedStringKey = useMemo(
+    () => (selectedKey == null ? selectedKey : String(selectedKey)),
+    [selectedKey]
+  );
+
+  const defaultSelectedStringKey = useMemo(
+    () =>
+      defaultSelectedKey == null
+        ? defaultSelectedKey
+        : String(defaultSelectedKey),
+    [defaultSelectedKey]
+  );
+
+  const disabledStringKeys = useMemo(
+    () => itemSelectionToStringSet(disabledKeys),
+    [disabledKeys]
+  );
+
+  const onStringSelectionChange = useCallback(
+    (key: Key): void => {
+      if (onChange == null) {
+        return;
+      }
+
+      // 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(getItemKey(item)) === key
+      );
+
+      const actualKey = getItemKey(selectedItem) ?? key;
+
+      onChange(actualKey);
+    },
+    [normalizedItems, onChange]
+  );
+
+  return {
+    selectedStringKey,
+    defaultSelectedStringKey,
+    disabledStringKeys,
+    onStringSelectionChange,
+  };
+}
+
+export default useStringifiedSelection;
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index 7fcea780f7..1e81739b97 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -15,6 +15,7 @@ import {
   Section,
   usePickerScrollOnOpen,
   useRenderNormalizedItem,
+  useStringifiedSelection,
 } from '@deephaven/components';
 import { dh as DhType } from '@deephaven/jsapi-types';
 import { Settings } from '@deephaven/jsapi-utils';
@@ -48,6 +49,7 @@ export function Picker({
   tooltip = true,
   selectedKey,
   defaultSelectedKey,
+  disabledKeys,
   UNSAFE_className,
   onChange,
   onOpenChange,
@@ -61,6 +63,7 @@ export function Picker({
     [tooltip]
   );
 
+  const isUncontrolled = selectedKey === undefined;
   const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
     useState(defaultSelectedKey);
 
@@ -143,26 +146,33 @@ export function Picker({
 
   const onSelectionChangeInternal = useCallback(
     (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(getItemKey(item)) === key
-      );
-
-      const actualKey = getItemKey(selectedItem) ?? key;
-
       // If our component is uncontrolled, track the selected key internally
       // so that we can scroll to the selected item if the user re-opens
-      if (selectedKey == null) {
+      if (isUncontrolled) {
         setUncontrolledSelectedKey(key);
       }
 
-      (onChange ?? onSelectionChange)?.(actualKey);
+      (onChange ?? onSelectionChange)?.(key);
     },
-    [normalizedItems, selectedKey, onChange, onSelectionChange]
+    [isUncontrolled, onChange, onSelectionChange]
   );
 
+  // Spectrum Picker treats keys as strings if the `key` prop is explicitly
+  // set on `Item` elements. Since we do this in `renderItem`, we need to
+  // map original key types to and from strings so that selection works.
+  const {
+    selectedStringKey,
+    defaultSelectedStringKey,
+    disabledStringKeys,
+    onStringSelectionChange,
+  } = useStringifiedSelection({
+    normalizedItems,
+    selectedKey,
+    defaultSelectedKey,
+    disabledKeys,
+    onChange: onSelectionChangeInternal,
+  });
+
   return (
     <SpectrumPicker
       // eslint-disable-next-line react/jsx-props-no-spreading
@@ -170,19 +180,10 @@ export function Picker({
       ref={scrollRef as DOMRef<HTMLDivElement>}
       UNSAFE_className={cl('dh-picker', UNSAFE_className)}
       items={normalizedItems}
-      // Spectrum Picker treats keys as strings if the `key` prop is explicitly
-      // set on `Item` elements. Since we do this in `renderItem`, we need to
-      // ensure that `selectedKey` and `defaultSelectedKey` are strings in order
-      // for selection to work.
-      selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()}
-      defaultSelectedKey={
-        defaultSelectedKey == null
-          ? defaultSelectedKey
-          : defaultSelectedKey.toString()
-      }
-      onSelectionChange={
-        onSelectionChangeInternal // as NormalizedSpectrumPickerProps['onSelectionChange']
-      }
+      selectedKey={selectedStringKey}
+      defaultSelectedKey={defaultSelectedStringKey}
+      disabledKeys={disabledStringKeys}
+      onSelectionChange={onStringSelectionChange}
       onOpenChange={onOpenChangeInternal}
     >
       {itemOrSection => {

From eb413590d63046e4491f487b95f9d766fb84ffb6 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 17:18:43 -0500
Subject: [PATCH 09/30] PickerNormalized (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    | 128 ++++++++++--------
 .../src/spectrum/picker/PickerNormalized.tsx  | 103 ++++++++++++++
 .../components/src/spectrum/picker/index.ts   |   1 +
 .../spectrum/utils/useStringifiedSelection.ts |   2 +-
 4 files changed, 179 insertions(+), 55 deletions(-)
 create mode 100644 packages/components/src/spectrum/picker/PickerNormalized.tsx

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index 89adb62356..282b653951 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -6,10 +6,13 @@ import {
   ItemKey,
   Section,
   Text,
+  PickerNormalized,
 } from '@deephaven/components';
 import { vsPerson } from '@deephaven/icons';
 import { Icon } from '@adobe/react-spectrum';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
+import { getPositionOfSelectedItem } from '@deephaven/react-hooks';
+import { PICKER_ITEM_HEIGHTS, PICKER_TOP_OFFSET } from '@deephaven/utils';
 import { generateNormalizedItems, sampleSectionIdAndClasses } from './utils';
 
 // Generate enough items to require scrolling
@@ -26,6 +29,17 @@ function PersonIcon(): JSX.Element {
 export function Pickers(): JSX.Element {
   const [selectedKey, setSelectedKey] = useState<ItemKey | null>(null);
 
+  const getInitialScrollPosition = useCallback(
+    async () =>
+      getPositionOfSelectedItem({
+        keyedItems: items,
+        itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+        selectedKey,
+        topOffset: PICKER_TOP_OFFSET,
+      }),
+    [selectedKey]
+  );
+
   const onChange = useCallback((key: ItemKey): void => {
     setSelectedKey(key);
   }, []);
@@ -36,70 +50,76 @@ export function Pickers(): JSX.Element {
       <h2 className="ui-title">Pickers</h2>
 
       <Flex gap={14}>
-        <Picker label="Single Child" tooltip={{ placement: 'bottom-end' }}>
-          <Item>Aaa</Item>
-        </Picker>
-
-        <Picker label="Mixed Children Types" defaultSelectedKey={999} tooltip>
-          {/* eslint-disable react/jsx-curly-brace-presence */}
-          {'String 1'}
-          {'String 2'}
-          {'String 3'}
-          {''}
-          {'Some really long text that should get truncated'}
-          {/* eslint-enable react/jsx-curly-brace-presence */}
-          {444}
-          {999}
-          {true}
-          {false}
-          <Item>Item Aaa</Item>
-          <Item>Item Bbb</Item>
-          <Item textValue="Complex Ccc">
-            <PersonIcon />
-            <Text>Complex Ccc with text that should be truncated</Text>
-          </Item>
-        </Picker>
+        {true && (
+          <Picker label="Single Child" tooltip={{ placement: 'bottom-end' }}>
+            <Item>Aaa</Item>
+          </Picker>
+        )}
 
-        <Picker label="Sections" tooltip>
-          {/* eslint-disable react/jsx-curly-brace-presence */}
-          {'String 1'}
-          {'String 2'}
-          {'String 3'}
-          <Section title="Section A">
+        {false && (
+          <Picker label="Mixed Children Types" defaultSelectedKey={999} tooltip>
+            {/* eslint-disable react/jsx-curly-brace-presence */}
+            {'String 1'}
+            {'String 2'}
+            {'String 3'}
+            {''}
+            {'Some really long text that should get truncated'}
+            {/* eslint-enable react/jsx-curly-brace-presence */}
+            {444}
+            {999}
+            {true}
+            {false}
             <Item>Item Aaa</Item>
             <Item>Item Bbb</Item>
             <Item textValue="Complex Ccc">
               <PersonIcon />
-              <Text>Complex Ccc</Text>
-            </Item>
-          </Section>
-          <Section key="Key B">
-            <Item>Item Ddd</Item>
-            <Item>Item Eee</Item>
-            <Item textValue="Complex Fff">
-              <PersonIcon />
-              <Text>Complex Fff</Text>
+              <Text>Complex Ccc with text that should be truncated</Text>
             </Item>
-            <Item key="Ggg">
-              <PersonIcon />
-              <Text>Label</Text>
-              <Text slot="description">Description</Text>
-            </Item>
-            <Item key="Hhh">
-              <PersonIcon />
-              <Text>Label that causes overflow</Text>
-              <Text slot="description">Description that causes overflow</Text>
-            </Item>
-          </Section>
-        </Picker>
+          </Picker>
+        )}
+
+        {false && (
+          <Picker label="Sections" tooltip>
+            {/* eslint-disable react/jsx-curly-brace-presence */}
+            {'String 1'}
+            {'String 2'}
+            {'String 3'}
+            <Section title="Section A">
+              <Item>Item Aaa</Item>
+              <Item>Item Bbb</Item>
+              <Item textValue="Complex Ccc">
+                <PersonIcon />
+                <Text>Complex Ccc</Text>
+              </Item>
+            </Section>
+            <Section key="Key B">
+              <Item>Item Ddd</Item>
+              <Item>Item Eee</Item>
+              <Item textValue="Complex Fff">
+                <PersonIcon />
+                <Text>Complex Fff</Text>
+              </Item>
+              <Item key="Ggg">
+                <PersonIcon />
+                <Text>Label</Text>
+                <Text slot="description">Description</Text>
+              </Item>
+              <Item key="Hhh">
+                <PersonIcon />
+                <Text>Label that causes overflow</Text>
+                <Text slot="description">Description that causes overflow</Text>
+              </Item>
+            </Section>
+          </Picker>
+        )}
 
-        <Picker
+        <PickerNormalized
           label="Controlled"
+          getInitialScrollPosition={getInitialScrollPosition}
+          normalizedItems={items}
           selectedKey={selectedKey}
           onChange={onChange}
-        >
-          {items}
-        </Picker>
+        />
       </Flex>
     </div>
   );
diff --git a/packages/components/src/spectrum/picker/PickerNormalized.tsx b/packages/components/src/spectrum/picker/PickerNormalized.tsx
new file mode 100644
index 0000000000..2adc3517e5
--- /dev/null
+++ b/packages/components/src/spectrum/picker/PickerNormalized.tsx
@@ -0,0 +1,103 @@
+import { useMemo } from 'react';
+import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
+import type { DOMRef } from '@react-types/shared';
+import cl from 'classnames';
+import { EMPTY_FUNCTION } from '@deephaven/utils';
+import { Section } from '../shared';
+import type { PickerProps as PickerBaseProps } from './Picker';
+
+import {
+  getItemKey,
+  isNormalizedSection,
+  NormalizedItem,
+  NormalizedSection,
+  normalizeTooltipOptions,
+  useRenderNormalizedItem,
+  useStringifiedSelection,
+} from '../utils';
+import usePickerScrollOnOpen from './usePickerScrollOnOpen';
+
+export interface PickerNormalizedProps
+  extends Omit<PickerBaseProps, 'children'> {
+  normalizedItems: (NormalizedItem | NormalizedSection)[];
+  getInitialScrollPosition?: () => Promise<number | null | undefined>;
+  onScroll?: (event: Event) => void;
+}
+
+export function PickerNormalized({
+  normalizedItems,
+  tooltip = true,
+  selectedKey,
+  defaultSelectedKey,
+  disabledKeys,
+  UNSAFE_className,
+  getInitialScrollPosition,
+  onChange,
+  onOpenChange,
+  onScroll = EMPTY_FUNCTION,
+  onSelectionChange,
+  ...props
+}: PickerNormalizedProps): JSX.Element {
+  const tooltipOptions = useMemo(
+    () => normalizeTooltipOptions(tooltip),
+    [tooltip]
+  );
+
+  const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
+
+  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
+    usePickerScrollOnOpen({
+      getInitialScrollPosition,
+      onScroll,
+      onOpenChange,
+    });
+
+  // Spectrum Picker treats keys as strings if the `key` prop is explicitly
+  // set on `Item` elements. Since we do this in `renderItem`, we need to
+  // map original key types to and from strings so that selection works.
+  const {
+    selectedStringKey,
+    defaultSelectedStringKey,
+    disabledStringKeys,
+    onStringSelectionChange,
+  } = useStringifiedSelection({
+    normalizedItems,
+    selectedKey,
+    defaultSelectedKey,
+    disabledKeys,
+    onChange: onChange ?? onSelectionChange,
+  });
+
+  return (
+    <SpectrumPicker
+      // eslint-disable-next-line react/jsx-props-no-spreading
+      {...props}
+      ref={scrollRef as DOMRef<HTMLDivElement>}
+      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+      items={normalizedItems}
+      selectedKey={selectedStringKey}
+      defaultSelectedKey={defaultSelectedStringKey}
+      disabledKeys={disabledStringKeys}
+      onSelectionChange={onStringSelectionChange}
+      onOpenChange={onOpenChangeInternal}
+    >
+      {itemOrSection => {
+        if (isNormalizedSection(itemOrSection)) {
+          return (
+            <Section
+              key={getItemKey(itemOrSection)}
+              title={itemOrSection.item?.title}
+              items={itemOrSection.item?.items}
+            >
+              {renderNormalizedItem}
+            </Section>
+          );
+        }
+
+        return renderNormalizedItem(itemOrSection);
+      }}
+    </SpectrumPicker>
+  );
+}
+
+export default PickerNormalized;
diff --git a/packages/components/src/spectrum/picker/index.ts b/packages/components/src/spectrum/picker/index.ts
index 2ca1d63502..2ce75f0446 100644
--- a/packages/components/src/spectrum/picker/index.ts
+++ b/packages/components/src/spectrum/picker/index.ts
@@ -1,2 +1,3 @@
 export * from './Picker';
+export * from './PickerNormalized';
 export * from './usePickerScrollOnOpen';
diff --git a/packages/components/src/spectrum/utils/useStringifiedSelection.ts b/packages/components/src/spectrum/utils/useStringifiedSelection.ts
index 5ed05a7e42..a13c7a29d7 100644
--- a/packages/components/src/spectrum/utils/useStringifiedSelection.ts
+++ b/packages/components/src/spectrum/utils/useStringifiedSelection.ts
@@ -12,7 +12,7 @@ export interface UseStringifiedSelectionOptions {
   selectedKey: ItemKey | null | undefined;
   defaultSelectedKey: ItemKey | undefined;
   disabledKeys: Iterable<ItemKey> | undefined;
-  onChange: (key: ItemKey) => void | undefined;
+  onChange: ((key: ItemKey) => void) | undefined;
 }
 
 export interface UseStringifiedSelectionResult {

From 3a1a9cb0c60e3faca7f1b239f3b4591282defc48 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 17 Apr 2024 17:33:46 -0500
Subject: [PATCH 10/30] Refactored to use PickerNormalized (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    | 106 +++++++++---------
 .../src/spectrum/picker/PickerNormalized.tsx  |  11 +-
 .../jsapi-components/src/spectrum/Picker.tsx  |  86 ++------------
 3 files changed, 71 insertions(+), 132 deletions(-)

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index 282b653951..80573ece25 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -50,68 +50,62 @@ export function Pickers(): JSX.Element {
       <h2 className="ui-title">Pickers</h2>
 
       <Flex gap={14}>
-        {true && (
-          <Picker label="Single Child" tooltip={{ placement: 'bottom-end' }}>
-            <Item>Aaa</Item>
-          </Picker>
-        )}
+        <Picker label="Single Child" tooltip={{ placement: 'bottom-end' }}>
+          <Item>Aaa</Item>
+        </Picker>
 
-        {false && (
-          <Picker label="Mixed Children Types" defaultSelectedKey={999} tooltip>
-            {/* eslint-disable react/jsx-curly-brace-presence */}
-            {'String 1'}
-            {'String 2'}
-            {'String 3'}
-            {''}
-            {'Some really long text that should get truncated'}
-            {/* eslint-enable react/jsx-curly-brace-presence */}
-            {444}
-            {999}
-            {true}
-            {false}
+        <Picker label="Mixed Children Types" defaultSelectedKey={999} tooltip>
+          {/* eslint-disable react/jsx-curly-brace-presence */}
+          {'String 1'}
+          {'String 2'}
+          {'String 3'}
+          {''}
+          {'Some really long text that should get truncated'}
+          {/* eslint-enable react/jsx-curly-brace-presence */}
+          {444}
+          {999}
+          {true}
+          {false}
+          <Item>Item Aaa</Item>
+          <Item>Item Bbb</Item>
+          <Item textValue="Complex Ccc">
+            <PersonIcon />
+            <Text>Complex Ccc with text that should be truncated</Text>
+          </Item>
+        </Picker>
+
+        <Picker label="Sections" tooltip>
+          {/* eslint-disable react/jsx-curly-brace-presence */}
+          {'String 1'}
+          {'String 2'}
+          {'String 3'}
+          <Section title="Section A">
             <Item>Item Aaa</Item>
             <Item>Item Bbb</Item>
             <Item textValue="Complex Ccc">
               <PersonIcon />
-              <Text>Complex Ccc with text that should be truncated</Text>
+              <Text>Complex Ccc</Text>
             </Item>
-          </Picker>
-        )}
-
-        {false && (
-          <Picker label="Sections" tooltip>
-            {/* eslint-disable react/jsx-curly-brace-presence */}
-            {'String 1'}
-            {'String 2'}
-            {'String 3'}
-            <Section title="Section A">
-              <Item>Item Aaa</Item>
-              <Item>Item Bbb</Item>
-              <Item textValue="Complex Ccc">
-                <PersonIcon />
-                <Text>Complex Ccc</Text>
-              </Item>
-            </Section>
-            <Section key="Key B">
-              <Item>Item Ddd</Item>
-              <Item>Item Eee</Item>
-              <Item textValue="Complex Fff">
-                <PersonIcon />
-                <Text>Complex Fff</Text>
-              </Item>
-              <Item key="Ggg">
-                <PersonIcon />
-                <Text>Label</Text>
-                <Text slot="description">Description</Text>
-              </Item>
-              <Item key="Hhh">
-                <PersonIcon />
-                <Text>Label that causes overflow</Text>
-                <Text slot="description">Description that causes overflow</Text>
-              </Item>
-            </Section>
-          </Picker>
-        )}
+          </Section>
+          <Section key="Key B">
+            <Item>Item Ddd</Item>
+            <Item>Item Eee</Item>
+            <Item textValue="Complex Fff">
+              <PersonIcon />
+              <Text>Complex Fff</Text>
+            </Item>
+            <Item key="Ggg">
+              <PersonIcon />
+              <Text>Label</Text>
+              <Text slot="description">Description</Text>
+            </Item>
+            <Item key="Hhh">
+              <PersonIcon />
+              <Text>Label that causes overflow</Text>
+              <Text slot="description">Description that causes overflow</Text>
+            </Item>
+          </Section>
+        </Picker>
 
         <PickerNormalized
           label="Controlled"
diff --git a/packages/components/src/spectrum/picker/PickerNormalized.tsx b/packages/components/src/spectrum/picker/PickerNormalized.tsx
index 2adc3517e5..9c1d17bf5f 100644
--- a/packages/components/src/spectrum/picker/PickerNormalized.tsx
+++ b/packages/components/src/spectrum/picker/PickerNormalized.tsx
@@ -24,6 +24,11 @@ export interface PickerNormalizedProps
   onScroll?: (event: Event) => void;
 }
 
+/**
+ * Picker that takes an array of `NormalizedItem` or `NormalizedSection` items
+ * as children and uses a render item function to render the items. This is
+ * necessary to support windowed data.
+ */
 export function PickerNormalized({
   normalizedItems,
   tooltip = true,
@@ -73,7 +78,11 @@ export function PickerNormalized({
       // eslint-disable-next-line react/jsx-props-no-spreading
       {...props}
       ref={scrollRef as DOMRef<HTMLDivElement>}
-      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
+      UNSAFE_className={cl(
+        'dh-picker',
+        'dh-picker-normalized',
+        UNSAFE_className
+      )}
       items={normalizedItems}
       selectedKey={selectedStringKey}
       defaultSelectedKey={defaultSelectedStringKey}
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index 1e81739b97..b7f96c1fd8 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -1,21 +1,12 @@
 import { useCallback, useEffect, useMemo, useState } from 'react';
-import { Picker as SpectrumPicker } from '@adobe/react-spectrum';
-import type { DOMRef } from '@react-types/shared';
-import cl from 'classnames';
 import {
-  getItemKey,
-  isNormalizedSection,
   ItemKey,
   NormalizedItem,
   NormalizedItemData,
   NormalizedSection,
   NormalizedSectionData,
-  normalizeTooltipOptions,
+  PickerNormalized,
   PickerProps as PickerBaseProps,
-  Section,
-  usePickerScrollOnOpen,
-  useRenderNormalizedItem,
-  useStringifiedSelection,
 } from '@deephaven/components';
 import { dh as DhType } from '@deephaven/jsapi-types';
 import { Settings } from '@deephaven/jsapi-utils';
@@ -46,27 +37,17 @@ export function Picker({
   keyColumn: keyColumnName,
   labelColumn: labelColumnName,
   settings,
-  tooltip = true,
-  selectedKey,
-  defaultSelectedKey,
-  disabledKeys,
-  UNSAFE_className,
   onChange,
-  onOpenChange,
   onSelectionChange,
   ...props
 }: PickerProps): JSX.Element {
   const { getFormattedString: formatValue } = useFormatter(settings);
 
-  const tooltipOptions = useMemo(
-    () => normalizeTooltipOptions(tooltip),
-    [tooltip]
+  const isUncontrolled = props.selectedKey === undefined;
+  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = useState(
+    props.defaultSelectedKey
   );
 
-  const isUncontrolled = selectedKey === undefined;
-  const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
-    useState(defaultSelectedKey);
-
   const keyColumn = useMemo(
     () => getItemKeyColumn(table, keyColumnName),
     [keyColumnName, table]
@@ -82,7 +63,7 @@ export function Picker({
   const getItemIndexByValue = useGetItemIndexByValue({
     table,
     columnName: keyColumn.name,
-    value: selectedKey ?? uncontrolledSelectedKey,
+    value: props.selectedKey ?? uncontrolledSelectedKey,
   });
 
   const getInitialScrollPosition = useCallback(async () => {
@@ -135,15 +116,6 @@ export function Picker({
     [getItemIndexByValue, settings, setViewport]
   );
 
-  const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions);
-
-  const { ref: scrollRef, onOpenChange: onOpenChangeInternal } =
-    usePickerScrollOnOpen({
-      getInitialScrollPosition,
-      onScroll,
-      onOpenChange,
-    });
-
   const onSelectionChangeInternal = useCallback(
     (key: ItemKey): void => {
       // If our component is uncontrolled, track the selected key internally
@@ -157,51 +129,15 @@ export function Picker({
     [isUncontrolled, onChange, onSelectionChange]
   );
 
-  // Spectrum Picker treats keys as strings if the `key` prop is explicitly
-  // set on `Item` elements. Since we do this in `renderItem`, we need to
-  // map original key types to and from strings so that selection works.
-  const {
-    selectedStringKey,
-    defaultSelectedStringKey,
-    disabledStringKeys,
-    onStringSelectionChange,
-  } = useStringifiedSelection({
-    normalizedItems,
-    selectedKey,
-    defaultSelectedKey,
-    disabledKeys,
-    onChange: onSelectionChangeInternal,
-  });
-
   return (
-    <SpectrumPicker
+    <PickerNormalized
       // eslint-disable-next-line react/jsx-props-no-spreading
       {...props}
-      ref={scrollRef as DOMRef<HTMLDivElement>}
-      UNSAFE_className={cl('dh-picker', UNSAFE_className)}
-      items={normalizedItems}
-      selectedKey={selectedStringKey}
-      defaultSelectedKey={defaultSelectedStringKey}
-      disabledKeys={disabledStringKeys}
-      onSelectionChange={onStringSelectionChange}
-      onOpenChange={onOpenChangeInternal}
-    >
-      {itemOrSection => {
-        if (isNormalizedSection(itemOrSection)) {
-          return (
-            <Section
-              key={getItemKey(itemOrSection)}
-              title={itemOrSection.item?.title}
-              items={itemOrSection.item?.items}
-            >
-              {renderNormalizedItem}
-            </Section>
-          );
-        }
-
-        return renderNormalizedItem(itemOrSection);
-      }}
-    </SpectrumPicker>
+      normalizedItems={normalizedItems}
+      getInitialScrollPosition={getInitialScrollPosition}
+      onChange={onSelectionChangeInternal}
+      onScroll={onScroll}
+    />
   );
 }
 

From 2c0af1409228f5e44426667e581cfbebda3914b5 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Thu, 18 Apr 2024 09:28:53 -0500
Subject: [PATCH 11/30] Removed redundant Text wrapper (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    | 20 ++++-----
 .../components/src/spectrum/ItemContent.tsx   |  3 +-
 .../spectrum/utils/itemWrapperUtils.test.tsx  | 45 +++++++++++++++++++
 .../src/spectrum/utils/itemWrapperUtils.tsx   | 34 +++-----------
 4 files changed, 62 insertions(+), 40 deletions(-)
 create mode 100644 packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index 80573ece25..9b04403f9b 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -51,10 +51,10 @@ export function Pickers(): JSX.Element {
 
       <Flex gap={14}>
         <Picker label="Single Child" tooltip={{ placement: 'bottom-end' }}>
-          <Item>Aaa</Item>
+          <Item textValue="Aaa">Aaa</Item>
         </Picker>
 
-        <Picker label="Mixed Children Types" defaultSelectedKey={999} tooltip>
+        <Picker label="Mixed Children Types" defaultSelectedKey="999" tooltip>
           {/* eslint-disable react/jsx-curly-brace-presence */}
           {'String 1'}
           {'String 2'}
@@ -66,8 +66,8 @@ export function Pickers(): JSX.Element {
           {999}
           {true}
           {false}
-          <Item>Item Aaa</Item>
-          <Item>Item Bbb</Item>
+          <Item textValue="Item Aaa">Item Aaa</Item>
+          <Item textValue="Item Bbb">Item Bbb</Item>
           <Item textValue="Complex Ccc">
             <PersonIcon />
             <Text>Complex Ccc with text that should be truncated</Text>
@@ -80,26 +80,26 @@ export function Pickers(): JSX.Element {
           {'String 2'}
           {'String 3'}
           <Section title="Section A">
-            <Item>Item Aaa</Item>
-            <Item>Item Bbb</Item>
+            <Item textValue="Item Aaa">Item Aaa</Item>
+            <Item textValue="Item Bbb">Item Bbb</Item>
             <Item textValue="Complex Ccc">
               <PersonIcon />
               <Text>Complex Ccc</Text>
             </Item>
           </Section>
           <Section key="Key B">
-            <Item>Item Ddd</Item>
-            <Item>Item Eee</Item>
+            <Item textValue="Item Ddd">Item Ddd</Item>
+            <Item textValue="Item Eee">Item Eee</Item>
             <Item textValue="Complex Fff">
               <PersonIcon />
               <Text>Complex Fff</Text>
             </Item>
-            <Item key="Ggg">
+            <Item key="Ggg" textValue="Ggg">
               <PersonIcon />
               <Text>Label</Text>
               <Text slot="description">Description</Text>
             </Item>
-            <Item key="Hhh">
+            <Item key="Hhh" textValue="Hhh">
               <PersonIcon />
               <Text>Label that causes overflow</Text>
               <Text slot="description">Description that causes overflow</Text>
diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx
index 1526d3ed64..f11e6c9638 100644
--- a/packages/components/src/spectrum/ItemContent.tsx
+++ b/packages/components/src/spectrum/ItemContent.tsx
@@ -7,6 +7,7 @@ import {
 } from 'react';
 import cl from 'classnames';
 import { isElementOfType, useCheckOverflow } from '@deephaven/react-hooks';
+import { NON_BREAKING_SPACE } from '@deephaven/utils';
 import { Text } from './Text';
 import { TooltipOptions } from './utils';
 import ItemTooltip from './ItemTooltip';
@@ -46,7 +47,7 @@ export function ItemContent({
   /* eslint-disable no-param-reassign */
   if (content === '') {
     // Prevent the item height from collapsing when the content is empty
-    content = '\xa0'; // Non-breaking space
+    content = NON_BREAKING_SPACE;
   } else if (typeof content === 'boolean') {
     // Boolean values need to be stringified to render
     content = String(content);
diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx
new file mode 100644
index 0000000000..7e1375f120
--- /dev/null
+++ b/packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx
@@ -0,0 +1,45 @@
+import React from 'react';
+import { ItemContent } from '../ItemContent';
+import { Item } from '../shared';
+import { wrapItemChildren } from './itemWrapperUtils';
+
+describe.each([null, { placement: 'top' }] as const)(
+  'wrapItemChildren: %s',
+  tooltipOptions => {
+    it('should wrap primitives with Item elements', () => {
+      const given = [
+        'Item 1',
+        2,
+        'Item 3',
+        <Item key="Item 4" textValue="Item 4">
+          Item 4
+        </Item>,
+        <Item key="Item 5" textValue="Item 5">
+          <ItemContent tooltipOptions={tooltipOptions}>Item 5</ItemContent>
+        </Item>,
+      ];
+
+      const expected = [
+        <Item key="Item 1" textValue="Item 1">
+          <ItemContent tooltipOptions={tooltipOptions}>Item 1</ItemContent>
+        </Item>,
+        <Item key="2" textValue="2">
+          <ItemContent tooltipOptions={tooltipOptions}>2</ItemContent>
+        </Item>,
+        <Item key="Item 3" textValue="Item 3">
+          <ItemContent tooltipOptions={tooltipOptions}>Item 3</ItemContent>
+        </Item>,
+        <Item key="Item 4" textValue="Item 4">
+          <ItemContent tooltipOptions={tooltipOptions}>Item 4</ItemContent>
+        </Item>,
+        <Item key="Item 5" textValue="Item 5">
+          <ItemContent tooltipOptions={tooltipOptions}>Item 5</ItemContent>
+        </Item>,
+      ];
+
+      const actual = wrapItemChildren(given, tooltipOptions);
+
+      expect(actual).toEqual(expected);
+    });
+  }
+);
diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
index d8f8087613..cad5a55a5d 100644
--- a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
+++ b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
@@ -1,8 +1,6 @@
-import { cloneElement, ReactElement, ReactNode } from 'react';
+import { cloneElement, ReactElement } from 'react';
 import { Item } from '@adobe/react-spectrum';
 import { isElementOfType } from '@deephaven/react-hooks';
-import { NON_BREAKING_SPACE } from '@deephaven/utils';
-import { Text } from '../Text';
 import {
   isItemElement,
   isSectionElement,
@@ -14,30 +12,6 @@ import {
 import { ItemProps } from '../shared';
 import ItemContent from '../ItemContent';
 
-/**
- * If the given content is a primitive type, wrap it in a Text component.
- * @param content The content to wrap
- * @param slot The slot to use for the Text component
- * @returns The wrapped content or original content if not a primitive type
- */
-export function wrapPrimitiveWithText(
-  content?: ReactNode,
-  slot?: string
-): ReactNode {
-  // eslint-disable-next-line no-param-reassign
-  content = content ?? '';
-
-  if (['string', 'boolean', 'number'].includes(typeof content)) {
-    return (
-      <Text slot={slot}>
-        {content === '' ? NON_BREAKING_SPACE : String(content)}
-      </Text>
-    );
-  }
-
-  return content;
-}
-
 /**
  * Ensure all primitive children are wrapped in `Item` elements and that all
  * `Item` element content is wrapped in `ItemContent` elements to handle text
@@ -54,7 +28,7 @@ export function wrapItemChildren(
     ? itemsOrSections
     : [itemsOrSections];
 
-  return itemsOrSectionsArray.map((item: ItemOrSection) => {
+  return itemsOrSectionsArray.map(item => {
     if (isItemElement(item)) {
       if (isElementOfType(item.props.children, ItemContent)) {
         return item;
@@ -66,7 +40,7 @@ export function wrapItemChildren(
         ...item.props,
         children: (
           <ItemContent tooltipOptions={tooltipOptions}>
-            {wrapPrimitiveWithText(item.props.children)}
+            {item.props.children}
           </ItemContent>
         ),
       });
@@ -98,3 +72,5 @@ export function wrapItemChildren(
     return item;
   });
 }
+
+export default wrapItemChildren;

From 7ac58b73d5426f937c4dfd519d2cf9ceaeca2242 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Thu, 18 Apr 2024 10:12:02 -0500
Subject: [PATCH 12/30] Added keys (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    | 36 +++++++++++++------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index 9b04403f9b..ffe94b99fe 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -51,7 +51,9 @@ export function Pickers(): JSX.Element {
 
       <Flex gap={14}>
         <Picker label="Single Child" tooltip={{ placement: 'bottom-end' }}>
-          <Item textValue="Aaa">Aaa</Item>
+          <Item key="Aaa" textValue="Aaa">
+            Aaa
+          </Item>
         </Picker>
 
         <Picker label="Mixed Children Types" defaultSelectedKey="999" tooltip>
@@ -66,9 +68,13 @@ export function Pickers(): JSX.Element {
           {999}
           {true}
           {false}
-          <Item textValue="Item Aaa">Item Aaa</Item>
-          <Item textValue="Item Bbb">Item Bbb</Item>
-          <Item textValue="Complex Ccc">
+          <Item key="Item Aaa" textValue="Item Aaa">
+            Item Aaa
+          </Item>
+          <Item key="Item Bbb" textValue="Item Bbb">
+            Item Bbb
+          </Item>
+          <Item key="Complex Ccc" textValue="Complex Ccc">
             <PersonIcon />
             <Text>Complex Ccc with text that should be truncated</Text>
           </Item>
@@ -79,18 +85,26 @@ export function Pickers(): JSX.Element {
           {'String 1'}
           {'String 2'}
           {'String 3'}
-          <Section title="Section A">
-            <Item textValue="Item Aaa">Item Aaa</Item>
-            <Item textValue="Item Bbb">Item Bbb</Item>
-            <Item textValue="Complex Ccc">
+          <Section key="Section A" title="Section A">
+            <Item key="Item Aaa" textValue="Item Aaa">
+              Item Aaa
+            </Item>
+            <Item key="Item Bbb" textValue="Item Bbb">
+              Item Bbb
+            </Item>
+            <Item key="Complex Ccc" textValue="Complex Ccc">
               <PersonIcon />
               <Text>Complex Ccc</Text>
             </Item>
           </Section>
           <Section key="Key B">
-            <Item textValue="Item Ddd">Item Ddd</Item>
-            <Item textValue="Item Eee">Item Eee</Item>
-            <Item textValue="Complex Fff">
+            <Item key="Item Ddd" textValue="Item Ddd">
+              Item Ddd
+            </Item>
+            <Item key="Item Eee" textValue="Item Eee">
+              Item Eee
+            </Item>
+            <Item key="Complex Fff" textValue="Complex Fff">
               <PersonIcon />
               <Text>Complex Fff</Text>
             </Item>

From 60d19728f8d37de314737dc1afcc769d06a65d31 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Thu, 18 Apr 2024 10:52:47 -0500
Subject: [PATCH 13/30] Derive key from text values (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    | 40 ++++++-------------
 packages/code-studio/src/styleguide/utils.ts  |  4 +-
 .../src/spectrum/utils/itemUtils.ts           |  2 +-
 .../src/spectrum/utils/itemWrapperUtils.tsx   |  4 ++
 .../spectrum/utils/useItemRowDeserializer.ts  |  1 +
 5 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index ffe94b99fe..bf8183fccf 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -51,9 +51,7 @@ export function Pickers(): JSX.Element {
 
       <Flex gap={14}>
         <Picker label="Single Child" tooltip={{ placement: 'bottom-end' }}>
-          <Item key="Aaa" textValue="Aaa">
-            Aaa
-          </Item>
+          <Item textValue="Aaa">Aaa</Item>
         </Picker>
 
         <Picker label="Mixed Children Types" defaultSelectedKey="999" tooltip>
@@ -68,13 +66,9 @@ export function Pickers(): JSX.Element {
           {999}
           {true}
           {false}
-          <Item key="Item Aaa" textValue="Item Aaa">
-            Item Aaa
-          </Item>
-          <Item key="Item Bbb" textValue="Item Bbb">
-            Item Bbb
-          </Item>
-          <Item key="Complex Ccc" textValue="Complex Ccc">
+          <Item textValue="Item Aaa">Item Aaa</Item>
+          <Item textValue="Item Bbb">Item Bbb</Item>
+          <Item textValue="Complex Ccc">
             <PersonIcon />
             <Text>Complex Ccc with text that should be truncated</Text>
           </Item>
@@ -85,35 +79,27 @@ export function Pickers(): JSX.Element {
           {'String 1'}
           {'String 2'}
           {'String 3'}
-          <Section key="Section A" title="Section A">
-            <Item key="Item Aaa" textValue="Item Aaa">
-              Item Aaa
-            </Item>
-            <Item key="Item Bbb" textValue="Item Bbb">
-              Item Bbb
-            </Item>
-            <Item key="Complex Ccc" textValue="Complex Ccc">
+          <Section title="Section A">
+            <Item textValue="Item Aaa">Item Aaa</Item>
+            <Item textValue="Item Bbb">Item Bbb</Item>
+            <Item textValue="Complex Ccc">
               <PersonIcon />
               <Text>Complex Ccc</Text>
             </Item>
           </Section>
           <Section key="Key B">
-            <Item key="Item Ddd" textValue="Item Ddd">
-              Item Ddd
-            </Item>
-            <Item key="Item Eee" textValue="Item Eee">
-              Item Eee
-            </Item>
-            <Item key="Complex Fff" textValue="Complex Fff">
+            <Item textValue="Item Ddd">Item Ddd</Item>
+            <Item textValue="Item Eee">Item Eee</Item>
+            <Item textValue="Complex Fff">
               <PersonIcon />
               <Text>Complex Fff</Text>
             </Item>
-            <Item key="Ggg" textValue="Ggg">
+            <Item textValue="Ggg">
               <PersonIcon />
               <Text>Label</Text>
               <Text slot="description">Description</Text>
             </Item>
-            <Item key="Hhh" textValue="Hhh">
+            <Item textValue="Hhh">
               <PersonIcon />
               <Text>Label that causes overflow</Text>
               <Text slot="description">Description that causes overflow</Text>
diff --git a/packages/code-studio/src/styleguide/utils.ts b/packages/code-studio/src/styleguide/utils.ts
index 7e7f9087e3..ad9e9b8b5f 100644
--- a/packages/code-studio/src/styleguide/utils.ts
+++ b/packages/code-studio/src/styleguide/utils.ts
@@ -23,12 +23,14 @@ export function* generateNormalizedItems(
     }
     const letter = letters[charI];
     const key = `${letter}${suffix}`;
+    const content = `${letter}${letter}${letter}${suffix}`;
 
     yield {
       key,
       item: {
         key: (i + 1) * 100,
-        content: `${letter}${letter}${letter}${suffix}`,
+        content,
+        textValue: content,
       },
     };
   }
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index dddd4fca87..ee7bb5caac 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -48,7 +48,7 @@ export type ItemSelectionChangeHandler = (key: ItemKey) => void;
 export interface NormalizedItemData {
   key?: ItemKey;
   content: ReactNode;
-  textValue?: string;
+  textValue: string;
 }
 
 export interface NormalizedSectionData {
diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
index cad5a55a5d..e5fc219e6b 100644
--- a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
+++ b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
@@ -38,6 +38,7 @@ export function wrapItemChildren(
       // overflow
       return cloneElement(item, {
         ...item.props,
+        key: item.key ?? item.props.textValue,
         children: (
           <ItemContent tooltipOptions={tooltipOptions}>
             {item.props.children}
@@ -49,6 +50,9 @@ export function wrapItemChildren(
     if (isSectionElement(item)) {
       return cloneElement(item, {
         ...item.props,
+        key:
+          item.key ??
+          (typeof item.props.title === 'string' ? item.props.title : undefined),
         children: wrapItemChildren(
           item.props.children,
           tooltipOptions
diff --git a/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.ts b/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.ts
index 6252209bd9..c09526d10e 100644
--- a/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.ts
+++ b/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.ts
@@ -56,6 +56,7 @@ export function useItemRowDeserializer({
       return {
         key,
         content,
+        textValue: content,
       };
     },
     [formatValue, keyColumn, labelColumn]

From 0bc0cfeda6ea844c64c4e5ab0715e98791b4c951 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Thu, 18 Apr 2024 11:36:58 -0500
Subject: [PATCH 14/30] Empty textValue handling + tests (#1935)

---
 .../__snapshots__/utils.test.ts.snap          | 100 ++++++++++++++++++
 .../src/spectrum/utils/itemUtils.ts           |   2 +
 .../spectrum/utils/itemWrapperUtils.test.tsx  |  21 +++-
 .../src/spectrum/utils/itemWrapperUtils.tsx   |  14 ++-
 .../utils/useRenderNormalizedItem.tsx         |  13 ++-
 .../utils/useItemRowDeserializer.test.ts      |   6 ++
 6 files changed, 147 insertions(+), 9 deletions(-)

diff --git a/packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap b/packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap
index a9964ea190..6ddf7d2bc4 100644
--- a/packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap
+++ b/packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap
@@ -6,6 +6,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "AAA",
       "key": 100,
+      "textValue": "AAA",
     },
     "key": "A",
   },
@@ -13,6 +14,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "BBB",
       "key": 200,
+      "textValue": "BBB",
     },
     "key": "B",
   },
@@ -20,6 +22,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "CCC",
       "key": 300,
+      "textValue": "CCC",
     },
     "key": "C",
   },
@@ -27,6 +30,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "DDD",
       "key": 400,
+      "textValue": "DDD",
     },
     "key": "D",
   },
@@ -34,6 +38,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "EEE",
       "key": 500,
+      "textValue": "EEE",
     },
     "key": "E",
   },
@@ -41,6 +46,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "FFF",
       "key": 600,
+      "textValue": "FFF",
     },
     "key": "F",
   },
@@ -48,6 +54,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "GGG",
       "key": 700,
+      "textValue": "GGG",
     },
     "key": "G",
   },
@@ -55,6 +62,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "HHH",
       "key": 800,
+      "textValue": "HHH",
     },
     "key": "H",
   },
@@ -62,6 +70,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "III",
       "key": 900,
+      "textValue": "III",
     },
     "key": "I",
   },
@@ -69,6 +78,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "JJJ",
       "key": 1000,
+      "textValue": "JJJ",
     },
     "key": "J",
   },
@@ -76,6 +86,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "KKK",
       "key": 1100,
+      "textValue": "KKK",
     },
     "key": "K",
   },
@@ -83,6 +94,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "LLL",
       "key": 1200,
+      "textValue": "LLL",
     },
     "key": "L",
   },
@@ -90,6 +102,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "MMM",
       "key": 1300,
+      "textValue": "MMM",
     },
     "key": "M",
   },
@@ -97,6 +110,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "NNN",
       "key": 1400,
+      "textValue": "NNN",
     },
     "key": "N",
   },
@@ -104,6 +118,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "OOO",
       "key": 1500,
+      "textValue": "OOO",
     },
     "key": "O",
   },
@@ -111,6 +126,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "PPP",
       "key": 1600,
+      "textValue": "PPP",
     },
     "key": "P",
   },
@@ -118,6 +134,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "QQQ",
       "key": 1700,
+      "textValue": "QQQ",
     },
     "key": "Q",
   },
@@ -125,6 +142,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "RRR",
       "key": 1800,
+      "textValue": "RRR",
     },
     "key": "R",
   },
@@ -132,6 +150,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "SSS",
       "key": 1900,
+      "textValue": "SSS",
     },
     "key": "S",
   },
@@ -139,6 +158,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "TTT",
       "key": 2000,
+      "textValue": "TTT",
     },
     "key": "T",
   },
@@ -146,6 +166,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "UUU",
       "key": 2100,
+      "textValue": "UUU",
     },
     "key": "U",
   },
@@ -153,6 +174,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "VVV",
       "key": 2200,
+      "textValue": "VVV",
     },
     "key": "V",
   },
@@ -160,6 +182,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "WWW",
       "key": 2300,
+      "textValue": "WWW",
     },
     "key": "W",
   },
@@ -167,6 +190,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "XXX",
       "key": 2400,
+      "textValue": "XXX",
     },
     "key": "X",
   },
@@ -174,6 +198,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "YYY",
       "key": 2500,
+      "textValue": "YYY",
     },
     "key": "Y",
   },
@@ -181,6 +206,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ZZZ",
       "key": 2600,
+      "textValue": "ZZZ",
     },
     "key": "Z",
   },
@@ -188,6 +214,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "aaa",
       "key": 2700,
+      "textValue": "aaa",
     },
     "key": "a",
   },
@@ -195,6 +222,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "bbb",
       "key": 2800,
+      "textValue": "bbb",
     },
     "key": "b",
   },
@@ -202,6 +230,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ccc",
       "key": 2900,
+      "textValue": "ccc",
     },
     "key": "c",
   },
@@ -209,6 +238,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ddd",
       "key": 3000,
+      "textValue": "ddd",
     },
     "key": "d",
   },
@@ -216,6 +246,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "eee",
       "key": 3100,
+      "textValue": "eee",
     },
     "key": "e",
   },
@@ -223,6 +254,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "fff",
       "key": 3200,
+      "textValue": "fff",
     },
     "key": "f",
   },
@@ -230,6 +262,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ggg",
       "key": 3300,
+      "textValue": "ggg",
     },
     "key": "g",
   },
@@ -237,6 +270,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "hhh",
       "key": 3400,
+      "textValue": "hhh",
     },
     "key": "h",
   },
@@ -244,6 +278,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "iii",
       "key": 3500,
+      "textValue": "iii",
     },
     "key": "i",
   },
@@ -251,6 +286,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "jjj",
       "key": 3600,
+      "textValue": "jjj",
     },
     "key": "j",
   },
@@ -258,6 +294,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "kkk",
       "key": 3700,
+      "textValue": "kkk",
     },
     "key": "k",
   },
@@ -265,6 +302,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "lll",
       "key": 3800,
+      "textValue": "lll",
     },
     "key": "l",
   },
@@ -272,6 +310,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "mmm",
       "key": 3900,
+      "textValue": "mmm",
     },
     "key": "m",
   },
@@ -279,6 +318,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "nnn",
       "key": 4000,
+      "textValue": "nnn",
     },
     "key": "n",
   },
@@ -286,6 +326,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ooo",
       "key": 4100,
+      "textValue": "ooo",
     },
     "key": "o",
   },
@@ -293,6 +334,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ppp",
       "key": 4200,
+      "textValue": "ppp",
     },
     "key": "p",
   },
@@ -300,6 +342,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "qqq",
       "key": 4300,
+      "textValue": "qqq",
     },
     "key": "q",
   },
@@ -307,6 +350,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "rrr",
       "key": 4400,
+      "textValue": "rrr",
     },
     "key": "r",
   },
@@ -314,6 +358,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "sss",
       "key": 4500,
+      "textValue": "sss",
     },
     "key": "s",
   },
@@ -321,6 +366,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ttt",
       "key": 4600,
+      "textValue": "ttt",
     },
     "key": "t",
   },
@@ -328,6 +374,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "uuu",
       "key": 4700,
+      "textValue": "uuu",
     },
     "key": "u",
   },
@@ -335,6 +382,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "vvv",
       "key": 4800,
+      "textValue": "vvv",
     },
     "key": "v",
   },
@@ -342,6 +390,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "www",
       "key": 4900,
+      "textValue": "www",
     },
     "key": "w",
   },
@@ -349,6 +398,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "xxx",
       "key": 5000,
+      "textValue": "xxx",
     },
     "key": "x",
   },
@@ -356,6 +406,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "yyy",
       "key": 5100,
+      "textValue": "yyy",
     },
     "key": "y",
   },
@@ -363,6 +414,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "zzz",
       "key": 5200,
+      "textValue": "zzz",
     },
     "key": "z",
   },
@@ -370,6 +422,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "AAA1",
       "key": 5300,
+      "textValue": "AAA1",
     },
     "key": "A1",
   },
@@ -377,6 +430,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "BBB1",
       "key": 5400,
+      "textValue": "BBB1",
     },
     "key": "B1",
   },
@@ -384,6 +438,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "CCC1",
       "key": 5500,
+      "textValue": "CCC1",
     },
     "key": "C1",
   },
@@ -391,6 +446,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "DDD1",
       "key": 5600,
+      "textValue": "DDD1",
     },
     "key": "D1",
   },
@@ -398,6 +454,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "EEE1",
       "key": 5700,
+      "textValue": "EEE1",
     },
     "key": "E1",
   },
@@ -405,6 +462,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "FFF1",
       "key": 5800,
+      "textValue": "FFF1",
     },
     "key": "F1",
   },
@@ -412,6 +470,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "GGG1",
       "key": 5900,
+      "textValue": "GGG1",
     },
     "key": "G1",
   },
@@ -419,6 +478,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "HHH1",
       "key": 6000,
+      "textValue": "HHH1",
     },
     "key": "H1",
   },
@@ -426,6 +486,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "III1",
       "key": 6100,
+      "textValue": "III1",
     },
     "key": "I1",
   },
@@ -433,6 +494,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "JJJ1",
       "key": 6200,
+      "textValue": "JJJ1",
     },
     "key": "J1",
   },
@@ -440,6 +502,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "KKK1",
       "key": 6300,
+      "textValue": "KKK1",
     },
     "key": "K1",
   },
@@ -447,6 +510,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "LLL1",
       "key": 6400,
+      "textValue": "LLL1",
     },
     "key": "L1",
   },
@@ -454,6 +518,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "MMM1",
       "key": 6500,
+      "textValue": "MMM1",
     },
     "key": "M1",
   },
@@ -461,6 +526,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "NNN1",
       "key": 6600,
+      "textValue": "NNN1",
     },
     "key": "N1",
   },
@@ -468,6 +534,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "OOO1",
       "key": 6700,
+      "textValue": "OOO1",
     },
     "key": "O1",
   },
@@ -475,6 +542,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "PPP1",
       "key": 6800,
+      "textValue": "PPP1",
     },
     "key": "P1",
   },
@@ -482,6 +550,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "QQQ1",
       "key": 6900,
+      "textValue": "QQQ1",
     },
     "key": "Q1",
   },
@@ -489,6 +558,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "RRR1",
       "key": 7000,
+      "textValue": "RRR1",
     },
     "key": "R1",
   },
@@ -496,6 +566,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "SSS1",
       "key": 7100,
+      "textValue": "SSS1",
     },
     "key": "S1",
   },
@@ -503,6 +574,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "TTT1",
       "key": 7200,
+      "textValue": "TTT1",
     },
     "key": "T1",
   },
@@ -510,6 +582,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "UUU1",
       "key": 7300,
+      "textValue": "UUU1",
     },
     "key": "U1",
   },
@@ -517,6 +590,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "VVV1",
       "key": 7400,
+      "textValue": "VVV1",
     },
     "key": "V1",
   },
@@ -524,6 +598,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "WWW1",
       "key": 7500,
+      "textValue": "WWW1",
     },
     "key": "W1",
   },
@@ -531,6 +606,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "XXX1",
       "key": 7600,
+      "textValue": "XXX1",
     },
     "key": "X1",
   },
@@ -538,6 +614,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "YYY1",
       "key": 7700,
+      "textValue": "YYY1",
     },
     "key": "Y1",
   },
@@ -545,6 +622,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ZZZ1",
       "key": 7800,
+      "textValue": "ZZZ1",
     },
     "key": "Z1",
   },
@@ -552,6 +630,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "aaa1",
       "key": 7900,
+      "textValue": "aaa1",
     },
     "key": "a1",
   },
@@ -559,6 +638,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "bbb1",
       "key": 8000,
+      "textValue": "bbb1",
     },
     "key": "b1",
   },
@@ -566,6 +646,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ccc1",
       "key": 8100,
+      "textValue": "ccc1",
     },
     "key": "c1",
   },
@@ -573,6 +654,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ddd1",
       "key": 8200,
+      "textValue": "ddd1",
     },
     "key": "d1",
   },
@@ -580,6 +662,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "eee1",
       "key": 8300,
+      "textValue": "eee1",
     },
     "key": "e1",
   },
@@ -587,6 +670,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "fff1",
       "key": 8400,
+      "textValue": "fff1",
     },
     "key": "f1",
   },
@@ -594,6 +678,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ggg1",
       "key": 8500,
+      "textValue": "ggg1",
     },
     "key": "g1",
   },
@@ -601,6 +686,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "hhh1",
       "key": 8600,
+      "textValue": "hhh1",
     },
     "key": "h1",
   },
@@ -608,6 +694,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "iii1",
       "key": 8700,
+      "textValue": "iii1",
     },
     "key": "i1",
   },
@@ -615,6 +702,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "jjj1",
       "key": 8800,
+      "textValue": "jjj1",
     },
     "key": "j1",
   },
@@ -622,6 +710,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "kkk1",
       "key": 8900,
+      "textValue": "kkk1",
     },
     "key": "k1",
   },
@@ -629,6 +718,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "lll1",
       "key": 9000,
+      "textValue": "lll1",
     },
     "key": "l1",
   },
@@ -636,6 +726,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "mmm1",
       "key": 9100,
+      "textValue": "mmm1",
     },
     "key": "m1",
   },
@@ -643,6 +734,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "nnn1",
       "key": 9200,
+      "textValue": "nnn1",
     },
     "key": "n1",
   },
@@ -650,6 +742,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ooo1",
       "key": 9300,
+      "textValue": "ooo1",
     },
     "key": "o1",
   },
@@ -657,6 +750,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ppp1",
       "key": 9400,
+      "textValue": "ppp1",
     },
     "key": "p1",
   },
@@ -664,6 +758,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "qqq1",
       "key": 9500,
+      "textValue": "qqq1",
     },
     "key": "q1",
   },
@@ -671,6 +766,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "rrr1",
       "key": 9600,
+      "textValue": "rrr1",
     },
     "key": "r1",
   },
@@ -678,6 +774,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "sss1",
       "key": 9700,
+      "textValue": "sss1",
     },
     "key": "s1",
   },
@@ -685,6 +782,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "ttt1",
       "key": 9800,
+      "textValue": "ttt1",
     },
     "key": "t1",
   },
@@ -692,6 +790,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "uuu1",
       "key": 9900,
+      "textValue": "uuu1",
     },
     "key": "u1",
   },
@@ -699,6 +798,7 @@ exports[`generateNormalizedItems should generate normalized items 1`] = `
     "item": {
       "content": "vvv1",
       "key": 10000,
+      "textValue": "vvv1",
     },
     "key": "v1",
   },
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index ee7bb5caac..de28d82dc9 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -11,6 +11,8 @@ import ItemContent from '../ItemContent';
 
 const log = Log.module('itemUtils');
 
+export const ITEM_EMPTY_STRING_TEXT_VALUE = 'Empty';
+
 export const INVALID_ITEM_ERROR_MESSAGE =
   'Items must be strings, numbers, booleans, <Item> or <Section> elements:';
 
diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx
index 7e1375f120..7551074fd1 100644
--- a/packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx
+++ b/packages/components/src/spectrum/utils/itemWrapperUtils.test.tsx
@@ -1,6 +1,7 @@
 import React from 'react';
 import { ItemContent } from '../ItemContent';
 import { Item } from '../shared';
+import { ITEM_EMPTY_STRING_TEXT_VALUE } from './itemUtils';
 import { wrapItemChildren } from './itemWrapperUtils';
 
 describe.each([null, { placement: 'top' }] as const)(
@@ -11,11 +12,14 @@ describe.each([null, { placement: 'top' }] as const)(
         'Item 1',
         2,
         'Item 3',
-        <Item key="Item 4" textValue="Item 4">
-          Item 4
-        </Item>,
+        '',
+        // eslint-disable-next-line react/jsx-key
+        <Item textValue="Item 4">Item 4</Item>,
         <Item key="Item 5" textValue="Item 5">
-          <ItemContent tooltipOptions={tooltipOptions}>Item 5</ItemContent>
+          Item 5
+        </Item>,
+        <Item key="Item 6" textValue="Item 6">
+          <ItemContent tooltipOptions={tooltipOptions}>Item 6</ItemContent>
         </Item>,
       ];
 
@@ -29,12 +33,21 @@ describe.each([null, { placement: 'top' }] as const)(
         <Item key="Item 3" textValue="Item 3">
           <ItemContent tooltipOptions={tooltipOptions}>Item 3</ItemContent>
         </Item>,
+        <Item key="" textValue={ITEM_EMPTY_STRING_TEXT_VALUE}>
+          <ItemContent tooltipOptions={tooltipOptions}>
+            {/* eslint-disable react/jsx-curly-brace-presence */}
+            {''}
+          </ItemContent>
+        </Item>,
         <Item key="Item 4" textValue="Item 4">
           <ItemContent tooltipOptions={tooltipOptions}>Item 4</ItemContent>
         </Item>,
         <Item key="Item 5" textValue="Item 5">
           <ItemContent tooltipOptions={tooltipOptions}>Item 5</ItemContent>
         </Item>,
+        <Item key="Item 6" textValue="Item 6">
+          <ItemContent tooltipOptions={tooltipOptions}>Item 6</ItemContent>
+        </Item>,
       ];
 
       const actual = wrapItemChildren(given, tooltipOptions);
diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
index e5fc219e6b..f706bfbc36 100644
--- a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
+++ b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
@@ -6,6 +6,7 @@ import {
   isSectionElement,
   ItemElement,
   ItemOrSection,
+  ITEM_EMPTY_STRING_TEXT_VALUE,
   SectionElement,
   TooltipOptions,
 } from './itemUtils';
@@ -34,11 +35,18 @@ export function wrapItemChildren(
         return item;
       }
 
+      const key = item.key ?? item.props.textValue;
+      const textValue =
+        item.props.textValue === ''
+          ? ITEM_EMPTY_STRING_TEXT_VALUE
+          : item.props.textValue;
+
       // Wrap in `ItemContent` so we can support tooltips and handle text
       // overflow
       return cloneElement(item, {
         ...item.props,
-        key: item.key ?? item.props.textValue,
+        key,
+        textValue,
         children: (
           <ItemContent tooltipOptions={tooltipOptions}>
             {item.props.children}
@@ -66,8 +74,10 @@ export function wrapItemChildren(
       typeof item === 'boolean'
     ) {
       const text = String(item);
+      const textValue = text === '' ? ITEM_EMPTY_STRING_TEXT_VALUE : text;
+
       return (
-        <Item key={text} textValue={text}>
+        <Item key={text} textValue={textValue}>
           <ItemContent tooltipOptions={tooltipOptions}>{text}</ItemContent>
         </Item>
       );
diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx
index 2904bbb558..1f199653d7 100644
--- a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx
+++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx
@@ -1,7 +1,12 @@
 import { Key, useCallback } from 'react';
 import { ItemContent } from '../ItemContent';
 import { Item } from '../shared';
-import { getItemKey, NormalizedItem, TooltipOptions } from './itemUtils';
+import {
+  getItemKey,
+  ITEM_EMPTY_STRING_TEXT_VALUE,
+  NormalizedItem,
+  TooltipOptions,
+} from './itemUtils';
 
 /**
  * Returns a render function that can be used to render a normalized item in
@@ -31,8 +36,10 @@ export function useRenderNormalizedItem(
           // The `textValue` prop gets used to provide the content of `<option>`
           // elements that back the Spectrum Picker. These are not visible in the UI,
           // but are used for accessibility purposes, so we set to an arbitrary
-          // 'Empty' value so that they are not empty strings.
-          textValue={textValue === '' ? 'Empty' : textValue}
+          // `ITEM_EMPTY_STRING_TEXT_VALUE` value so that they are not empty strings.
+          textValue={
+            textValue === '' ? ITEM_EMPTY_STRING_TEXT_VALUE : textValue
+          }
         >
           <ItemContent tooltipOptions={tooltipOptions}>{content}</ItemContent>
         </Item>
diff --git a/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.test.ts b/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.test.ts
index 6ae5cf95ff..1d0bff5fff 100644
--- a/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.test.ts
+++ b/packages/jsapi-components/src/spectrum/utils/useItemRowDeserializer.test.ts
@@ -39,6 +39,7 @@ describe('useItemRowDeserializer', () => {
       {
         key: 'mock.keyValue',
         content: 'mock.labelValue',
+        textValue: 'mock.labelValue',
       },
     ],
     [
@@ -49,6 +50,7 @@ describe('useItemRowDeserializer', () => {
       {
         key: 888,
         content: '999',
+        textValue: '999',
       },
     ],
     [
@@ -59,6 +61,7 @@ describe('useItemRowDeserializer', () => {
       {
         key: true,
         content: 'false',
+        textValue: 'false',
       },
     ],
     [
@@ -69,6 +72,7 @@ describe('useItemRowDeserializer', () => {
       {
         key: false,
         content: 'true',
+        textValue: 'true',
       },
     ],
     [
@@ -79,6 +83,7 @@ describe('useItemRowDeserializer', () => {
       {
         key: String({}),
         content: String({}),
+        textValue: String({}),
       },
     ],
     [
@@ -89,6 +94,7 @@ describe('useItemRowDeserializer', () => {
       {
         key: 'mock.keyValue',
         content: formattedValue,
+        textValue: formattedValue,
       },
     ],
   ])(

From bd4f6279fab6f9a3bb34445ee91553214c0dc8ae Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Thu, 18 Apr 2024 11:53:28 -0500
Subject: [PATCH 15/30] Fixed uncontrolled selection detection (#1935)

---
 packages/components/src/spectrum/picker/Picker.tsx | 6 ++++--
 packages/jsapi-components/src/spectrum/Picker.tsx  | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index 539a9f44bc..15538abda7 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -78,6 +78,8 @@ export function Picker({
     [tooltip]
   );
 
+  // `null` is a valid value for `selectedKey` in controlled mode, so we check
+  // for explicit `undefined` to identify uncontrolled mode.
   const isUncontrolled = selectedKey === undefined;
   const [uncontrolledSelectedKey, setUncontrolledSelectedKey] =
     useState(defaultSelectedKey);
@@ -93,10 +95,10 @@ export function Picker({
         children: wrappedItems,
         itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
         itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-        selectedKey: selectedKey ?? uncontrolledSelectedKey,
+        selectedKey: isUncontrolled ? uncontrolledSelectedKey : selectedKey,
         topOffset: PICKER_TOP_OFFSET,
       }),
-    [selectedKey, uncontrolledSelectedKey, wrappedItems]
+    [isUncontrolled, selectedKey, uncontrolledSelectedKey, wrappedItems]
   );
 
   const onSelectionChangeInternal = useCallback(
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index b7f96c1fd8..85e193c786 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -43,6 +43,8 @@ export function Picker({
 }: PickerProps): JSX.Element {
   const { getFormattedString: formatValue } = useFormatter(settings);
 
+  // `null` is a valid value for `selectedKey` in controlled mode, so we check
+  // for explicit `undefined` to identify uncontrolled mode.
   const isUncontrolled = props.selectedKey === undefined;
   const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = useState(
     props.defaultSelectedKey
@@ -63,7 +65,7 @@ export function Picker({
   const getItemIndexByValue = useGetItemIndexByValue({
     table,
     columnName: keyColumn.name,
-    value: props.selectedKey ?? uncontrolledSelectedKey,
+    value: isUncontrolled ? uncontrolledSelectedKey : props.selectedKey,
   });
 
   const getInitialScrollPosition = useCallback(async () => {

From b565cdca5bb8fca358abbddd0158b02224c66ce3 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Thu, 18 Apr 2024 16:29:03 -0500
Subject: [PATCH 16/30] Section heights in scroll position (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    | 18 ++++++-
 packages/code-studio/src/styleguide/utils.ts  | 33 +++++++++++-
 .../components/src/spectrum/picker/Picker.tsx |  5 +-
 .../src/spectrum/utils/itemUtils.ts           | 50 +++++++++++++++----
 packages/utils/src/UIConstants.ts             |  4 ++
 5 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index bf8183fccf..a266cc8c6b 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -13,10 +13,19 @@ import { Icon } from '@adobe/react-spectrum';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { getPositionOfSelectedItem } from '@deephaven/react-hooks';
 import { PICKER_ITEM_HEIGHTS, PICKER_TOP_OFFSET } from '@deephaven/utils';
-import { generateNormalizedItems, sampleSectionIdAndClasses } from './utils';
+import {
+  generateItemElements,
+  generateNormalizedItems,
+  sampleSectionIdAndClasses,
+} from './utils';
 
 // Generate enough items to require scrolling
 const items = [...generateNormalizedItems(52)];
+const itemElementsA = [...generateItemElements(0, 51)];
+const itemElementsB = [...generateItemElements(52, 103)];
+const itemElementsC = [...generateItemElements(104, 155)];
+const itemElementsD = [...generateItemElements(156, 207)];
+const itemElementsE = [...generateItemElements(208, 259)];
 
 function PersonIcon(): JSX.Element {
   return (
@@ -79,7 +88,7 @@ export function Pickers(): JSX.Element {
           {'String 1'}
           {'String 2'}
           {'String 3'}
-          <Section title="Section A">
+          <Section title="Section">
             <Item textValue="Item Aaa">Item Aaa</Item>
             <Item textValue="Item Bbb">Item Bbb</Item>
             <Item textValue="Complex Ccc">
@@ -105,6 +114,11 @@ export function Pickers(): JSX.Element {
               <Text slot="description">Description that causes overflow</Text>
             </Item>
           </Section>
+          <Section title="Section A">{itemElementsA}</Section>
+          <Section title="Section B">{itemElementsB}</Section>
+          <Section key="Section C">{itemElementsC}</Section>
+          <Section key="Section D">{itemElementsD}</Section>
+          <Section title="Section E">{itemElementsE}</Section>
         </Picker>
 
         <PickerNormalized
diff --git a/packages/code-studio/src/styleguide/utils.ts b/packages/code-studio/src/styleguide/utils.ts
index ad9e9b8b5f..8707ce5826 100644
--- a/packages/code-studio/src/styleguide/utils.ts
+++ b/packages/code-studio/src/styleguide/utils.ts
@@ -1,10 +1,39 @@
 import cl from 'classnames';
-import { useCallback, useState } from 'react';
-import { NormalizedItem } from '@deephaven/components';
+import { createElement, useCallback, useState } from 'react';
+import { Item, ItemElement, NormalizedItem } from '@deephaven/components';
 
 export const HIDE_FROM_E2E_TESTS_CLASS = 'hide-from-e2e-tests';
 export const SAMPLE_SECTION_CLASS = 'sample-section';
 
+/**
+ * Generate a given number of `Item` elements.
+ */
+export function* generateItemElements(
+  start: number,
+  end: number
+): Generator<ItemElement> {
+  const letters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
+  const len = letters.length;
+
+  for (let i = start; i <= end; i += 1) {
+    const charI = i % len;
+    let suffix = String(Math.floor(i / len));
+    if (suffix === '0') {
+      suffix = '';
+    }
+    const letter = letters[charI];
+    const key = `${letter}${suffix}`;
+    const content = `${letter}${letter}${letter}${suffix}`;
+
+    // eslint-disable-next-line react/no-children-prop
+    yield createElement(Item, {
+      key,
+      textValue: content,
+      children: content,
+    });
+  }
+}
+
 /**
  * Generate a given number of NormalizedItems.
  * @param count The number of items to generate
diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index 15538abda7..2adcf6448b 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -5,6 +5,7 @@ import cl from 'classnames';
 import {
   EMPTY_FUNCTION,
   PICKER_ITEM_HEIGHTS,
+  PICKER_SECTION_HEIGHTS,
   PICKER_TOP_OFFSET,
 } from '@deephaven/utils';
 import {
@@ -92,9 +93,11 @@ export function Picker({
   const getInitialScrollPosition = useCallback(
     async () =>
       getPositionOfSelectedItemElement({
-        children: wrappedItems,
+        itemsOrSections: wrappedItems,
         itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
         itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
+        sectionTitleHeight: PICKER_SECTION_HEIGHTS.title,
+        sectionEmptyTitleHeight: PICKER_SECTION_HEIGHTS.emptyTitle,
         selectedKey: isUncontrolled ? uncontrolledSelectedKey : selectedKey,
         topOffset: PICKER_TOP_OFFSET,
       }),
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index de28d82dc9..c1c0bb2fb3 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -101,37 +101,67 @@ export function getItemKey<
   return (item?.item?.key ?? item?.key) as TKey;
 }
 
+/**
+ * Get the position of the item with the given selected key in a list of items.
+ */
 export async function getPositionOfSelectedItemElement<
   TKey extends string | number | boolean | undefined,
 >({
-  children,
+  itemsOrSections,
   itemHeight,
   itemHeightWithDescription,
+  sectionTitleHeight,
+  sectionEmptyTitleHeight,
   selectedKey,
   topOffset,
 }: {
-  children: (ItemElement | SectionElement)[];
+  itemsOrSections: (ItemElement | SectionElement)[];
   selectedKey: TKey | null | undefined;
   itemHeight: number;
   itemHeightWithDescription: number;
+  sectionTitleHeight: number;
+  sectionEmptyTitleHeight: number;
   topOffset: number;
 }): Promise<number> {
-  let position = 0;
+  let position = topOffset;
+
+  if (selectedKey == null) {
+    return position;
+  }
+
+  const getItemHeight = (item: ItemElement) =>
+    isItemElementWithDescription(item) ? itemHeightWithDescription : itemHeight;
 
   // eslint-disable-next-line no-restricted-syntax
-  for (const child of children) {
-    if (child.key === selectedKey) {
-      break;
+  for (const itemOrSection of itemsOrSections) {
+    if (itemOrSection.key === selectedKey) {
+      return position;
     }
 
-    if (isItemElementWithDescription(child)) {
-      position += itemHeightWithDescription;
+    if (isSectionElement(itemOrSection)) {
+      position +=
+        (itemOrSection.props.title ?? '') === ''
+          ? sectionEmptyTitleHeight
+          : sectionTitleHeight;
+
+      const childItems = Array.isArray(itemOrSection.props.children)
+        ? itemOrSection.props.children
+        : [itemOrSection.props.children];
+
+      // eslint-disable-next-line no-restricted-syntax
+      for (const childItem of childItems) {
+        if (childItem.key === selectedKey) {
+          return position;
+        }
+
+        position += getItemHeight(childItem);
+      }
     } else {
-      position += itemHeight;
+      position += getItemHeight(itemOrSection);
     }
   }
 
-  return position + topOffset;
+  return topOffset;
 }
 
 /**
diff --git a/packages/utils/src/UIConstants.ts b/packages/utils/src/UIConstants.ts
index 4286849996..fcaf459c4e 100644
--- a/packages/utils/src/UIConstants.ts
+++ b/packages/utils/src/UIConstants.ts
@@ -6,6 +6,10 @@ export const PICKER_ITEM_HEIGHTS = {
   noDescription: 32,
   withDescription: 48,
 } as const;
+export const PICKER_SECTION_HEIGHTS = {
+  title: 33,
+  emptyTitle: 5,
+};
 export const PICKER_TOP_OFFSET = 4;
 export const TABLE_ROW_HEIGHT = 33;
 export const SCROLL_DEBOUNCE_MS = 150;

From 72b491adc847b0537dab534b3e2c6a9d0aa4fd24 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Fri, 19 Apr 2024 10:28:17 -0500
Subject: [PATCH 17/30] Diff cleanup (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index 2adcf6448b..94c2523fc4 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -20,18 +20,17 @@ import { wrapItemChildren } from '../utils/itemWrapperUtils';
 import usePickerScrollOnOpen from './usePickerScrollOnOpen';
 
 export type PickerProps = {
+  children: ItemOrSection | ItemOrSection[];
+
+  /** Can be set to true or a TooltipOptions to enable item tooltips */
+  tooltip?: boolean | TooltipOptions;
+
   /** The currently selected key in the collection (controlled). */
   selectedKey?: ItemKey | null;
 
   /** The initial selected key in the collection (uncontrolled). */
   defaultSelectedKey?: ItemKey;
 
-  /** Can be set to true or a TooltipOptions to enable item tooltips */
-  tooltip?: boolean | TooltipOptions;
-
-  /** Handler that is called when the picker is scrolled. */
-  onScroll?: (event: Event) => void;
-
   /**
    * Handler that is called when the selection change.
    * Note that under the hood, this is just an alias for Spectrum's
@@ -40,19 +39,20 @@ export type PickerProps = {
    */
   onChange?: (key: ItemKey) => void;
 
+  /** Handler that is called when the picker is scrolled. */
+  onScroll?: (event: Event) => void;
+
   /**
    * Handler that is called when the selection changes.
    * @deprecated Use `onChange` instead
    */
   onSelectionChange?: (key: ItemKey) => void;
-
-  children: ItemOrSection | ItemOrSection[];
-} & /*
+} /*
  * Support remaining SpectrumPickerProps.
  * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are
  * re-defined above to account for boolean types which aren't included in the
  * React `Key` type, but are actually supported by the Spectrum Picker component.
- */ Omit<
+ */ & Omit<
   NormalizedSpectrumPickerProps,
   | 'children'
   | 'items'

From c880becc492738436292c6a0954d56e754bf03b0 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Fri, 19 Apr 2024 10:32:39 -0500
Subject: [PATCH 18/30] Comment (#1935)

---
 packages/components/src/spectrum/picker/Picker.tsx | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index 94c2523fc4..852412e155 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -61,6 +61,13 @@ export type PickerProps = {
   | 'defaultSelectedKey'
 >;
 
+/**
+ * Picker component for selecting items from a list of items. Items can be
+ * provided via the `children` prop. Each item can be a string,	number, boolean,
+ * or a Spectrum <Item> element. The remaining props are just	pass through props
+ * for the Spectrum Picker component.
+ * See https://react-spectrum.adobe.com/react-spectrum/Picker.html
+ */
 export function Picker({
   children,
   tooltip = true,

From 3266fec17fdfb16c2078bc77d2a22f33427671f6 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Fri, 19 Apr 2024 10:46:25 -0500
Subject: [PATCH 19/30] Added comments (#1935)

---
 .../src/spectrum/utils/itemUtils.ts           | 20 ++++++++++---------
 .../src/spectrum/utils/itemWrapperUtils.tsx   |  1 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index c1c0bb2fb3..9addf8c3cf 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -11,6 +11,10 @@ import ItemContent from '../ItemContent';
 
 const log = Log.module('itemUtils');
 
+/**
+ * `Item.textValue` prop needs to be a non-empty string for accessibility
+ * purposes. This is not displayed in the UI.
+ */
 export const ITEM_EMPTY_STRING_TEXT_VALUE = 'Empty';
 
 export const INVALID_ITEM_ERROR_MESSAGE =
@@ -186,6 +190,12 @@ export function isItemElement<T>(
   return isElementOfType(node, Item);
 }
 
+/**
+ * Determine if a node is an Item element containing a child `Text` element with
+ * a `slot` prop set to `description`.
+ * @param node The node to check
+ * @returns True if the node is an Item element with a description
+ */
 export function isItemElementWithDescription<T>(
   node: ReactNode
 ): node is ReactElement<ItemProps<T>> {
@@ -193,6 +203,7 @@ export function isItemElementWithDescription<T>(
     return false;
   }
 
+  // If children are wrapped in `ItemContent`, go down 1 level
   const children = isElementOfType(node.props.children, ItemContent)
     ? node.props.children.props.children
     : node.props.children;
@@ -201,17 +212,8 @@ export function isItemElementWithDescription<T>(
 
   const result = childrenArray.some(
     child => child.props?.slot === 'description' && isElementOfType(child, Text)
-    // (isElementOfType(child, Text) || child.props.children?.[0].type === Text)
   );
 
-  // console.log(
-  //   '[TESTING] result:',
-  //   result,
-  //   node.key,
-  //   childrenArray,
-  //   childrenArray.map(child => [child, Text])
-  // );
-
   return result;
 }
 
diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
index f706bfbc36..47d8ae75bf 100644
--- a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
+++ b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx
@@ -31,6 +31,7 @@ export function wrapItemChildren(
 
   return itemsOrSectionsArray.map(item => {
     if (isItemElement(item)) {
+      // Item content is already wrapped
       if (isElementOfType(item.props.children, ItemContent)) {
         return item;
       }

From a6de5c7d0533879da71d821bb999cf106b1cb3d5 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Fri, 19 Apr 2024 14:24:58 -0500
Subject: [PATCH 20/30] useStringifiedSelection tests (#1935)

---
 .../useStringifiedMultiSelection.test.ts      |  2 +-
 .../utils/useStringifiedSelection.test.ts     | 57 +++++++++++++++++++
 .../spectrum/utils/useStringifiedSelection.ts | 15 +++++
 3 files changed, 73 insertions(+), 1 deletion(-)
 create mode 100644 packages/components/src/spectrum/utils/useStringifiedSelection.test.ts

diff --git a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.test.ts b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.test.ts
index a429d9c1e7..8f5c4e89d4 100644
--- a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.test.ts
+++ b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.test.ts
@@ -11,7 +11,7 @@ describe('useStringifiedMultiSelection', () => {
   const normalizedItems: NormalizedItem[] = [1, 2, 3, 4, 5, 6, 7, 8, 9].map(
     i => ({
       key: i,
-      item: { key: i, content: `Item ${i}` },
+      item: { key: i, content: `Item ${i}`, textValue: `Item ${i}` },
     })
   );
 
diff --git a/packages/components/src/spectrum/utils/useStringifiedSelection.test.ts b/packages/components/src/spectrum/utils/useStringifiedSelection.test.ts
new file mode 100644
index 0000000000..b1b179b08b
--- /dev/null
+++ b/packages/components/src/spectrum/utils/useStringifiedSelection.test.ts
@@ -0,0 +1,57 @@
+import { renderHook } from '@testing-library/react-hooks';
+import { NormalizedItem } from './itemUtils';
+import { useStringifiedSelection } from './useStringifiedSelection';
+
+describe('useStringifiedSelection', () => {
+  const normalizedItems: NormalizedItem[] = [1, 2, 3, 4, 5, 6, 7, 8, 9].map(
+    i => ({
+      key: i,
+      item: { key: i, content: `Item ${i}`, textValue: `Item ${i}` },
+    })
+  );
+
+  const selectedKey = 1;
+  const defaultSelectedKey = 4;
+  const disabledKeys = [7, 8, 9];
+
+  const selectedStringKey = '1';
+  const defaultSelectedStringKey = '4';
+  const disabledStringKeys = new Set(['7', '8', '9']);
+
+  it('should stringify selections', () => {
+    const { result } = renderHook(() =>
+      useStringifiedSelection({
+        normalizedItems,
+        selectedKey,
+        defaultSelectedKey,
+        disabledKeys,
+        onChange: undefined,
+      })
+    );
+
+    expect(result.current).toEqual({
+      selectedStringKey,
+      defaultSelectedStringKey,
+      disabledStringKeys,
+      onStringSelectionChange: expect.any(Function),
+    });
+  });
+
+  it('should call onChange with actual key', () => {
+    const onChange = jest.fn().mockName('onChange');
+
+    const { result } = renderHook(() =>
+      useStringifiedSelection({
+        normalizedItems,
+        selectedKey,
+        defaultSelectedKey,
+        disabledKeys,
+        onChange,
+      })
+    );
+
+    result.current.onStringSelectionChange('2');
+
+    expect(onChange).toHaveBeenCalledWith(2);
+  });
+});
diff --git a/packages/components/src/spectrum/utils/useStringifiedSelection.ts b/packages/components/src/spectrum/utils/useStringifiedSelection.ts
index a13c7a29d7..ffea625f53 100644
--- a/packages/components/src/spectrum/utils/useStringifiedSelection.ts
+++ b/packages/components/src/spectrum/utils/useStringifiedSelection.ts
@@ -22,6 +22,21 @@ export interface UseStringifiedSelectionResult {
   onStringSelectionChange: (key: Key) => void;
 }
 
+/**
+ * Spectrum collection components treat keys as strings if the `key` prop is
+ * explicitly set on `Item` elements. Since we do this in `useRenderNormalizedItem`,
+ * we need to ensure that keys are strings in order for selection to work. We
+ * then need to convert back to the original key types in the onChange handler.
+ * This hook encapsulates converting to and from strings so that keys can match
+ * the original key type.
+ * @param normalizedItems The normalized items to select from.
+ * @param selectedKey The currently selected key in the collection.
+ * @param defaultSelectedKey The initial selected key in the collection.
+ * @param disabledKeys The currently disabled keys in the collection.
+ * @param onChange Handler that is called when the selection changes.
+ * @returns UseStringifiedSelectionResult with stringified key sets and string
+ * key selection change handler.
+ */
 export function useStringifiedSelection({
   normalizedItems,
   defaultSelectedKey,

From c1f427057636c40c23ba479d74631be792e73aae Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 10:14:00 -0500
Subject: [PATCH 21/30] Disable auto scroll for description / section scenarios
 (#1935)

---
 .../components/src/spectrum/picker/Picker.tsx | 44 ++++++++++++++-----
 .../src/spectrum/utils/itemUtils.ts           | 39 +++-------------
 .../jsapi-components/src/spectrum/Picker.tsx  | 10 +++--
 packages/utils/src/UIConstants.ts             |  9 ++--
 4 files changed, 48 insertions(+), 54 deletions(-)

diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx
index 852412e155..f9fcf64263 100644
--- a/packages/components/src/spectrum/picker/Picker.tsx
+++ b/packages/components/src/spectrum/picker/Picker.tsx
@@ -5,7 +5,6 @@ import cl from 'classnames';
 import {
   EMPTY_FUNCTION,
   PICKER_ITEM_HEIGHTS,
-  PICKER_SECTION_HEIGHTS,
   PICKER_TOP_OFFSET,
 } from '@deephaven/utils';
 import {
@@ -15,9 +14,12 @@ import {
   ItemKey,
   normalizeTooltipOptions,
   TooltipOptions,
+  isItemElementWithDescription,
+  isSectionElement,
 } from '../utils/itemUtils';
 import { wrapItemChildren } from '../utils/itemWrapperUtils';
 import usePickerScrollOnOpen from './usePickerScrollOnOpen';
+import { useSpectrumThemeProvider } from '../../theme';
 
 export type PickerProps = {
   children: ItemOrSection | ItemOrSection[];
@@ -81,6 +83,9 @@ export function Picker({
   UNSAFE_className,
   ...spectrumPickerProps
 }: PickerProps): JSX.Element {
+  const { scale } = useSpectrumThemeProvider();
+  const itemHeight = PICKER_ITEM_HEIGHTS[scale];
+
   const tooltipOptions = useMemo(
     () => normalizeTooltipOptions(tooltip),
     [tooltip]
@@ -97,18 +102,35 @@ export function Picker({
     [children, tooltipOptions]
   );
 
+  // Item descriptions and Section elements introduce variable item heights.
+  // This throws off scroll position calculations, so we disable auto scrolling
+  // if either of these are found.
+  const disableScrollOnOpen = useMemo(
+    () =>
+      wrappedItems.some(
+        item => isSectionElement(item) || isItemElementWithDescription(item)
+      ),
+    [wrappedItems]
+  );
+
   const getInitialScrollPosition = useCallback(
     async () =>
-      getPositionOfSelectedItemElement({
-        itemsOrSections: wrappedItems,
-        itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
-        itemHeightWithDescription: PICKER_ITEM_HEIGHTS.withDescription,
-        sectionTitleHeight: PICKER_SECTION_HEIGHTS.title,
-        sectionEmptyTitleHeight: PICKER_SECTION_HEIGHTS.emptyTitle,
-        selectedKey: isUncontrolled ? uncontrolledSelectedKey : selectedKey,
-        topOffset: PICKER_TOP_OFFSET,
-      }),
-    [isUncontrolled, selectedKey, uncontrolledSelectedKey, wrappedItems]
+      disableScrollOnOpen
+        ? null
+        : getPositionOfSelectedItemElement({
+            items: wrappedItems,
+            itemHeight,
+            selectedKey: isUncontrolled ? uncontrolledSelectedKey : selectedKey,
+            topOffset: PICKER_TOP_OFFSET,
+          }),
+    [
+      disableScrollOnOpen,
+      isUncontrolled,
+      itemHeight,
+      selectedKey,
+      uncontrolledSelectedKey,
+      wrappedItems,
+    ]
   );
 
   const onSelectionChangeInternal = useCallback(
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index 9addf8c3cf..5a13d5a548 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -111,20 +111,14 @@ export function getItemKey<
 export async function getPositionOfSelectedItemElement<
   TKey extends string | number | boolean | undefined,
 >({
-  itemsOrSections,
+  items,
   itemHeight,
-  itemHeightWithDescription,
-  sectionTitleHeight,
-  sectionEmptyTitleHeight,
   selectedKey,
   topOffset,
 }: {
-  itemsOrSections: (ItemElement | SectionElement)[];
+  items: ItemElement[];
   selectedKey: TKey | null | undefined;
   itemHeight: number;
-  itemHeightWithDescription: number;
-  sectionTitleHeight: number;
-  sectionEmptyTitleHeight: number;
   topOffset: number;
 }): Promise<number> {
   let position = topOffset;
@@ -133,36 +127,13 @@ export async function getPositionOfSelectedItemElement<
     return position;
   }
 
-  const getItemHeight = (item: ItemElement) =>
-    isItemElementWithDescription(item) ? itemHeightWithDescription : itemHeight;
-
   // eslint-disable-next-line no-restricted-syntax
-  for (const itemOrSection of itemsOrSections) {
-    if (itemOrSection.key === selectedKey) {
+  for (const item of items) {
+    if (item.key === selectedKey) {
       return position;
     }
 
-    if (isSectionElement(itemOrSection)) {
-      position +=
-        (itemOrSection.props.title ?? '') === ''
-          ? sectionEmptyTitleHeight
-          : sectionTitleHeight;
-
-      const childItems = Array.isArray(itemOrSection.props.children)
-        ? itemOrSection.props.children
-        : [itemOrSection.props.children];
-
-      // eslint-disable-next-line no-restricted-syntax
-      for (const childItem of childItems) {
-        if (childItem.key === selectedKey) {
-          return position;
-        }
-
-        position += getItemHeight(childItem);
-      }
-    } else {
-      position += getItemHeight(itemOrSection);
-    }
+    position += itemHeight;
   }
 
   return topOffset;
diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx
index 85e193c786..838aa08e7a 100644
--- a/packages/jsapi-components/src/spectrum/Picker.tsx
+++ b/packages/jsapi-components/src/spectrum/Picker.tsx
@@ -7,6 +7,7 @@ import {
   NormalizedSectionData,
   PickerNormalized,
   PickerProps as PickerBaseProps,
+  useSpectrumThemeProvider,
 } from '@deephaven/components';
 import { dh as DhType } from '@deephaven/jsapi-types';
 import { Settings } from '@deephaven/jsapi-utils';
@@ -41,6 +42,9 @@ export function Picker({
   onSelectionChange,
   ...props
 }: PickerProps): JSX.Element {
+  const { scale } = useSpectrumThemeProvider();
+  const itemHeight = PICKER_ITEM_HEIGHTS[scale];
+
   const { getFormattedString: formatValue } = useFormatter(settings);
 
   // `null` is a valid value for `selectedKey` in controlled mode, so we check
@@ -75,8 +79,8 @@ export function Picker({
       return null;
     }
 
-    return index * PICKER_ITEM_HEIGHTS.noDescription + PICKER_TOP_OFFSET;
-  }, [getItemIndexByValue]);
+    return index * itemHeight + PICKER_TOP_OFFSET;
+  }, [getItemIndexByValue, itemHeight]);
 
   const { viewportData, onScroll, setViewport } = useViewportData<
     NormalizedItemData | NormalizedSectionData,
@@ -84,7 +88,7 @@ export function Picker({
   >({
     reuseItemsOnTableResize: true,
     table,
-    itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+    itemHeight,
     deserializeRow,
   });
 
diff --git a/packages/utils/src/UIConstants.ts b/packages/utils/src/UIConstants.ts
index fcaf459c4e..2a5d7b962a 100644
--- a/packages/utils/src/UIConstants.ts
+++ b/packages/utils/src/UIConstants.ts
@@ -2,14 +2,11 @@ export const ACTION_ICON_HEIGHT = 24;
 export const COMBO_BOX_ITEM_HEIGHT = 32;
 export const COMBO_BOX_TOP_OFFSET = 4;
 export const ITEM_KEY_PREFIX = 'DH_ITEM_KEY';
+// https://github.com/adobe/react-spectrum/blob/main/packages/%40react-spectrum/listbox/src/ListBoxBase.tsx#L56
 export const PICKER_ITEM_HEIGHTS = {
-  noDescription: 32,
-  withDescription: 48,
+  medium: 32,
+  large: 48,
 } as const;
-export const PICKER_SECTION_HEIGHTS = {
-  title: 33,
-  emptyTitle: 5,
-};
 export const PICKER_TOP_OFFSET = 4;
 export const TABLE_ROW_HEIGHT = 33;
 export const SCROLL_DEBOUNCE_MS = 150;

From b5a6d76fa778ed5dbdacff388252ff81d4c85d69 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 10:59:54 -0500
Subject: [PATCH 22/30] Tests (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    |   2 +-
 .../picker/usePickerScrollOnOpen.test.ts      |   0
 .../src/spectrum/utils/itemUtils.test.tsx     | 113 +++++++++++++++++-
 .../src/spectrum/utils/itemUtils.ts           |   2 +-
 4 files changed, 114 insertions(+), 3 deletions(-)
 create mode 100644 packages/components/src/spectrum/picker/usePickerScrollOnOpen.test.ts

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index a266cc8c6b..008117d700 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -42,7 +42,7 @@ export function Pickers(): JSX.Element {
     async () =>
       getPositionOfSelectedItem({
         keyedItems: items,
-        itemHeight: PICKER_ITEM_HEIGHTS.noDescription,
+        itemHeight: PICKER_ITEM_HEIGHTS.medium,
         selectedKey,
         topOffset: PICKER_TOP_OFFSET,
       }),
diff --git a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.test.ts b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.test.ts
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/packages/components/src/spectrum/utils/itemUtils.test.tsx b/packages/components/src/spectrum/utils/itemUtils.test.tsx
index 0585cfb8c8..ad4a985496 100644
--- a/packages/components/src/spectrum/utils/itemUtils.test.tsx
+++ b/packages/components/src/spectrum/utils/itemUtils.test.tsx
@@ -15,10 +15,13 @@ import {
   ItemOrSection,
   SectionElement,
   itemSelectionToStringSet,
+  getPositionOfSelectedItemElement,
+  isItemElementWithDescription,
 } from './itemUtils';
 import type { PickerProps } from '../picker/Picker';
 import { Item, Section } from '../shared';
 import { Text } from '../Text';
+import ItemContent from '../ItemContent';
 
 beforeEach(() => {
   expect.hasAssertions();
@@ -76,7 +79,10 @@ const expectedItems = {
       <span>No textValue</span>
     </Item>,
     {
-      item: { content: <span>No textValue</span> },
+      item: {
+        content: <span>No textValue</span>,
+        textValue: undefined,
+      },
     },
   ],
   explicitKey: [
@@ -171,6 +177,111 @@ describe('getItemKey', () => {
   );
 });
 
+describe('getPositionOfSelectedItemElement', () => {
+  const items = [
+    <Item key="1">A</Item>,
+    <Item key="2">B</Item>,
+    <Item key="3">C</Item>,
+  ];
+  const itemHeight = 40;
+  const topOffset = 5;
+
+  it.each([null, undefined])(
+    'should return top offset if selectedKey is not defined: %s',
+    async selectedKey => {
+      const actual = await getPositionOfSelectedItemElement({
+        items,
+        itemHeight,
+        selectedKey,
+        topOffset,
+      });
+
+      expect(actual).toEqual(topOffset);
+    }
+  );
+
+  it('should return top offset if selectedKey is not found', async () => {
+    const selectedKey = '4';
+
+    const actual = await getPositionOfSelectedItemElement({
+      items,
+      itemHeight,
+      selectedKey,
+      topOffset,
+    });
+
+    expect(actual).toEqual(topOffset);
+  });
+
+  it.each(['1', '2', '3'])(
+    'should return the position of the selected item element: %s',
+    async selectedKey => {
+      const expected = (Number(selectedKey) - 1) * itemHeight + topOffset;
+
+      const actual = await getPositionOfSelectedItemElement({
+        items,
+        itemHeight,
+        selectedKey,
+        topOffset,
+      });
+
+      expect(actual).toEqual(expected);
+    }
+  );
+});
+
+describe('isItemElementWithDescription', () => {
+  it.each([
+    [
+      'Item with description',
+      true,
+      <Item key="1">
+        <Text>Label</Text>
+        <Text slot="description">Description</Text>
+      </Item>,
+    ],
+    [
+      'ItemContent with description',
+      true,
+      <Item key="1">
+        <ItemContent>
+          <Text>Label</Text>
+          <Text slot="description">Description</Text>
+        </ItemContent>
+      </Item>,
+    ],
+    [
+      'Section with Item description',
+      false,
+      <Section key="1">
+        <Item key="1">
+          <Text>Label</Text>
+          <Text slot="description">Description</Text>
+        </Item>
+      </Section>,
+    ],
+    [
+      'Item no description',
+      false,
+      <Item key="1">
+        <Text>Label</Text>
+      </Item>,
+    ],
+    [
+      'ItemContent no description',
+      false,
+      <Item key="1">
+        <ItemContent>
+          <Text>Label</Text>
+        </ItemContent>
+      </Item>,
+    ],
+  ])(`%s should return %s`, (_label, expected, node) => {
+    const actual = isItemElementWithDescription(node);
+    expect(actual).toEqual(expected);
+  });
+});
+
 describe('isNormalizedItemsWithKeysList', () => {
   const mock = {
     normalizedItemWithKey: {
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index 5a13d5a548..66498ae4b7 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -54,7 +54,7 @@ export type ItemSelectionChangeHandler = (key: ItemKey) => void;
 export interface NormalizedItemData {
   key?: ItemKey;
   content: ReactNode;
-  textValue: string;
+  textValue: string | undefined;
 }
 
 export interface NormalizedSectionData {

From 678b74bdd222f71fb740507d9de8b3770d36017a Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 11:18:56 -0500
Subject: [PATCH 23/30] usePickerScrollOnOpen tests (#1935)

---
 .../picker/usePickerScrollOnOpen.test.ts      | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.test.ts b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.test.ts
index e69de29bb2..fa829ce5d2 100644
--- a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.test.ts
+++ b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.test.ts
@@ -0,0 +1,71 @@
+import { renderHook } from '@testing-library/react-hooks';
+import { TestUtils } from '@deephaven/utils';
+import {
+  findSpectrumPickerScrollArea,
+  usePopoverOnScrollRef,
+  UsePopoverOnScrollRefResult,
+} from '@deephaven/react-hooks';
+import { usePickerScrollOnOpen } from './usePickerScrollOnOpen';
+
+const { asMock, createMockProxy } = TestUtils;
+
+jest.mock('@deephaven/react-hooks');
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  expect.hasAssertions();
+});
+
+describe('usePickerScrollOnOpen', () => {
+  const getInitialScrollPosition = jest
+    .fn()
+    .mockName('getInitialScrollPosition');
+  const onScroll = jest.fn().mockName('onScroll');
+  const onOpenChange = jest.fn().mockName('onOpenChange');
+
+  const mockUsePopoverOnScrollRefResult =
+    createMockProxy<UsePopoverOnScrollRefResult<HTMLElement>>();
+
+  beforeEach(() => {
+    asMock(usePopoverOnScrollRef)
+      .mockName('usePopoverOnScrollRef')
+      .mockReturnValue(mockUsePopoverOnScrollRefResult);
+  });
+
+  it('should return ref from usePopoverOnScrollRef', () => {
+    const { result } = renderHook(() =>
+      usePickerScrollOnOpen({
+        getInitialScrollPosition,
+        onScroll,
+        onOpenChange,
+      })
+    );
+
+    expect(usePopoverOnScrollRef).toHaveBeenCalledWith(
+      findSpectrumPickerScrollArea,
+      onScroll,
+      getInitialScrollPosition
+    );
+    expect(result.current.ref).toBe(mockUsePopoverOnScrollRefResult.ref);
+  });
+
+  it.each([true, false])(
+    'should return a callback that calls popoverOnOpenChange and onOpenChange: %s',
+    isOpen => {
+      const { result } = renderHook(() =>
+        usePickerScrollOnOpen({
+          getInitialScrollPosition,
+          onScroll,
+          onOpenChange,
+        })
+      );
+
+      result.current.onOpenChange(isOpen);
+
+      expect(mockUsePopoverOnScrollRefResult.onOpenChange).toHaveBeenCalledWith(
+        isOpen
+      );
+      expect(onOpenChange).toHaveBeenCalledWith(isOpen);
+    }
+  );
+});

From 6a87a6ff48a3c73c38f5ed8a5fd88dec99e01488 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 11:24:10 -0500
Subject: [PATCH 24/30] Comments (#1935)

---
 .../src/spectrum/picker/usePickerScrollOnOpen.ts         | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
index 635b4adec6..e043eb5b3f 100644
--- a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
+++ b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
@@ -16,6 +16,15 @@ export interface UsePickerScrollOnOpenResult {
   onOpenChange: (isOpen: boolean) => void;
 }
 
+/**
+ * Handle scroll event registration and scrolling to initial scroll position
+ * whenever a Picker popover is opened.
+ * @param getInitialScrollPosition Function to get the initial scroll position.
+ * @param onScroll Callback for scroll events.
+ * @param onOpenChange Callback for open change events.
+ * @return A ref to attach to the Picker and a callback to handle open change
+ * events for the Picker.
+ */
 export function usePickerScrollOnOpen({
   getInitialScrollPosition,
   onScroll,

From 4122ef58c05dc1eec53a101513da6d60d2024749 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 11:26:31 -0500
Subject: [PATCH 25/30] Comment (#1935)

---
 packages/components/src/spectrum/utils/itemUtils.ts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index 66498ae4b7..65be80f05e 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -107,6 +107,11 @@ export function getItemKey<
 
 /**
  * Get the position of the item with the given selected key in a list of items.
+ * @param items The items to search
+ * @param itemHeight The height of each item
+ * @param selectedKey The key of the selected item
+ * @param topOffset The top offset of the list
+ * @returns The position of the selected item or the top offset if not found
  */
 export async function getPositionOfSelectedItemElement<
   TKey extends string | number | boolean | undefined,

From 3bc0bdd48816d6d681bd2307b5b059bed582bae3 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 11:39:13 -0500
Subject: [PATCH 26/30] test coverage (#1935)

---
 .../utils/useStringifiedSelection.test.ts     | 49 +++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/packages/components/src/spectrum/utils/useStringifiedSelection.test.ts b/packages/components/src/spectrum/utils/useStringifiedSelection.test.ts
index b1b179b08b..3d442397bf 100644
--- a/packages/components/src/spectrum/utils/useStringifiedSelection.test.ts
+++ b/packages/components/src/spectrum/utils/useStringifiedSelection.test.ts
@@ -18,6 +18,28 @@ describe('useStringifiedSelection', () => {
   const defaultSelectedStringKey = '4';
   const disabledStringKeys = new Set(['7', '8', '9']);
 
+  it.each([null, undefined])(
+    'should return null or undefined for null or undefined keys: %s',
+    nullOrUndefinedKey => {
+      const { result } = renderHook(() =>
+        useStringifiedSelection({
+          normalizedItems,
+          selectedKey: nullOrUndefinedKey,
+          defaultSelectedKey: undefined,
+          disabledKeys: undefined,
+          onChange: undefined,
+        })
+      );
+
+      expect(result.current).toEqual({
+        selectedStringKey: nullOrUndefinedKey,
+        defaultSelectedStringKey: undefined,
+        disabledStringKeys: undefined,
+        onStringSelectionChange: expect.any(Function),
+      });
+    }
+  );
+
   it('should stringify selections', () => {
     const { result } = renderHook(() =>
       useStringifiedSelection({
@@ -37,7 +59,28 @@ describe('useStringifiedSelection', () => {
     });
   });
 
-  it('should call onChange with actual key', () => {
+  it.each([undefined, jest.fn().mockName('onChange')])(
+    'should call onChange with actual key: %s',
+    onChange => {
+      const { result } = renderHook(() =>
+        useStringifiedSelection({
+          normalizedItems,
+          selectedKey,
+          defaultSelectedKey,
+          disabledKeys,
+          onChange,
+        })
+      );
+
+      result.current.onStringSelectionChange('2');
+
+      if (onChange) {
+        expect(onChange).toHaveBeenCalledWith(2);
+      }
+    }
+  );
+
+  it('should call onChange with given key when actual key is not found', () => {
     const onChange = jest.fn().mockName('onChange');
 
     const { result } = renderHook(() =>
@@ -50,8 +93,8 @@ describe('useStringifiedSelection', () => {
       })
     );
 
-    result.current.onStringSelectionChange('2');
+    result.current.onStringSelectionChange('some.key');
 
-    expect(onChange).toHaveBeenCalledWith(2);
+    expect(onChange).toHaveBeenCalledWith('some.key');
   });
 });

From 4a7539af13a5842dfb7582cb81322fc7ea707e4a Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 15:23:18 -0500
Subject: [PATCH 27/30] Styleguide tweak (#1935)

---
 .../code-studio/src/styleguide/Pickers.tsx    | 45 +++++++++++--------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx
index 008117d700..f6a035a6fc 100644
--- a/packages/code-studio/src/styleguide/Pickers.tsx
+++ b/packages/code-studio/src/styleguide/Pickers.tsx
@@ -1,4 +1,4 @@
-import React, { useCallback, useState } from 'react';
+import React, { cloneElement, useCallback, useState } from 'react';
 import {
   Flex,
   Item,
@@ -27,6 +27,31 @@ const itemElementsC = [...generateItemElements(104, 155)];
 const itemElementsD = [...generateItemElements(156, 207)];
 const itemElementsE = [...generateItemElements(208, 259)];
 
+const mixedItemsWithIconsNoDescriptions = [
+  'String 1',
+  'String 2',
+  'String 3',
+  '',
+  'Some really long text that should get truncated',
+  444,
+  999,
+  true,
+  false,
+  ...itemElementsA.map((itemEl, i) =>
+    i % 5 > 0
+      ? itemEl
+      : cloneElement(itemEl, {
+          ...itemEl.props,
+          children: [
+            <PersonIcon key={`icon-${itemEl.props.children}`} />,
+            <Text key={`label-${itemEl.props.children}`}>
+              {itemEl.props.children}
+            </Text>,
+          ],
+        })
+  ),
+];
+
 function PersonIcon(): JSX.Element {
   return (
     <Icon>
@@ -64,23 +89,7 @@ export function Pickers(): JSX.Element {
         </Picker>
 
         <Picker label="Mixed Children Types" defaultSelectedKey="999" tooltip>
-          {/* eslint-disable react/jsx-curly-brace-presence */}
-          {'String 1'}
-          {'String 2'}
-          {'String 3'}
-          {''}
-          {'Some really long text that should get truncated'}
-          {/* eslint-enable react/jsx-curly-brace-presence */}
-          {444}
-          {999}
-          {true}
-          {false}
-          <Item textValue="Item Aaa">Item Aaa</Item>
-          <Item textValue="Item Bbb">Item Bbb</Item>
-          <Item textValue="Complex Ccc">
-            <PersonIcon />
-            <Text>Complex Ccc with text that should be truncated</Text>
-          </Item>
+          {mixedItemsWithIconsNoDescriptions}
         </Picker>
 
         <Picker label="Sections" tooltip>

From fa5e1798562bb8a3a15f116e67f9a97b3c066281 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Mon, 22 Apr 2024 16:01:11 -0500
Subject: [PATCH 28/30] Added to index (#1935)

---
 packages/components/src/spectrum/utils/index.ts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/components/src/spectrum/utils/index.ts b/packages/components/src/spectrum/utils/index.ts
index 72e2ecfe4d..e70285dc86 100644
--- a/packages/components/src/spectrum/utils/index.ts
+++ b/packages/components/src/spectrum/utils/index.ts
@@ -1,4 +1,5 @@
 export * from './itemUtils';
+export * from './itemWrapperUtils';
 export * from './themeUtils';
 export * from './useRenderNormalizedItem';
 export * from './useStringifiedMultiSelection';

From 7c25f9fb2c367d5d24de871d3bd511d58568f149 Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 24 Apr 2024 09:57:42 -0500
Subject: [PATCH 29/30] Review comments (#1935)

---
 packages/code-studio/src/styleguide/utils.ts                  | 4 ++--
 .../components/src/spectrum/picker/usePickerScrollOnOpen.ts   | 4 ++--
 packages/components/src/spectrum/utils/itemUtils.ts           | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/code-studio/src/styleguide/utils.ts b/packages/code-studio/src/styleguide/utils.ts
index 8707ce5826..e3a778d764 100644
--- a/packages/code-studio/src/styleguide/utils.ts
+++ b/packages/code-studio/src/styleguide/utils.ts
@@ -23,7 +23,7 @@ export function* generateItemElements(
     }
     const letter = letters[charI];
     const key = `${letter}${suffix}`;
-    const content = `${letter}${letter}${letter}${suffix}`;
+    const content = `${letter.repeat(3)}${suffix}`;
 
     // eslint-disable-next-line react/no-children-prop
     yield createElement(Item, {
@@ -52,7 +52,7 @@ export function* generateNormalizedItems(
     }
     const letter = letters[charI];
     const key = `${letter}${suffix}`;
-    const content = `${letter}${letter}${letter}${suffix}`;
+    const content = `${letter.repeat(3)}${suffix}`;
 
     yield {
       key,
diff --git a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
index e043eb5b3f..d9ad3114f4 100644
--- a/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
+++ b/packages/components/src/spectrum/picker/usePickerScrollOnOpen.ts
@@ -1,5 +1,5 @@
 import { useCallback } from 'react';
-import type { DOMRefValue } from '@react-types/shared';
+import type { DOMRef } from '@react-types/shared';
 import {
   findSpectrumPickerScrollArea,
   usePopoverOnScrollRef,
@@ -12,7 +12,7 @@ export interface UsePickerScrollOnOpenOptions {
 }
 
 export interface UsePickerScrollOnOpenResult {
-  ref: React.RefObject<DOMRefValue<HTMLElement>>;
+  ref: DOMRef<HTMLElement>;
   onOpenChange: (isOpen: boolean) => void;
 }
 
diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index 65be80f05e..e293b46a67 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -132,8 +132,8 @@ export async function getPositionOfSelectedItemElement<
     return position;
   }
 
-  // eslint-disable-next-line no-restricted-syntax
-  for (const item of items) {
+  for (let i = 0; i <= items.length; i += 1) {
+    const item = items[i];
     if (item.key === selectedKey) {
       return position;
     }

From 1f37fdea119c57b680808546e0b38f25fe6a71bf Mon Sep 17 00:00:00 2001
From: Brian Ingles <brianingles@deephaven.io>
Date: Wed, 24 Apr 2024 10:21:21 -0500
Subject: [PATCH 30/30] Fixed off by 1 bug (#1935)

---
 packages/components/src/spectrum/utils/itemUtils.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts
index e293b46a67..a103ea03fe 100644
--- a/packages/components/src/spectrum/utils/itemUtils.ts
+++ b/packages/components/src/spectrum/utils/itemUtils.ts
@@ -132,7 +132,7 @@ export async function getPositionOfSelectedItemElement<
     return position;
   }
 
-  for (let i = 0; i <= items.length; i += 1) {
+  for (let i = 0; i < items.length; i += 1) {
     const item = items[i];
     if (item.key === selectedKey) {
       return position;