From ed6ed2a8b74d41138f1f003cbcdc25dbe69f3dc8 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 2 Apr 2024 09:48:53 -0500 Subject: [PATCH 01/41] Generics to narrow types to item only when appropriate (#1909) --- .../src/spectrum/utils/itemUtils.ts | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index 06f5e852dc..b00db9af65 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -67,6 +67,9 @@ export type NormalizedSection = KeyedItem< Key | undefined >; +export type NormalizedItemOrSection = + TItemOrSection extends SectionElement ? NormalizedSection : NormalizedItem; + export type NormalizedSpectrumPickerProps = SpectrumPickerProps; export type TooltipOptions = { placement: PopperOptions['placement'] }; @@ -114,14 +117,19 @@ export function isItemElement( } /** - * Determine if a node is an array containing normalized items with keys. - * Note that this only checks the first node in the array. + * Determine if a node is an array containing normalized items or sections with + * keys. Note that this only checks the first node in the array. * @param node The node to check - * @returns True if the node is a normalized item with keys array + * @returns True if the node is a normalized item or section with keys array */ -export function isNormalizedItemsWithKeysList( - node: ItemOrSection | ItemOrSection[] | (NormalizedItem | NormalizedSection)[] -): node is (NormalizedItem | NormalizedSection)[] { +export function isNormalizedItemsWithKeysList< + TItemOrSection extends ItemOrSection, +>( + node: + | TItemOrSection + | TItemOrSection[] + | NormalizedItemOrSection[] +): node is NormalizedItemOrSection[] { if (!Array.isArray(node)) { return false; } @@ -225,9 +233,9 @@ function normalizeTextValue(item: ItemElementOrPrimitive): string | undefined { * @param itemOrSection item to normalize * @returns NormalizedItem or NormalizedSection object */ -function normalizeItem( - itemOrSection: ItemOrSection -): NormalizedItem | NormalizedSection { +function normalizeItem( + itemOrSection: TItemOrSection +): NormalizedItemOrSection { if (!isItemOrSection(itemOrSection)) { log.debug(INVALID_ITEM_ERROR_MESSAGE, itemOrSection); throw new Error(INVALID_ITEM_ERROR_MESSAGE); @@ -244,7 +252,7 @@ function normalizeItem( return { item: { key, title, items }, - }; + } as NormalizedItemOrSection; } const key = normalizeItemKey(itemOrSection); @@ -255,23 +263,23 @@ function normalizeItem( return { item: { key, content, textValue }, - }; + } as NormalizedItemOrSection; } /** - * Get normalized items from an item or array of items. - * @param itemsOrSections An item or array of items - * @returns An array of normalized items + * Normalize an item or section or a list of items or sections. + * @param itemsOrSections An item or section or array of items or sections + * @returns An array of normalized items or sections */ -export function normalizeItemList( - itemsOrSections: ItemOrSection | ItemOrSection[] | NormalizedItem[] -): (NormalizedItem | NormalizedSection)[] { +export function normalizeItemList( + itemsOrSections: TItemOrSection | TItemOrSection[] | NormalizedItem[] +): NormalizedItemOrSection[] { // If already normalized, just return as-is if (isNormalizedItemsWithKeysList(itemsOrSections)) { - return itemsOrSections; + return itemsOrSections as NormalizedItemOrSection[]; } - const itemsArray = Array.isArray(itemsOrSections) + const itemsArray: TItemOrSection[] = Array.isArray(itemsOrSections) ? itemsOrSections : [itemsOrSections]; From 22191159cb9721e4933af46085c37fd6f42ec015 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 2 Apr 2024 10:04:48 -0500 Subject: [PATCH 02/41] Tooltip placement override (#1909) --- .../components/src/spectrum/utils/itemUtils.test.tsx | 5 +++++ packages/components/src/spectrum/utils/itemUtils.ts | 9 ++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/components/src/spectrum/utils/itemUtils.test.tsx b/packages/components/src/spectrum/utils/itemUtils.test.tsx index e2cdad44c7..4be6d49580 100644 --- a/packages/components/src/spectrum/utils/itemUtils.test.tsx +++ b/packages/components/src/spectrum/utils/itemUtils.test.tsx @@ -289,4 +289,9 @@ describe('normalizeTooltipOptions', () => { const actual = normalizeTooltipOptions(options); expect(actual).toEqual(expected); }); + + it('should allow overriding default placement', () => { + const actual = normalizeTooltipOptions(true, 'top'); + expect(actual).toEqual({ placement: 'top' }); + }); }); diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index b00db9af65..eb7065b94b 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -288,18 +288,21 @@ export function normalizeItemList( /** * Returns a TooltipOptions object or null if options is false or null. - * @param options + * @param options Tooltip options + * @param placement Default placement for the tooltip if `options` is set + * explicitly to `true` * @returns TooltipOptions or null */ export function normalizeTooltipOptions( - options?: boolean | TooltipOptions | null + options?: boolean | TooltipOptions | null, + placement: TooltipOptions['placement'] = 'right' ): TooltipOptions | null { if (options == null || options === false) { return null; } if (options === true) { - return { placement: 'right' }; + return { placement }; } return options; From 5a98665d3e5915b344bacc3b8449fe1231fdf758 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 2 Apr 2024 10:33:06 -0500 Subject: [PATCH 03/41] Rename / move PickerItemContent (#1909) --- .../{picker/PickerItemContent.tsx => ItemContent.tsx} | 10 +++++----- packages/components/src/spectrum/index.ts | 1 + packages/components/src/spectrum/picker/Picker.tsx | 4 ++-- packages/components/src/spectrum/picker/index.ts | 1 - 4 files changed, 8 insertions(+), 8 deletions(-) rename packages/components/src/spectrum/{picker/PickerItemContent.tsx => ItemContent.tsx} (88%) diff --git a/packages/components/src/spectrum/picker/PickerItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx similarity index 88% rename from packages/components/src/spectrum/picker/PickerItemContent.tsx rename to packages/components/src/spectrum/ItemContent.tsx index d680c56ed2..7f4af2059d 100644 --- a/packages/components/src/spectrum/picker/PickerItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -2,9 +2,9 @@ import { Children, cloneElement, isValidElement, ReactNode } from 'react'; import { Text } from '@adobe/react-spectrum'; import cl from 'classnames'; import { isElementOfType } from '@deephaven/react-hooks'; -import stylesCommon from '../../SpectrumComponent.module.scss'; +import stylesCommon from '../SpectrumComponent.module.scss'; -export interface PickerItemContentProps { +export interface ItemContentProps { children: ReactNode; } @@ -12,9 +12,9 @@ export interface PickerItemContentProps { * Picker item content. Text content will be wrapped in a Spectrum Text * component with ellipsis overflow handling. */ -export function PickerItemContent({ +export function ItemContent({ children: content, -}: PickerItemContentProps): JSX.Element | null { +}: ItemContentProps): JSX.Element | null { if (isValidElement(content)) { return content; } @@ -57,4 +57,4 @@ export function PickerItemContent({ ); } -export default PickerItemContent; +export default ItemContent; diff --git a/packages/components/src/spectrum/index.ts b/packages/components/src/spectrum/index.ts index e001ef8beb..bdb4f7a506 100644 --- a/packages/components/src/spectrum/index.ts +++ b/packages/components/src/spectrum/index.ts @@ -24,4 +24,5 @@ export * from './View'; /** * Custom DH spectrum utils */ +export * from './ItemContent'; export * from './utils'; diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx index e706ede9d8..8d533480a5 100644 --- a/packages/components/src/spectrum/picker/Picker.tsx +++ b/packages/components/src/spectrum/picker/Picker.tsx @@ -25,7 +25,7 @@ import { ItemKey, getItemKey, } from '../utils/itemUtils'; -import { PickerItemContent } from './PickerItemContent'; +import { ItemContent } from '../ItemContent'; import { Item, Section } from '../shared'; import { Text } from '../Text'; @@ -143,7 +143,7 @@ export function Picker({ textValue={textValue === '' ? 'Empty' : textValue} > <> - {content} + {content} {tooltipOptions == null || content === '' ? null : ( {createTooltipContent(content)} diff --git a/packages/components/src/spectrum/picker/index.ts b/packages/components/src/spectrum/picker/index.ts index b666d3021b..c434d5d810 100644 --- a/packages/components/src/spectrum/picker/index.ts +++ b/packages/components/src/spectrum/picker/index.ts @@ -1,2 +1 @@ export * from './Picker'; -export * from './PickerItemContent'; From 991042f5651dc2459253aaf14d387ab802254c8a Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 2 Apr 2024 12:26:29 -0500 Subject: [PATCH 04/41] Smarter item tooltips (#1909) --- .../components/src/spectrum/ItemContent.tsx | 76 +++++++++++++++++-- .../components/src/spectrum/ItemTooltip.tsx | 34 +++++++++ packages/components/src/spectrum/Text.tsx | 34 ++++++--- packages/components/src/spectrum/index.ts | 1 + .../components/src/spectrum/picker/Picker.tsx | 36 +-------- 5 files changed, 128 insertions(+), 53 deletions(-) create mode 100644 packages/components/src/spectrum/ItemTooltip.tsx diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx index 7f4af2059d..d5c92180d4 100644 --- a/packages/components/src/spectrum/ItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -1,20 +1,63 @@ -import { Children, cloneElement, isValidElement, ReactNode } from 'react'; -import { Text } from '@adobe/react-spectrum'; +import { + Children, + cloneElement, + isValidElement, + ReactNode, + useCallback, + useState, +} from 'react'; +import { DOMRefValue } from '@react-types/shared'; import cl from 'classnames'; import { isElementOfType } from '@deephaven/react-hooks'; +import { Text } from './Text'; import stylesCommon from '../SpectrumComponent.module.scss'; +import { TooltipOptions } from './utils'; +import ItemTooltip from './ItemTooltip'; export interface ItemContentProps { children: ReactNode; + tooltipOptions?: TooltipOptions | null; } /** * Picker item content. Text content will be wrapped in a Spectrum Text - * component with ellipsis overflow handling. + * component with ellipsis overflow handling. If text content overflow and + * tooltipOptions are provided a tooltip will be displayed when hovering over + * the item content. */ export function ItemContent({ children: content, + tooltipOptions, }: ItemContentProps): JSX.Element | null { + const [previousContent, setPreviousContent] = useState(content); + const [isOverflowing, setIsOverflowing] = useState(false); + + // Reset `isOverflowing` if content changes. It will get re-calculated as + // `Text` components render. + if (previousContent !== content) { + setPreviousContent(content); + setIsOverflowing(false); + } + + /** + * Whenever a `Text` component renders, see if the content is overflowing so + * we can render a tooltip. + */ + const checkOverflow = useCallback( + (ref: DOMRefValue | null) => { + const el = ref?.UNSAFE_getDOMNode(); + + if (el == null) { + return; + } + + if (el.scrollWidth > el.offsetWidth) { + setIsOverflowing(true); + } + }, + [] + ); + if (isValidElement(content)) { return content; } @@ -39,6 +82,7 @@ export function ItemContent({ isElementOfType(el, Text) ? cloneElement(el, { ...el.props, + ref: checkOverflow, UNSAFE_className: cl( el.props.UNSAFE_className, stylesCommon.spectrumEllipsis @@ -47,13 +91,29 @@ export function ItemContent({ : el ); } + + if (typeof content === 'string' || typeof content === 'number') { + content = ( + + {content} + + ); + } /* eslint-enable no-param-reassign */ - return typeof content === 'string' || typeof content === 'number' ? ( - {content} - ) : ( - // eslint-disable-next-line react/jsx-no-useless-fragment - <>{content} + const tooltip = + tooltipOptions == null || !isOverflowing ? null : ( + {content} + ); + + return ( + <> + {content} + {tooltip} + ); } diff --git a/packages/components/src/spectrum/ItemTooltip.tsx b/packages/components/src/spectrum/ItemTooltip.tsx new file mode 100644 index 0000000000..d2f031457d --- /dev/null +++ b/packages/components/src/spectrum/ItemTooltip.tsx @@ -0,0 +1,34 @@ +import { ReactNode } from 'react'; +import { isElementOfType } from '@deephaven/react-hooks'; +import { TooltipOptions } from './utils'; +import { Tooltip } from '../popper'; +import { Flex } from './layout'; +import { Text } from './Text'; + +export interface ItemTooltipProps { + children: ReactNode; + options: TooltipOptions; +} + +export function ItemTooltip({ + children, + options, +}: ItemTooltipProps): JSX.Element { + if (typeof children === 'boolean') { + return {children}; + } + + if (Array.isArray(children)) { + return ( + + + {children.filter(node => isElementOfType(node, Text))} + + + ); + } + + return {children}; +} + +export default ItemTooltip; diff --git a/packages/components/src/spectrum/Text.tsx b/packages/components/src/spectrum/Text.tsx index 3164c2eafd..da6c7348eb 100644 --- a/packages/components/src/spectrum/Text.tsx +++ b/packages/components/src/spectrum/Text.tsx @@ -1,9 +1,10 @@ /* eslint-disable react/jsx-props-no-spreading */ -import { useMemo } from 'react'; +import { forwardRef, useMemo } from 'react'; import { Text as SpectrumText, type TextProps as SpectrumTextProps, } from '@adobe/react-spectrum'; +import type { DOMRef, DOMRefValue } from '@react-types/shared'; import { type ColorValue, colorValueStyle } from '../theme/colorUtils'; export type TextProps = SpectrumTextProps & { @@ -19,18 +20,27 @@ export type TextProps = SpectrumTextProps & { * @returns The Text component * */ +export const Text = forwardRef, SpectrumTextProps>( + (props: TextProps, ref): JSX.Element => { + const { color, UNSAFE_style, ...rest } = props; + const style = useMemo( + () => ({ + ...UNSAFE_style, + color: colorValueStyle(color), + }), + [color, UNSAFE_style] + ); -export function Text(props: TextProps): JSX.Element { - const { color, UNSAFE_style, ...rest } = props; - const style = useMemo( - () => ({ - ...UNSAFE_style, - color: colorValueStyle(color), - }), - [color, UNSAFE_style] - ); + return ( + } + UNSAFE_style={style} + /> + ); + } +); - return ; -} +Text.displayName = 'Text'; export default Text; diff --git a/packages/components/src/spectrum/index.ts b/packages/components/src/spectrum/index.ts index bdb4f7a506..8db2ca0cb3 100644 --- a/packages/components/src/spectrum/index.ts +++ b/packages/components/src/spectrum/index.ts @@ -25,4 +25,5 @@ export * from './View'; * Custom DH spectrum utils */ export * from './ItemContent'; +export * from './ItemTooltip'; export * from './utils'; diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx index 8d533480a5..e1ac6cae2e 100644 --- a/packages/components/src/spectrum/picker/Picker.tsx +++ b/packages/components/src/spectrum/picker/Picker.tsx @@ -1,10 +1,9 @@ -import { Key, ReactNode, useCallback, useMemo } from 'react'; +import { Key, useCallback, useMemo } from 'react'; import { DOMRef } from '@react-types/shared'; -import { Flex, Picker as SpectrumPicker } from '@adobe/react-spectrum'; +import { Picker as SpectrumPicker } from '@adobe/react-spectrum'; import { getPositionOfSelectedItem, findSpectrumPickerScrollArea, - isElementOfType, usePopoverOnScrollRef, } from '@deephaven/react-hooks'; import { @@ -13,7 +12,6 @@ import { PICKER_TOP_OFFSET, } from '@deephaven/utils'; import cl from 'classnames'; -import { Tooltip } from '../../popper'; import { isNormalizedSection, NormalizedSpectrumPickerProps, @@ -27,7 +25,6 @@ import { } from '../utils/itemUtils'; import { ItemContent } from '../ItemContent'; import { Item, Section } from '../shared'; -import { Text } from '../Text'; export type PickerProps = { children: ItemOrSection | ItemOrSection[] | NormalizedItem[]; @@ -69,26 +66,6 @@ export type PickerProps = { | 'defaultSelectedKey' >; -/** - * Create tooltip content optionally wrapping with a Flex column for array - * content. This is needed for Items containing description `Text` elements. - */ -function createTooltipContent(content: ReactNode) { - if (typeof content === 'boolean') { - return String(content); - } - - if (Array.isArray(content)) { - return ( - - {content.filter(node => isElementOfType(node, Text))} - - ); - } - - return content; -} - /** * 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, @@ -142,14 +119,7 @@ export function Picker({ // 'Empty' value so that they are not empty strings. textValue={textValue === '' ? 'Empty' : textValue} > - <> - {content} - {tooltipOptions == null || content === '' ? null : ( - - {createTooltipContent(content)} - - )} - + {content} ); }, From 97d74128d019b244d856ae2157be38a2d82427df Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 2 Apr 2024 12:34:17 -0500 Subject: [PATCH 05/41] forwardedRefs (#1909) --- packages/components/src/spectrum/Heading.tsx | 14 +++++++--- packages/components/src/spectrum/Text.tsx | 14 +++------- packages/components/src/spectrum/View.tsx | 29 ++++++++++++-------- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/packages/components/src/spectrum/Heading.tsx b/packages/components/src/spectrum/Heading.tsx index 38682e6ca7..f29e7b2729 100644 --- a/packages/components/src/spectrum/Heading.tsx +++ b/packages/components/src/spectrum/Heading.tsx @@ -1,9 +1,10 @@ /* eslint-disable react/jsx-props-no-spreading */ -import { useMemo } from 'react'; +import { forwardRef, useMemo } from 'react'; import { Heading as SpectrumHeading, type HeadingProps as SpectrumHeadingProps, } from '@adobe/react-spectrum'; +import type { DOMRefValue } from '@react-types/shared'; import { type ColorValue, colorValueStyle } from '../theme/colorUtils'; export type HeadingProps = SpectrumHeadingProps & { @@ -20,7 +21,10 @@ export type HeadingProps = SpectrumHeadingProps & { * */ -export function Heading(props: HeadingProps): JSX.Element { +export const Heading = forwardRef< + DOMRefValue, + HeadingProps +>((props, forwardedRef): JSX.Element => { const { color, UNSAFE_style, ...rest } = props; const style = useMemo( () => ({ @@ -30,7 +34,9 @@ export function Heading(props: HeadingProps): JSX.Element { [color, UNSAFE_style] ); - return ; -} + return ; +}); + +Heading.displayName = 'Heading'; export default Heading; diff --git a/packages/components/src/spectrum/Text.tsx b/packages/components/src/spectrum/Text.tsx index da6c7348eb..d0467f275d 100644 --- a/packages/components/src/spectrum/Text.tsx +++ b/packages/components/src/spectrum/Text.tsx @@ -4,7 +4,7 @@ import { Text as SpectrumText, type TextProps as SpectrumTextProps, } from '@adobe/react-spectrum'; -import type { DOMRef, DOMRefValue } from '@react-types/shared'; +import type { DOMRefValue } from '@react-types/shared'; import { type ColorValue, colorValueStyle } from '../theme/colorUtils'; export type TextProps = SpectrumTextProps & { @@ -20,8 +20,8 @@ export type TextProps = SpectrumTextProps & { * @returns The Text component * */ -export const Text = forwardRef, SpectrumTextProps>( - (props: TextProps, ref): JSX.Element => { +export const Text = forwardRef, TextProps>( + (props, forwardedRef): JSX.Element => { const { color, UNSAFE_style, ...rest } = props; const style = useMemo( () => ({ @@ -31,13 +31,7 @@ export const Text = forwardRef, SpectrumTextProps>( [color, UNSAFE_style] ); - return ( - } - UNSAFE_style={style} - /> - ); + return ; } ); diff --git a/packages/components/src/spectrum/View.tsx b/packages/components/src/spectrum/View.tsx index 847900541e..03c28a193e 100644 --- a/packages/components/src/spectrum/View.tsx +++ b/packages/components/src/spectrum/View.tsx @@ -1,9 +1,10 @@ /* eslint-disable react/jsx-props-no-spreading */ -import { useMemo } from 'react'; +import { forwardRef, useMemo } from 'react'; import { View as SpectrumView, type ViewProps as SpectrumViewProps, } from '@adobe/react-spectrum'; +import type { DOMRefValue } from '@react-types/shared'; import { type ColorValue, colorValueStyle } from '../theme/colorUtils'; export type ViewProps = Omit, 'backgroundColor'> & { @@ -20,17 +21,21 @@ export type ViewProps = Omit, 'backgroundColor'> & { * */ -export function View(props: ViewProps): JSX.Element { - const { backgroundColor, UNSAFE_style, ...rest } = props; - const style = useMemo( - () => ({ - ...UNSAFE_style, - backgroundColor: colorValueStyle(backgroundColor), - }), - [backgroundColor, UNSAFE_style] - ); +export const View = forwardRef, ViewProps>( + (props, forwardedRef): JSX.Element => { + const { backgroundColor, UNSAFE_style, ...rest } = props; + const style = useMemo( + () => ({ + ...UNSAFE_style, + backgroundColor: colorValueStyle(backgroundColor), + }), + [backgroundColor, UNSAFE_style] + ); - return ; -} + return ; + } +); + +View.displayName = 'View'; export default View; From b7f4df343cc12ec012184ecc1ca2af48b280c89a Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 2 Apr 2024 14:02:31 -0500 Subject: [PATCH 06/41] Basic ListView implementation (#1909) --- .../code-studio/src/styleguide/ListViews.tsx | 119 ++++++++++++++++++ .../code-studio/src/styleguide/Pickers.tsx | 2 +- .../code-studio/src/styleguide/StyleGuide.tsx | 2 + .../components/src/spectrum/collections.ts | 2 - packages/components/src/spectrum/index.ts | 1 + .../src/spectrum/listView/ListView.tsx | 103 +++++++++++++++ .../components/src/spectrum/listView/index.ts | 1 + .../components/src/spectrum/picker/Picker.tsx | 47 ++----- .../components/src/spectrum/utils/index.ts | 2 + .../utils/useRenderNormalizedItem.tsx | 39 ++++++ .../utils/useStringifiedMultiSelection.ts | 104 +++++++++++++++ 11 files changed, 384 insertions(+), 38 deletions(-) create mode 100644 packages/code-studio/src/styleguide/ListViews.tsx create mode 100644 packages/components/src/spectrum/listView/ListView.tsx create mode 100644 packages/components/src/spectrum/listView/index.ts create mode 100644 packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx create mode 100644 packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts diff --git a/packages/code-studio/src/styleguide/ListViews.tsx b/packages/code-studio/src/styleguide/ListViews.tsx new file mode 100644 index 0000000000..f521321c5f --- /dev/null +++ b/packages/code-studio/src/styleguide/ListViews.tsx @@ -0,0 +1,119 @@ +import React, { useCallback, useState } from 'react'; +import { Grid, Item, ListView, ItemKey, Text } from '@deephaven/components'; +import { vsAccount, vsPerson } from '@deephaven/icons'; +import { Icon } from '@adobe/react-spectrum'; +import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; +import { sampleSectionIdAndClasses } from './utils'; + +// Generate enough items to require scrolling +const itemsSimple = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz' + .split('') + .map((key, i) => ({ + key, + item: { key: (i + 1) * 100, content: `${key}${key}${key}` }, + })); + +function AccountIcon({ + slot, +}: { + slot?: 'illustration' | 'image'; +}): JSX.Element { + return ( + // Images in ListView items require a slot of 'image' or 'illustration' to + // be set in order to be positioned correctly: + // https://github.com/adobe/react-spectrum/blob/784737effd44b9d5e2b1316e690da44555eafd7e/packages/%40react-spectrum/list/src/ListViewItem.tsx#L266-L267 + + + + ); +} + +export function ListViews(): JSX.Element { + const [selectedKeys, setSelectedKeys] = useState<'all' | Iterable>( + [] + ); + + const onChange = useCallback((keys: 'all' | Iterable): void => { + setSelectedKeys(keys); + }, []); + + return ( + // eslint-disable-next-line react/jsx-props-no-spreading +
+

List View

+ + + Single Child + + Aaa + + + + + + + Item with icon A + + + + Item with icon B + + + + Item with icon C + + + + Item with icon D with overflowing content + + + + + + {/* 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 Aaa + Item Bbb + + + + + Complex Ccc with text that should be truncated + + + + + + {itemsSimple} + + +
+ ); +} + +export default ListViews; diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx index 18dc678c10..bee79b1d0b 100644 --- a/packages/code-studio/src/styleguide/Pickers.tsx +++ b/packages/code-studio/src/styleguide/Pickers.tsx @@ -29,7 +29,7 @@ function PersonIcon(): JSX.Element { } export function Pickers(): JSX.Element { - const [selectedKey, setSelectedKey] = useState(); + const [selectedKey, setSelectedKey] = useState(null); const onChange = useCallback((key: ItemKey): void => { setSelectedKey(key); diff --git a/packages/code-studio/src/styleguide/StyleGuide.tsx b/packages/code-studio/src/styleguide/StyleGuide.tsx index 051a58917f..72c3689777 100644 --- a/packages/code-studio/src/styleguide/StyleGuide.tsx +++ b/packages/code-studio/src/styleguide/StyleGuide.tsx @@ -36,6 +36,7 @@ import { GoldenLayout } from './GoldenLayout'; import { RandomAreaPlotAnimation } from './RandomAreaPlotAnimation'; import SpectrumComparison from './SpectrumComparison'; import Pickers from './Pickers'; +import ListViews from './ListViews'; const stickyProps = { position: 'sticky', @@ -109,6 +110,7 @@ function StyleGuide(): React.ReactElement { + diff --git a/packages/components/src/spectrum/collections.ts b/packages/components/src/spectrum/collections.ts index 619027a946..a1d502975c 100644 --- a/packages/components/src/spectrum/collections.ts +++ b/packages/components/src/spectrum/collections.ts @@ -3,8 +3,6 @@ export { type SpectrumActionBarProps as ActionBarProps, ActionMenu, type SpectrumActionMenuProps as ActionMenuProps, - ListView, - type SpectrumListViewProps as ListViewProps, MenuTrigger, type SpectrumMenuTriggerProps as MenuTriggerProps, TagGroup, diff --git a/packages/components/src/spectrum/index.ts b/packages/components/src/spectrum/index.ts index 8db2ca0cb3..02f1d4df7a 100644 --- a/packages/components/src/spectrum/index.ts +++ b/packages/components/src/spectrum/index.ts @@ -16,6 +16,7 @@ export * from './status'; /** * Custom DH components wrapping React Spectrum components. */ +export * from './listView'; export * from './picker'; export * from './Heading'; export * from './Text'; diff --git a/packages/components/src/spectrum/listView/ListView.tsx b/packages/components/src/spectrum/listView/ListView.tsx new file mode 100644 index 0000000000..ff9cb04b7f --- /dev/null +++ b/packages/components/src/spectrum/listView/ListView.tsx @@ -0,0 +1,103 @@ +import { useMemo } from 'react'; +import { + ListView as SpectrumListView, + SpectrumListViewProps, +} from '@adobe/react-spectrum'; +import cl from 'classnames'; +import { + ItemElementOrPrimitive, + ItemKey, + NormalizedItem, + normalizeItemList, + normalizeTooltipOptions, + TooltipOptions, + useRenderNormalizedItem, + useStringifiedMultiSelection, +} from '../utils'; + +export type ListViewProps = { + children: + | ItemElementOrPrimitive + | ItemElementOrPrimitive[] + | NormalizedItem[]; + /** Can be set to true or a TooltipOptions to enable item tooltips */ + tooltip?: boolean | TooltipOptions; + selectedKeys?: 'all' | Iterable; + defaultSelectedKeys?: 'all' | Iterable; + disabledKeys?: Iterable; + /** + * 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?: (keys: 'all' | Set) => void; + + /** + * Handler that is called when the selection changes. + * @deprecated Use `onChange` instead + */ + onSelectionChange?: (keys: 'all' | Set) => void; +} & Omit< + SpectrumListViewProps, + | 'children' + | 'items' + | 'selectedKeys' + | 'defaultSelectedKeys' + | 'disabledKeys' + | 'onSelectionChange' +>; + +export function ListView({ + children, + tooltip = true, + selectedKeys, + defaultSelectedKeys, + disabledKeys, + UNSAFE_className, + onChange, + onSelectionChange, + ...spectrumListViewProps +}: ListViewProps): JSX.Element { + const normalizedItems = useMemo( + () => normalizeItemList(children), + [children] + ); + + const tooltipOptions = useMemo( + () => normalizeTooltipOptions(tooltip, 'bottom'), + [tooltip] + ); + + const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions); + + const { + selectedStringKeys, + defaultSelectedStringKeys, + disabledStringKeys, + onStringSelectionChange, + } = useStringifiedMultiSelection({ + normalizedItems, + selectedKeys, + defaultSelectedKeys, + disabledKeys, + onChange: onChange ?? onSelectionChange, + }); + + return ( + + {renderNormalizedItem} + + ); +} + +export default ListView; diff --git a/packages/components/src/spectrum/listView/index.ts b/packages/components/src/spectrum/listView/index.ts new file mode 100644 index 0000000000..e1e4de2f28 --- /dev/null +++ b/packages/components/src/spectrum/listView/index.ts @@ -0,0 +1 @@ +export * from './ListView'; diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx index e1ac6cae2e..415f3051cb 100644 --- a/packages/components/src/spectrum/picker/Picker.tsx +++ b/packages/components/src/spectrum/picker/Picker.tsx @@ -1,4 +1,4 @@ -import { Key, useCallback, useMemo } from 'react'; +import { useCallback, useMemo } from 'react'; import { DOMRef } from '@react-types/shared'; import { Picker as SpectrumPicker } from '@adobe/react-spectrum'; import { @@ -23,8 +23,8 @@ import { ItemKey, getItemKey, } from '../utils/itemUtils'; -import { ItemContent } from '../ItemContent'; -import { Item, Section } from '../shared'; +import { Section } from '../shared'; +import { useRenderNormalizedItem } from '../utils'; export type PickerProps = { children: ItemOrSection | ItemOrSection[] | NormalizedItem[]; @@ -97,34 +97,7 @@ export function Picker({ [tooltip] ); - const renderItem = useCallback( - (normalizedItem: NormalizedItem) => { - const key = getItemKey(normalizedItem); - const content = normalizedItem.item?.content ?? ''; - const textValue = normalizedItem.item?.textValue ?? ''; - - return ( - ` - // 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} - > - {content} - - ); - }, - [tooltipOptions] - ); + const renderNormalizedItem = useRenderNormalizedItem(tooltipOptions); const getInitialScrollPositionInternal = useCallback( () => @@ -187,8 +160,12 @@ export function Picker({ // 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?.toString()} - defaultSelectedKey={defaultSelectedKey?.toString()} + selectedKey={selectedKey == null ? selectedKey : selectedKey.toString()} + defaultSelectedKey={ + defaultSelectedKey == null + ? defaultSelectedKey + : defaultSelectedKey.toString() + } // `onChange` is just an alias for `onSelectionChange` onSelectionChange={ onSelectionChangeInternal as NormalizedSpectrumPickerProps['onSelectionChange'] @@ -202,12 +179,12 @@ export function Picker({ title={itemOrSection.item?.title} items={itemOrSection.item?.items} > - {renderItem} + {renderNormalizedItem} ); } - return renderItem(itemOrSection); + return renderNormalizedItem(itemOrSection); }} ); diff --git a/packages/components/src/spectrum/utils/index.ts b/packages/components/src/spectrum/utils/index.ts index ab03442699..ef406aba98 100644 --- a/packages/components/src/spectrum/utils/index.ts +++ b/packages/components/src/spectrum/utils/index.ts @@ -1,2 +1,4 @@ export * from './itemUtils'; export * from './themeUtils'; +export * from './useRenderNormalizedItem'; +export * from './useStringifiedMultiSelection'; diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx new file mode 100644 index 0000000000..52a70f62dc --- /dev/null +++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx @@ -0,0 +1,39 @@ +import { Key, useCallback } from 'react'; +import { ItemContent } from '../ItemContent'; +import { Item } from '../shared'; +import { getItemKey, NormalizedItem, TooltipOptions } from './itemUtils'; + +export function useRenderNormalizedItem( + tooltipOptions: TooltipOptions | null +): (normalizedItem: NormalizedItem) => JSX.Element { + return useCallback( + (normalizedItem: NormalizedItem) => { + const key = getItemKey(normalizedItem); + const content = normalizedItem.item?.content ?? ''; + const textValue = normalizedItem.item?.textValue ?? ''; + + return ( + ` + // 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} + > + {content} + + ); + }, + [tooltipOptions] + ); +} + +export default useRenderNormalizedItem; diff --git a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts new file mode 100644 index 0000000000..cadb8a4888 --- /dev/null +++ b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts @@ -0,0 +1,104 @@ +import { Key, useCallback, useMemo } from 'react'; +import { getItemKey, ItemKey, NormalizedItem } from '.'; + +function toStringKeySet( + keys?: 'all' | Iterable +): undefined | 'all' | Set { + if (keys == null || keys === 'all') { + return keys as undefined | 'all'; + } + + return new Set([...keys].map(String)); +} + +export interface UseStringifiedMultiSelectionOptions { + normalizedItems: NormalizedItem[]; + selectedKeys?: 'all' | Iterable; + defaultSelectedKeys?: 'all' | Iterable; + disabledKeys?: Iterable; + /** + * 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?: (keys: 'all' | Set) => void; +} + +export interface UseStringifiedMultiSelectionResult { + /** Stringified selection keys */ + selectedStringKeys?: 'all' | Set; + /** Stringified default selection keys */ + defaultSelectedStringKeys?: 'all' | Set; + /** Stringified disabled keys */ + disabledStringKeys?: 'all' | Set; + /** Handler that is called when the string key selections change */ + onStringSelectionChange: (keys: 'all' | Set) => 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 selectedKeys The currently selected keys in the collection. + * @param defaultSelectedKeys The initial selected keys in the collection. + * @param disabledKeys The currently disabled keys in the collection. + * @param onChange Handler that is called when the selection changes. + * @returns UseStringifiedMultiSelectionResult with stringified key sets and + * string key selection change handler. + */ +export function useStringifiedMultiSelection({ + normalizedItems, + defaultSelectedKeys, + disabledKeys, + selectedKeys, + onChange, +}: UseStringifiedMultiSelectionOptions): UseStringifiedMultiSelectionResult { + const selectedStringKeys = useMemo( + () => toStringKeySet(selectedKeys), + [selectedKeys] + ); + + const defaultSelectedStringKeys = useMemo( + () => toStringKeySet(defaultSelectedKeys), + [defaultSelectedKeys] + ); + + const disabledStringKeys = useMemo( + () => toStringKeySet(disabledKeys), + [disabledKeys] + ); + + const onStringSelectionChange = useCallback( + (keys: 'all' | Set) => { + if (keys === 'all') { + onChange?.('all'); + return; + } + + const actualKeys = new Set(); + + normalizedItems.forEach(item => { + if (keys.has(String(getItemKey(item)))) { + actualKeys.add(getItemKey(item)); + } + }); + + onChange?.(actualKeys); + }, + [normalizedItems, onChange] + ); + + return { + selectedStringKeys, + defaultSelectedStringKeys, + disabledStringKeys, + onStringSelectionChange, + }; +} + +export default useStringifiedMultiSelection; From 9fb7de14f48bd6175404c7bf14d28435017d4be2 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 2 Apr 2024 15:10:31 -0500 Subject: [PATCH 07/41] ListView table support (#1909) --- .../src/spectrum/listView/ListView.tsx | 12 ++++ .../components/src/spectrum/picker/Picker.tsx | 1 - .../src/spectrum/ListView.tsx | 62 +++++++++++++++++++ .../jsapi-components/src/spectrum/index.ts | 1 + 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 packages/jsapi-components/src/spectrum/ListView.tsx diff --git a/packages/components/src/spectrum/listView/ListView.tsx b/packages/components/src/spectrum/listView/ListView.tsx index ff9cb04b7f..8d10b22ecd 100644 --- a/packages/components/src/spectrum/listView/ListView.tsx +++ b/packages/components/src/spectrum/listView/ListView.tsx @@ -3,6 +3,11 @@ import { ListView as SpectrumListView, SpectrumListViewProps, } from '@adobe/react-spectrum'; +import { EMPTY_FUNCTION } from '@deephaven/utils'; +import { + extractSpectrumHTMLElement, + useOnScrollRef, +} from '@deephaven/react-hooks'; import cl from 'classnames'; import { ItemElementOrPrimitive, @@ -33,6 +38,9 @@ export type ListViewProps = { */ onChange?: (keys: 'all' | Set) => 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 @@ -56,6 +64,7 @@ export function ListView({ disabledKeys, UNSAFE_className, onChange, + onScroll = EMPTY_FUNCTION, onSelectionChange, ...spectrumListViewProps }: ListViewProps): JSX.Element { @@ -84,10 +93,13 @@ export function ListView({ onChange: onChange ?? onSelectionChange, }); + const scrollRef = useOnScrollRef(onScroll, extractSpectrumHTMLElement); + return ( } onOpenChange={onOpenChangeInternal} UNSAFE_className={cl('dh-picker', UNSAFE_className)} diff --git a/packages/jsapi-components/src/spectrum/ListView.tsx b/packages/jsapi-components/src/spectrum/ListView.tsx new file mode 100644 index 0000000000..49d8a166bd --- /dev/null +++ b/packages/jsapi-components/src/spectrum/ListView.tsx @@ -0,0 +1,62 @@ +import { + ListView as ListViewBase, + ListViewProps as ListViewPropsBase, + NormalizedItemData, +} from '@deephaven/components'; +import { dh as DhType } from '@deephaven/jsapi-types'; +import { Settings } from '@deephaven/jsapi-utils'; +import { LIST_VIEW_ROW_HEIGHT } from '@deephaven/utils'; +import useFormatter from '../useFormatter'; +import useViewportData from '../useViewportData'; +import { useItemRowDeserializer } from './utils'; + +export interface ListViewProps extends Omit { + table: DhType.Table; + /* The column of values to use as item keys. Defaults to the first column. */ + keyColumn?: string; + /* The column of values to display as primary text. Defaults to the `keyColumn` value. */ + labelColumn?: string; + + // TODO #1890 : descriptionColumn, iconColumn + + settings?: Settings; +} + +export function ListView({ + table, + keyColumn: keyColumnName, + labelColumn: labelColumnName, + settings, + ...props +}: ListViewProps): JSX.Element { + const { getFormattedString: formatValue } = useFormatter(settings); + + const deserializeRow = useItemRowDeserializer({ + table, + keyColumnName, + labelColumnName, + formatValue, + }); + + const { viewportData, onScroll } = useViewportData< + NormalizedItemData, + DhType.Table + >({ + reuseItemsOnTableResize: true, + table, + itemHeight: LIST_VIEW_ROW_HEIGHT, + deserializeRow, + }); + + return ( + + {viewportData.items} + + ); +} + +export default ListView; diff --git a/packages/jsapi-components/src/spectrum/index.ts b/packages/jsapi-components/src/spectrum/index.ts index c434d5d810..49fd5a7e25 100644 --- a/packages/jsapi-components/src/spectrum/index.ts +++ b/packages/jsapi-components/src/spectrum/index.ts @@ -1 +1,2 @@ +export * from './ListView'; export * from './Picker'; From 2a04b319a030065bcf90b2d659c93df3f583db15 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 3 Apr 2024 14:55:55 -0500 Subject: [PATCH 08/41] Fixed import (#1909) --- .../src/spectrum/utils/useStringifiedMultiSelection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts index cadb8a4888..af0619f62c 100644 --- a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts +++ b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts @@ -1,5 +1,5 @@ import { Key, useCallback, useMemo } from 'react'; -import { getItemKey, ItemKey, NormalizedItem } from '.'; +import { getItemKey, ItemKey, NormalizedItem } from './itemUtils'; function toStringKeySet( keys?: 'all' | Iterable From f566e720ef46e7b814c0ecc4c6dddcd1e40c86a7 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 3 Apr 2024 16:48:25 -0500 Subject: [PATCH 09/41] Mocking virtualizer (#1909) --- .../src/styleguide/StyleGuide.test.tsx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/code-studio/src/styleguide/StyleGuide.test.tsx b/packages/code-studio/src/styleguide/StyleGuide.test.tsx index f86f510f5e..f238df8847 100644 --- a/packages/code-studio/src/styleguide/StyleGuide.test.tsx +++ b/packages/code-studio/src/styleguide/StyleGuide.test.tsx @@ -14,6 +14,25 @@ describe(' mounts', () => { // Provide a non-null array to ThemeProvider to tell it to initialize const customThemes: ThemeData[] = []; + // React Spectrum `useVirtualizerItem` depends on `scrollWidth` and `scrollHeight`. + // Mocking these to avoid React "Maximum update depth exceeded" errors. + // https://github.com/adobe/react-spectrum/blob/0b2a838b36ad6d86eee13abaf68b7e4d2b4ada6c/packages/%40react-aria/virtualizer/src/useVirtualizerItem.ts#L49C3-L49C60 + + // From preview docs: https://reactspectrum.blob.core.windows.net/reactspectrum/726a5e8f0ed50fc8d98e39c74bd6dfeb3660fbdf/docs/react-spectrum/testing.html#virtualized-components + // The virtualizer will now think it has a visible area of 1000px x 1000px and that the items within it are 40px x 40px + jest + .spyOn(window.HTMLElement.prototype, 'clientWidth', 'get') + .mockImplementation(() => 1000); + jest + .spyOn(window.HTMLElement.prototype, 'clientHeight', 'get') + .mockImplementation(() => 1000); + jest + .spyOn(window.HTMLElement.prototype, 'scrollHeight', 'get') + .mockImplementation(() => 40); + jest + .spyOn(window.HTMLElement.prototype, 'scrollWidth', 'get') + .mockImplementation(() => 40); + expect(() => render( From 9b5a119b63403d21cdd69e4df9a3db67d0fae98b Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Wed, 3 Apr 2024 16:50:03 -0500 Subject: [PATCH 10/41] generate normalized items styleguide util (#1909) --- .../code-studio/src/styleguide/ListViews.tsx | 9 +- .../code-studio/src/styleguide/Pickers.tsx | 9 +- .../__snapshots__/utils.test.ts.snap | 706 ++++++++++++++++++ .../code-studio/src/styleguide/utils.test.ts | 8 + packages/code-studio/src/styleguide/utils.ts | 30 + 5 files changed, 748 insertions(+), 14 deletions(-) create mode 100644 packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap diff --git a/packages/code-studio/src/styleguide/ListViews.tsx b/packages/code-studio/src/styleguide/ListViews.tsx index f521321c5f..a7fe4be584 100644 --- a/packages/code-studio/src/styleguide/ListViews.tsx +++ b/packages/code-studio/src/styleguide/ListViews.tsx @@ -3,15 +3,10 @@ import { Grid, Item, ListView, ItemKey, Text } from '@deephaven/components'; import { vsAccount, vsPerson } from '@deephaven/icons'; import { Icon } from '@adobe/react-spectrum'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { sampleSectionIdAndClasses } from './utils'; +import { generateNormalizedItems, sampleSectionIdAndClasses } from './utils'; // Generate enough items to require scrolling -const itemsSimple = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz' - .split('') - .map((key, i) => ({ - key, - item: { key: (i + 1) * 100, content: `${key}${key}${key}` }, - })); +const itemsSimple = [...generateNormalizedItems(52)]; function AccountIcon({ slot, diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx index bee79b1d0b..89adb62356 100644 --- a/packages/code-studio/src/styleguide/Pickers.tsx +++ b/packages/code-studio/src/styleguide/Pickers.tsx @@ -10,15 +10,10 @@ import { import { vsPerson } from '@deephaven/icons'; import { Icon } from '@adobe/react-spectrum'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { sampleSectionIdAndClasses } from './utils'; +import { generateNormalizedItems, sampleSectionIdAndClasses } from './utils'; // Generate enough items to require scrolling -const items = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz' - .split('') - .map((key, i) => ({ - key, - item: { key: (i + 1) * 100, content: `${key}${key}${key}` }, - })); +const items = [...generateNormalizedItems(52)]; function PersonIcon(): JSX.Element { return ( diff --git a/packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap b/packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap new file mode 100644 index 0000000000..a9964ea190 --- /dev/null +++ b/packages/code-studio/src/styleguide/__snapshots__/utils.test.ts.snap @@ -0,0 +1,706 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`generateNormalizedItems should generate normalized items 1`] = ` +[ + { + "item": { + "content": "AAA", + "key": 100, + }, + "key": "A", + }, + { + "item": { + "content": "BBB", + "key": 200, + }, + "key": "B", + }, + { + "item": { + "content": "CCC", + "key": 300, + }, + "key": "C", + }, + { + "item": { + "content": "DDD", + "key": 400, + }, + "key": "D", + }, + { + "item": { + "content": "EEE", + "key": 500, + }, + "key": "E", + }, + { + "item": { + "content": "FFF", + "key": 600, + }, + "key": "F", + }, + { + "item": { + "content": "GGG", + "key": 700, + }, + "key": "G", + }, + { + "item": { + "content": "HHH", + "key": 800, + }, + "key": "H", + }, + { + "item": { + "content": "III", + "key": 900, + }, + "key": "I", + }, + { + "item": { + "content": "JJJ", + "key": 1000, + }, + "key": "J", + }, + { + "item": { + "content": "KKK", + "key": 1100, + }, + "key": "K", + }, + { + "item": { + "content": "LLL", + "key": 1200, + }, + "key": "L", + }, + { + "item": { + "content": "MMM", + "key": 1300, + }, + "key": "M", + }, + { + "item": { + "content": "NNN", + "key": 1400, + }, + "key": "N", + }, + { + "item": { + "content": "OOO", + "key": 1500, + }, + "key": "O", + }, + { + "item": { + "content": "PPP", + "key": 1600, + }, + "key": "P", + }, + { + "item": { + "content": "QQQ", + "key": 1700, + }, + "key": "Q", + }, + { + "item": { + "content": "RRR", + "key": 1800, + }, + "key": "R", + }, + { + "item": { + "content": "SSS", + "key": 1900, + }, + "key": "S", + }, + { + "item": { + "content": "TTT", + "key": 2000, + }, + "key": "T", + }, + { + "item": { + "content": "UUU", + "key": 2100, + }, + "key": "U", + }, + { + "item": { + "content": "VVV", + "key": 2200, + }, + "key": "V", + }, + { + "item": { + "content": "WWW", + "key": 2300, + }, + "key": "W", + }, + { + "item": { + "content": "XXX", + "key": 2400, + }, + "key": "X", + }, + { + "item": { + "content": "YYY", + "key": 2500, + }, + "key": "Y", + }, + { + "item": { + "content": "ZZZ", + "key": 2600, + }, + "key": "Z", + }, + { + "item": { + "content": "aaa", + "key": 2700, + }, + "key": "a", + }, + { + "item": { + "content": "bbb", + "key": 2800, + }, + "key": "b", + }, + { + "item": { + "content": "ccc", + "key": 2900, + }, + "key": "c", + }, + { + "item": { + "content": "ddd", + "key": 3000, + }, + "key": "d", + }, + { + "item": { + "content": "eee", + "key": 3100, + }, + "key": "e", + }, + { + "item": { + "content": "fff", + "key": 3200, + }, + "key": "f", + }, + { + "item": { + "content": "ggg", + "key": 3300, + }, + "key": "g", + }, + { + "item": { + "content": "hhh", + "key": 3400, + }, + "key": "h", + }, + { + "item": { + "content": "iii", + "key": 3500, + }, + "key": "i", + }, + { + "item": { + "content": "jjj", + "key": 3600, + }, + "key": "j", + }, + { + "item": { + "content": "kkk", + "key": 3700, + }, + "key": "k", + }, + { + "item": { + "content": "lll", + "key": 3800, + }, + "key": "l", + }, + { + "item": { + "content": "mmm", + "key": 3900, + }, + "key": "m", + }, + { + "item": { + "content": "nnn", + "key": 4000, + }, + "key": "n", + }, + { + "item": { + "content": "ooo", + "key": 4100, + }, + "key": "o", + }, + { + "item": { + "content": "ppp", + "key": 4200, + }, + "key": "p", + }, + { + "item": { + "content": "qqq", + "key": 4300, + }, + "key": "q", + }, + { + "item": { + "content": "rrr", + "key": 4400, + }, + "key": "r", + }, + { + "item": { + "content": "sss", + "key": 4500, + }, + "key": "s", + }, + { + "item": { + "content": "ttt", + "key": 4600, + }, + "key": "t", + }, + { + "item": { + "content": "uuu", + "key": 4700, + }, + "key": "u", + }, + { + "item": { + "content": "vvv", + "key": 4800, + }, + "key": "v", + }, + { + "item": { + "content": "www", + "key": 4900, + }, + "key": "w", + }, + { + "item": { + "content": "xxx", + "key": 5000, + }, + "key": "x", + }, + { + "item": { + "content": "yyy", + "key": 5100, + }, + "key": "y", + }, + { + "item": { + "content": "zzz", + "key": 5200, + }, + "key": "z", + }, + { + "item": { + "content": "AAA1", + "key": 5300, + }, + "key": "A1", + }, + { + "item": { + "content": "BBB1", + "key": 5400, + }, + "key": "B1", + }, + { + "item": { + "content": "CCC1", + "key": 5500, + }, + "key": "C1", + }, + { + "item": { + "content": "DDD1", + "key": 5600, + }, + "key": "D1", + }, + { + "item": { + "content": "EEE1", + "key": 5700, + }, + "key": "E1", + }, + { + "item": { + "content": "FFF1", + "key": 5800, + }, + "key": "F1", + }, + { + "item": { + "content": "GGG1", + "key": 5900, + }, + "key": "G1", + }, + { + "item": { + "content": "HHH1", + "key": 6000, + }, + "key": "H1", + }, + { + "item": { + "content": "III1", + "key": 6100, + }, + "key": "I1", + }, + { + "item": { + "content": "JJJ1", + "key": 6200, + }, + "key": "J1", + }, + { + "item": { + "content": "KKK1", + "key": 6300, + }, + "key": "K1", + }, + { + "item": { + "content": "LLL1", + "key": 6400, + }, + "key": "L1", + }, + { + "item": { + "content": "MMM1", + "key": 6500, + }, + "key": "M1", + }, + { + "item": { + "content": "NNN1", + "key": 6600, + }, + "key": "N1", + }, + { + "item": { + "content": "OOO1", + "key": 6700, + }, + "key": "O1", + }, + { + "item": { + "content": "PPP1", + "key": 6800, + }, + "key": "P1", + }, + { + "item": { + "content": "QQQ1", + "key": 6900, + }, + "key": "Q1", + }, + { + "item": { + "content": "RRR1", + "key": 7000, + }, + "key": "R1", + }, + { + "item": { + "content": "SSS1", + "key": 7100, + }, + "key": "S1", + }, + { + "item": { + "content": "TTT1", + "key": 7200, + }, + "key": "T1", + }, + { + "item": { + "content": "UUU1", + "key": 7300, + }, + "key": "U1", + }, + { + "item": { + "content": "VVV1", + "key": 7400, + }, + "key": "V1", + }, + { + "item": { + "content": "WWW1", + "key": 7500, + }, + "key": "W1", + }, + { + "item": { + "content": "XXX1", + "key": 7600, + }, + "key": "X1", + }, + { + "item": { + "content": "YYY1", + "key": 7700, + }, + "key": "Y1", + }, + { + "item": { + "content": "ZZZ1", + "key": 7800, + }, + "key": "Z1", + }, + { + "item": { + "content": "aaa1", + "key": 7900, + }, + "key": "a1", + }, + { + "item": { + "content": "bbb1", + "key": 8000, + }, + "key": "b1", + }, + { + "item": { + "content": "ccc1", + "key": 8100, + }, + "key": "c1", + }, + { + "item": { + "content": "ddd1", + "key": 8200, + }, + "key": "d1", + }, + { + "item": { + "content": "eee1", + "key": 8300, + }, + "key": "e1", + }, + { + "item": { + "content": "fff1", + "key": 8400, + }, + "key": "f1", + }, + { + "item": { + "content": "ggg1", + "key": 8500, + }, + "key": "g1", + }, + { + "item": { + "content": "hhh1", + "key": 8600, + }, + "key": "h1", + }, + { + "item": { + "content": "iii1", + "key": 8700, + }, + "key": "i1", + }, + { + "item": { + "content": "jjj1", + "key": 8800, + }, + "key": "j1", + }, + { + "item": { + "content": "kkk1", + "key": 8900, + }, + "key": "k1", + }, + { + "item": { + "content": "lll1", + "key": 9000, + }, + "key": "l1", + }, + { + "item": { + "content": "mmm1", + "key": 9100, + }, + "key": "m1", + }, + { + "item": { + "content": "nnn1", + "key": 9200, + }, + "key": "n1", + }, + { + "item": { + "content": "ooo1", + "key": 9300, + }, + "key": "o1", + }, + { + "item": { + "content": "ppp1", + "key": 9400, + }, + "key": "p1", + }, + { + "item": { + "content": "qqq1", + "key": 9500, + }, + "key": "q1", + }, + { + "item": { + "content": "rrr1", + "key": 9600, + }, + "key": "r1", + }, + { + "item": { + "content": "sss1", + "key": 9700, + }, + "key": "s1", + }, + { + "item": { + "content": "ttt1", + "key": 9800, + }, + "key": "t1", + }, + { + "item": { + "content": "uuu1", + "key": 9900, + }, + "key": "u1", + }, + { + "item": { + "content": "vvv1", + "key": 10000, + }, + "key": "v1", + }, +] +`; diff --git a/packages/code-studio/src/styleguide/utils.test.ts b/packages/code-studio/src/styleguide/utils.test.ts index fdc3f3ad85..0c3e973c7e 100644 --- a/packages/code-studio/src/styleguide/utils.test.ts +++ b/packages/code-studio/src/styleguide/utils.test.ts @@ -1,8 +1,16 @@ import { + generateNormalizedItems, sampleSectionIdAndClasses, sampleSectionIdAndClassesSpectrum, } from './utils'; +describe('generateNormalizedItems', () => { + it('should generate normalized items', () => { + const actual = [...generateNormalizedItems(100)]; + expect(actual).toMatchSnapshot(); + }); +}); + describe('sampleSectionIdAndClasses', () => { it('should return id and className', () => { const actual = sampleSectionIdAndClasses('some-id', [ diff --git a/packages/code-studio/src/styleguide/utils.ts b/packages/code-studio/src/styleguide/utils.ts index a256f79b4c..7e7f9087e3 100644 --- a/packages/code-studio/src/styleguide/utils.ts +++ b/packages/code-studio/src/styleguide/utils.ts @@ -1,9 +1,39 @@ import cl from 'classnames'; import { useCallback, useState } from 'react'; +import { 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 NormalizedItems. + * @param count The number of items to generate + */ +export function* generateNormalizedItems( + count: number +): Generator { + const letters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'; + const len = letters.length; + + for (let i = 0; i < count; 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}`; + + yield { + key, + item: { + key: (i + 1) * 100, + content: `${letter}${letter}${letter}${suffix}`, + }, + }; + } +} + /** * Pseudo random number generator with seed so we get reproducible output. * This is necessary in order for e2e tests to work. From 157e46f4d22da6835f1cb958c0b3f6dda353c0bb Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 4 Apr 2024 12:43:16 -0500 Subject: [PATCH 11/41] comments (#1909) --- packages/components/src/spectrum/ItemContent.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx index d5c92180d4..6824daddd0 100644 --- a/packages/components/src/spectrum/ItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -10,9 +10,9 @@ import { DOMRefValue } from '@react-types/shared'; import cl from 'classnames'; import { isElementOfType } from '@deephaven/react-hooks'; import { Text } from './Text'; -import stylesCommon from '../SpectrumComponent.module.scss'; import { TooltipOptions } from './utils'; import ItemTooltip from './ItemTooltip'; +import stylesCommon from '../SpectrumComponent.module.scss'; export interface ItemContentProps { children: ReactNode; @@ -20,8 +20,8 @@ export interface ItemContentProps { } /** - * Picker item content. Text content will be wrapped in a Spectrum Text - * component with ellipsis overflow handling. If text content overflow and + * Item content. Text content will be wrapped in a Spectrum Text + * component with ellipsis overflow handling. If text content overflows and * tooltipOptions are provided a tooltip will be displayed when hovering over * the item content. */ @@ -41,7 +41,7 @@ export function ItemContent({ /** * Whenever a `Text` component renders, see if the content is overflowing so - * we can render a tooltip. + * we know whether to render a tooltip showing the full content or not. */ const checkOverflow = useCallback( (ref: DOMRefValue | null) => { From 85f87c3423c2881444e12a6eb4b474d848e305eb Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 4 Apr 2024 12:46:08 -0500 Subject: [PATCH 12/41] Renamed callback ref (#1909) --- packages/components/src/spectrum/ItemContent.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx index 6824daddd0..8aea897c47 100644 --- a/packages/components/src/spectrum/ItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -43,7 +43,7 @@ export function ItemContent({ * Whenever a `Text` component renders, see if the content is overflowing so * we know whether to render a tooltip showing the full content or not. */ - const checkOverflow = useCallback( + const checkTextOverflowRef = useCallback( (ref: DOMRefValue | null) => { const el = ref?.UNSAFE_getDOMNode(); @@ -82,7 +82,7 @@ export function ItemContent({ isElementOfType(el, Text) ? cloneElement(el, { ...el.props, - ref: checkOverflow, + ref: checkTextOverflowRef, UNSAFE_className: cl( el.props.UNSAFE_className, stylesCommon.spectrumEllipsis @@ -95,7 +95,7 @@ export function ItemContent({ if (typeof content === 'string' || typeof content === 'number') { content = ( {content} From 9acfc308179978aa614922332125a7eb1f49dcba Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 4 Apr 2024 13:01:06 -0500 Subject: [PATCH 13/41] useCheckOverflowRef hook (#1909) --- .../components/src/spectrum/ItemContent.tsx | 34 ++++---------- packages/react-hooks/src/index.ts | 1 + .../react-hooks/src/useCheckOverflowRef.ts | 44 +++++++++++++++++++ 3 files changed, 54 insertions(+), 25 deletions(-) create mode 100644 packages/react-hooks/src/useCheckOverflowRef.ts diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx index 8aea897c47..e085b8372a 100644 --- a/packages/components/src/spectrum/ItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -3,12 +3,10 @@ import { cloneElement, isValidElement, ReactNode, - useCallback, useState, } from 'react'; -import { DOMRefValue } from '@react-types/shared'; import cl from 'classnames'; -import { isElementOfType } from '@deephaven/react-hooks'; +import { isElementOfType, useCheckOverflowRef } from '@deephaven/react-hooks'; import { Text } from './Text'; import { TooltipOptions } from './utils'; import ItemTooltip from './ItemTooltip'; @@ -29,35 +27,21 @@ export function ItemContent({ children: content, tooltipOptions, }: ItemContentProps): JSX.Element | null { + const { + isOverflowing: isTextOverflowing, + ref: checkTextOverflowRef, + reset: resetIsOverflowing, + } = useCheckOverflowRef(); + const [previousContent, setPreviousContent] = useState(content); - const [isOverflowing, setIsOverflowing] = useState(false); // Reset `isOverflowing` if content changes. It will get re-calculated as // `Text` components render. if (previousContent !== content) { setPreviousContent(content); - setIsOverflowing(false); + resetIsOverflowing(); } - /** - * Whenever a `Text` component renders, see if the content is overflowing so - * we know whether to render a tooltip showing the full content or not. - */ - const checkTextOverflowRef = useCallback( - (ref: DOMRefValue | null) => { - const el = ref?.UNSAFE_getDOMNode(); - - if (el == null) { - return; - } - - if (el.scrollWidth > el.offsetWidth) { - setIsOverflowing(true); - } - }, - [] - ); - if (isValidElement(content)) { return content; } @@ -105,7 +89,7 @@ export function ItemContent({ /* eslint-enable no-param-reassign */ const tooltip = - tooltipOptions == null || !isOverflowing ? null : ( + tooltipOptions == null || !isTextOverflowing ? null : ( {content} ); diff --git a/packages/react-hooks/src/index.ts b/packages/react-hooks/src/index.ts index 1555908b88..28f1f8bbe4 100644 --- a/packages/react-hooks/src/index.ts +++ b/packages/react-hooks/src/index.ts @@ -3,6 +3,7 @@ export * from './SelectionUtils'; export * from './SpectrumUtils'; export * from './useAsyncInterval'; export * from './useCallbackWithAction'; +export * from './useCheckOverflowRef'; export { default as useContextOrThrow } from './useContextOrThrow'; export * from './useDebouncedCallback'; export * from './useDelay'; diff --git a/packages/react-hooks/src/useCheckOverflowRef.ts b/packages/react-hooks/src/useCheckOverflowRef.ts new file mode 100644 index 0000000000..48f00df370 --- /dev/null +++ b/packages/react-hooks/src/useCheckOverflowRef.ts @@ -0,0 +1,44 @@ +import { useCallback, useState } from 'react'; +import type { DOMRefValue } from '@react-types/shared'; + +export interface CheckOverflowRefResult { + isOverflowing: boolean; + ref: (elRef: DOMRefValue | null) => void; + reset: () => void; +} + +export function useCheckOverflowRef(): CheckOverflowRefResult { + const [isOverflowing, setIsOverflowing] = useState(false); + + /** + * Whenever a Spectrum `DOMRefValue` component renders, see if the content is + * overflowing so we know whether to render a tooltip showing the full content + * or not. + */ + const ref = useCallback( + (elRef: DOMRefValue | null) => { + const el = elRef?.UNSAFE_getDOMNode(); + + if (el == null) { + return; + } + + if (el.scrollWidth > el.offsetWidth) { + setIsOverflowing(true); + } + }, + [] + ); + + const reset = useCallback(() => { + setIsOverflowing(false); + }, []); + + return { + isOverflowing, + ref, + reset, + }; +} + +export default useCheckOverflowRef; From 4b2cd06a2c5a6ff770b3af70e39d852f98c7518c Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 4 Apr 2024 13:04:14 -0500 Subject: [PATCH 14/41] renames (#1909) --- packages/components/src/spectrum/ItemContent.tsx | 8 ++++---- packages/react-hooks/src/useCheckOverflowRef.ts | 14 ++++++++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx index e085b8372a..fa85d9bb5d 100644 --- a/packages/components/src/spectrum/ItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -28,9 +28,9 @@ export function ItemContent({ tooltipOptions, }: ItemContentProps): JSX.Element | null { const { + checkOverflowRef, isOverflowing: isTextOverflowing, - ref: checkTextOverflowRef, - reset: resetIsOverflowing, + resetIsOverflowing, } = useCheckOverflowRef(); const [previousContent, setPreviousContent] = useState(content); @@ -66,7 +66,7 @@ export function ItemContent({ isElementOfType(el, Text) ? cloneElement(el, { ...el.props, - ref: checkTextOverflowRef, + ref: checkOverflowRef, UNSAFE_className: cl( el.props.UNSAFE_className, stylesCommon.spectrumEllipsis @@ -79,7 +79,7 @@ export function ItemContent({ if (typeof content === 'string' || typeof content === 'number') { content = ( {content} diff --git a/packages/react-hooks/src/useCheckOverflowRef.ts b/packages/react-hooks/src/useCheckOverflowRef.ts index 48f00df370..aefabd6478 100644 --- a/packages/react-hooks/src/useCheckOverflowRef.ts +++ b/packages/react-hooks/src/useCheckOverflowRef.ts @@ -2,9 +2,11 @@ import { useCallback, useState } from 'react'; import type { DOMRefValue } from '@react-types/shared'; export interface CheckOverflowRefResult { + checkOverflowRef: ( + elRef: DOMRefValue | null + ) => void; isOverflowing: boolean; - ref: (elRef: DOMRefValue | null) => void; - reset: () => void; + resetIsOverflowing: () => void; } export function useCheckOverflowRef(): CheckOverflowRefResult { @@ -15,7 +17,7 @@ export function useCheckOverflowRef(): CheckOverflowRefResult { * overflowing so we know whether to render a tooltip showing the full content * or not. */ - const ref = useCallback( + const checkOverflowRef = useCallback( (elRef: DOMRefValue | null) => { const el = elRef?.UNSAFE_getDOMNode(); @@ -30,14 +32,14 @@ export function useCheckOverflowRef(): CheckOverflowRefResult { [] ); - const reset = useCallback(() => { + const resetIsOverflowing = useCallback(() => { setIsOverflowing(false); }, []); return { isOverflowing, - ref, - reset, + checkOverflowRef, + resetIsOverflowing, }; } From ebf1598baa4bf02724c462919a74c86600f56b96 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 4 Apr 2024 13:07:39 -0500 Subject: [PATCH 15/41] Changed "ref" naming to "callback" (#1909) --- packages/components/src/spectrum/ItemContent.tsx | 15 ++++++--------- packages/react-hooks/src/index.ts | 2 +- ...useCheckOverflowRef.ts => useCheckOverflow.ts} | 14 ++++++-------- 3 files changed, 13 insertions(+), 18 deletions(-) rename packages/react-hooks/src/{useCheckOverflowRef.ts => useCheckOverflow.ts} (73%) diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx index fa85d9bb5d..729fdcf624 100644 --- a/packages/components/src/spectrum/ItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -6,7 +6,7 @@ import { useState, } from 'react'; import cl from 'classnames'; -import { isElementOfType, useCheckOverflowRef } from '@deephaven/react-hooks'; +import { isElementOfType, useCheckOverflow } from '@deephaven/react-hooks'; import { Text } from './Text'; import { TooltipOptions } from './utils'; import ItemTooltip from './ItemTooltip'; @@ -27,11 +27,8 @@ export function ItemContent({ children: content, tooltipOptions, }: ItemContentProps): JSX.Element | null { - const { - checkOverflowRef, - isOverflowing: isTextOverflowing, - resetIsOverflowing, - } = useCheckOverflowRef(); + const { checkOverflow, isOverflowing, resetIsOverflowing } = + useCheckOverflow(); const [previousContent, setPreviousContent] = useState(content); @@ -66,7 +63,7 @@ export function ItemContent({ isElementOfType(el, Text) ? cloneElement(el, { ...el.props, - ref: checkOverflowRef, + ref: checkOverflow, UNSAFE_className: cl( el.props.UNSAFE_className, stylesCommon.spectrumEllipsis @@ -79,7 +76,7 @@ export function ItemContent({ if (typeof content === 'string' || typeof content === 'number') { content = ( {content} @@ -89,7 +86,7 @@ export function ItemContent({ /* eslint-enable no-param-reassign */ const tooltip = - tooltipOptions == null || !isTextOverflowing ? null : ( + tooltipOptions == null || !isOverflowing ? null : ( {content} ); diff --git a/packages/react-hooks/src/index.ts b/packages/react-hooks/src/index.ts index 28f1f8bbe4..bec624de9e 100644 --- a/packages/react-hooks/src/index.ts +++ b/packages/react-hooks/src/index.ts @@ -3,7 +3,7 @@ export * from './SelectionUtils'; export * from './SpectrumUtils'; export * from './useAsyncInterval'; export * from './useCallbackWithAction'; -export * from './useCheckOverflowRef'; +export * from './useCheckOverflow'; export { default as useContextOrThrow } from './useContextOrThrow'; export * from './useDebouncedCallback'; export * from './useDelay'; diff --git a/packages/react-hooks/src/useCheckOverflowRef.ts b/packages/react-hooks/src/useCheckOverflow.ts similarity index 73% rename from packages/react-hooks/src/useCheckOverflowRef.ts rename to packages/react-hooks/src/useCheckOverflow.ts index aefabd6478..b998f46975 100644 --- a/packages/react-hooks/src/useCheckOverflowRef.ts +++ b/packages/react-hooks/src/useCheckOverflow.ts @@ -1,15 +1,13 @@ import { useCallback, useState } from 'react'; import type { DOMRefValue } from '@react-types/shared'; -export interface CheckOverflowRefResult { - checkOverflowRef: ( - elRef: DOMRefValue | null - ) => void; +export interface CheckOverflowResult { + checkOverflow: (elRef: DOMRefValue | null) => void; isOverflowing: boolean; resetIsOverflowing: () => void; } -export function useCheckOverflowRef(): CheckOverflowRefResult { +export function useCheckOverflow(): CheckOverflowResult { const [isOverflowing, setIsOverflowing] = useState(false); /** @@ -17,7 +15,7 @@ export function useCheckOverflowRef(): CheckOverflowRefResult { * overflowing so we know whether to render a tooltip showing the full content * or not. */ - const checkOverflowRef = useCallback( + const checkOverflow = useCallback( (elRef: DOMRefValue | null) => { const el = elRef?.UNSAFE_getDOMNode(); @@ -38,9 +36,9 @@ export function useCheckOverflowRef(): CheckOverflowRefResult { return { isOverflowing, - checkOverflowRef, + checkOverflow, resetIsOverflowing, }; } -export default useCheckOverflowRef; +export default useCheckOverflow; From 55d0fc2430ee829e0a0bbdb86a4ab1ec5c3482a1 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 4 Apr 2024 13:20:14 -0500 Subject: [PATCH 16/41] cleanup (#1909) --- packages/react-hooks/src/useCheckOverflow.ts | 26 +++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/packages/react-hooks/src/useCheckOverflow.ts b/packages/react-hooks/src/useCheckOverflow.ts index b998f46975..9b33c3eb8e 100644 --- a/packages/react-hooks/src/useCheckOverflow.ts +++ b/packages/react-hooks/src/useCheckOverflow.ts @@ -2,18 +2,37 @@ import { useCallback, useState } from 'react'; import type { DOMRefValue } from '@react-types/shared'; export interface CheckOverflowResult { + /** + * Callback to check if a Spectrum `DOMRefValue` is overflowing. If an + * overflowing value is passed, `isOverflowing` will be set to true. Note that + * calling again with a non-overflowing value will *not* reset the state. + * Instead `resetIsOverflowing` must be called explicitly. This is to allow + * multiple `DOMRefValue`s to be checked and `isOverflowing` to remain `true` + * if at least one of them is overflowing. + */ checkOverflow: (elRef: DOMRefValue | null) => void; + + /** + * Will be set to true whenever `checkOverflow` is called with an overflowing + * `DOMRefValue`. It will remain `true` until `resetIsOverflowing` is called. + * Default state is `false`. + */ isOverflowing: boolean; + + /** Reset `isOverflowing` to false */ resetIsOverflowing: () => void; } +/** + * Provides a callback to check a Spectrum `DOMRefValue` for overflow. If + * overflow is detected, `isOverflowing` will be set to `true` until reset by + * calling `resetIsOverflowing`. + */ export function useCheckOverflow(): CheckOverflowResult { const [isOverflowing, setIsOverflowing] = useState(false); /** - * Whenever a Spectrum `DOMRefValue` component renders, see if the content is - * overflowing so we know whether to render a tooltip showing the full content - * or not. + * Check if a Spectrum `DOMRefValue` is overflowing. */ const checkOverflow = useCallback( (elRef: DOMRefValue | null) => { @@ -30,6 +49,7 @@ export function useCheckOverflow(): CheckOverflowResult { [] ); + /** Reset `isOverflowing` to false */ const resetIsOverflowing = useCallback(() => { setIsOverflowing(false); }, []); From 9f4c1fbf2b404899ad1d88895ab49eea8bfde765 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Thu, 4 Apr 2024 13:59:32 -0500 Subject: [PATCH 17/41] Cleanup (#1909) --- packages/components/src/spectrum/ItemContent.tsx | 2 +- packages/components/src/spectrum/ItemTooltip.tsx | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/components/src/spectrum/ItemContent.tsx b/packages/components/src/spectrum/ItemContent.tsx index 729fdcf624..1526d3ed64 100644 --- a/packages/components/src/spectrum/ItemContent.tsx +++ b/packages/components/src/spectrum/ItemContent.tsx @@ -59,7 +59,7 @@ export function ItemContent({ // Some Label // Some Description // - content = Children.map(content, (el, i) => + content = Children.map(content, el => isElementOfType(el, Text) ? cloneElement(el, { ...el.props, diff --git a/packages/components/src/spectrum/ItemTooltip.tsx b/packages/components/src/spectrum/ItemTooltip.tsx index d2f031457d..a86588d2ff 100644 --- a/packages/components/src/spectrum/ItemTooltip.tsx +++ b/packages/components/src/spectrum/ItemTooltip.tsx @@ -10,17 +10,19 @@ export interface ItemTooltipProps { options: TooltipOptions; } +/** + * Tooltip for `` content. + */ export function ItemTooltip({ children, options, }: ItemTooltipProps): JSX.Element { - if (typeof children === 'boolean') { - return {children}; - } - if (Array.isArray(children)) { return ( + {/* Multiple children scenarios include a `` node for the label + and at least 1 of an optional icon or `` node. + In such cases we only show the label and description `` nodes. */} {children.filter(node => isElementOfType(node, Text))} From c964da7ad0669aa9d3b7a4160e3d56488f008bcf Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Mon, 8 Apr 2024 14:54:50 -0500 Subject: [PATCH 18/41] ItemSelection type (#1909) --- packages/components/src/spectrum/listView/ListView.tsx | 5 +++-- packages/components/src/spectrum/utils/itemUtils.ts | 2 ++ .../src/spectrum/utils/useStringifiedMultiSelection.ts | 9 +++++++-- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/components/src/spectrum/listView/ListView.tsx b/packages/components/src/spectrum/listView/ListView.tsx index 8d10b22ecd..d867e2b6b9 100644 --- a/packages/components/src/spectrum/listView/ListView.tsx +++ b/packages/components/src/spectrum/listView/ListView.tsx @@ -12,6 +12,7 @@ import cl from 'classnames'; import { ItemElementOrPrimitive, ItemKey, + ItemSelection, NormalizedItem, normalizeItemList, normalizeTooltipOptions, @@ -36,7 +37,7 @@ export type ListViewProps = { * `onSelectionChange`. We are renaming for better consistency with other * components. */ - onChange?: (keys: 'all' | Set) => void; + onChange?: (keys: ItemSelection) => void; /** Handler that is called when the picker is scrolled. */ onScroll?: (event: Event) => void; @@ -45,7 +46,7 @@ export type ListViewProps = { * Handler that is called when the selection changes. * @deprecated Use `onChange` instead */ - onSelectionChange?: (keys: 'all' | Set) => void; + onSelectionChange?: (keys: ItemSelection) => void; } & Omit< SpectrumListViewProps, | 'children' diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index eb7065b94b..ddbc143693 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -33,6 +33,8 @@ export type ItemOrSection = ItemElementOrPrimitive | SectionElement; */ export type ItemKey = Key | boolean; +export type ItemSelection = 'all' | Set; + /** * Augment the Spectrum selection change handler type to include boolean keys. * Spectrum components already supports this, but the built in types don't diff --git a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts index af0619f62c..7104638f2b 100644 --- a/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts +++ b/packages/components/src/spectrum/utils/useStringifiedMultiSelection.ts @@ -1,5 +1,10 @@ import { Key, useCallback, useMemo } from 'react'; -import { getItemKey, ItemKey, NormalizedItem } from './itemUtils'; +import { + getItemKey, + ItemKey, + ItemSelection, + NormalizedItem, +} from './itemUtils'; function toStringKeySet( keys?: 'all' | Iterable @@ -22,7 +27,7 @@ export interface UseStringifiedMultiSelectionOptions { * `onSelectionChange`. We are renaming for better consistency with other * components. */ - onChange?: (keys: 'all' | Set) => void; + onChange?: (keys: ItemSelection) => void; } export interface UseStringifiedMultiSelectionResult { From a111d5aaaeda458bc2bf673544b087e51c879f79 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 9 Apr 2024 08:55:50 -0500 Subject: [PATCH 19/41] Item heights (#1909) --- .../jsapi-components/src/spectrum/ListView.tsx | 8 ++++++-- packages/utils/src/UIConstants.ts | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/jsapi-components/src/spectrum/ListView.tsx b/packages/jsapi-components/src/spectrum/ListView.tsx index 49d8a166bd..c01ee194f9 100644 --- a/packages/jsapi-components/src/spectrum/ListView.tsx +++ b/packages/jsapi-components/src/spectrum/ListView.tsx @@ -1,3 +1,4 @@ +import { useProvider } from '@adobe/react-spectrum'; import { ListView as ListViewBase, ListViewProps as ListViewPropsBase, @@ -5,7 +6,7 @@ import { } from '@deephaven/components'; import { dh as DhType } from '@deephaven/jsapi-types'; import { Settings } from '@deephaven/jsapi-utils'; -import { LIST_VIEW_ROW_HEIGHT } from '@deephaven/utils'; +import { LIST_VIEW_ROW_HEIGHTS } from '@deephaven/utils'; import useFormatter from '../useFormatter'; import useViewportData from '../useViewportData'; import { useItemRowDeserializer } from './utils'; @@ -29,6 +30,9 @@ export function ListView({ settings, ...props }: ListViewProps): JSX.Element { + const { scale } = useProvider(); + const itemHeight = LIST_VIEW_ROW_HEIGHTS[props.density ?? 'regular'][scale]; + const { getFormattedString: formatValue } = useFormatter(settings); const deserializeRow = useItemRowDeserializer({ @@ -44,7 +48,7 @@ export function ListView({ >({ reuseItemsOnTableResize: true, table, - itemHeight: LIST_VIEW_ROW_HEIGHT, + itemHeight, deserializeRow, }); diff --git a/packages/utils/src/UIConstants.ts b/packages/utils/src/UIConstants.ts index 3be0c189ef..a7cf22fabf 100644 --- a/packages/utils/src/UIConstants.ts +++ b/packages/utils/src/UIConstants.ts @@ -2,7 +2,6 @@ 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 LIST_VIEW_ROW_HEIGHT = 32; export const PICKER_ITEM_HEIGHT = 32; export const PICKER_TOP_OFFSET = 4; export const TABLE_ROW_HEIGHT = 33; @@ -12,3 +11,19 @@ export const VIEWPORT_SIZE = 500; export const VIEWPORT_PADDING = 250; export const SPELLCHECK_FALSE_ATTRIBUTE = { spellCheck: false } as const; + +// Copied from https://github.com/adobe/react-spectrum/blob/b2d25ef23b827ec2427bf47b343e6dbd66326ed3/packages/%40react-spectrum/list/src/ListView.tsx#L78 +export const LIST_VIEW_ROW_HEIGHTS = { + compact: { + medium: 32, + large: 40, + }, + regular: { + medium: 40, + large: 50, + }, + spacious: { + medium: 48, + large: 60, + }, +} as const; From d488c926cc9b17b655377569d04bc0014a7c8db3 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 9 Apr 2024 08:56:22 -0500 Subject: [PATCH 20/41] Only render ListView when non-zero height (#1909) --- package-lock.json | 2 + .../src/spectrum/listView/ListView.tsx | 49 ++++++++++++++----- packages/jsapi-components/package.json | 1 + packages/react-hooks/src/index.ts | 1 + packages/react-hooks/src/useContentRect.ts | 44 +++++++++++++++++ 5 files changed, 85 insertions(+), 12 deletions(-) create mode 100644 packages/react-hooks/src/useContentRect.ts diff --git a/package-lock.json b/package-lock.json index a9d06f5266..acfb66a308 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29360,6 +29360,7 @@ "version": "0.72.0", "license": "Apache-2.0", "dependencies": { + "@adobe/react-spectrum": "^3.34.1", "@deephaven/components": "file:../components", "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", "@deephaven/jsapi-types": "1.0.0-dev0.33.1", @@ -31564,6 +31565,7 @@ "@deephaven/jsapi-components": { "version": "file:packages/jsapi-components", "requires": { + "@adobe/react-spectrum": "^3.34.1", "@deephaven/components": "file:../components", "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", "@deephaven/jsapi-shim": "file:../jsapi-shim", diff --git a/packages/components/src/spectrum/listView/ListView.tsx b/packages/components/src/spectrum/listView/ListView.tsx index d867e2b6b9..723394f219 100644 --- a/packages/components/src/spectrum/listView/ListView.tsx +++ b/packages/components/src/spectrum/listView/ListView.tsx @@ -1,11 +1,13 @@ import { useMemo } from 'react'; import { + Flex, ListView as SpectrumListView, SpectrumListViewProps, } from '@adobe/react-spectrum'; import { EMPTY_FUNCTION } from '@deephaven/utils'; import { extractSpectrumHTMLElement, + useContentRect, useOnScrollRef, } from '@deephaven/react-hooks'; import cl from 'classnames'; @@ -68,7 +70,7 @@ export function ListView({ onScroll = EMPTY_FUNCTION, onSelectionChange, ...spectrumListViewProps -}: ListViewProps): JSX.Element { +}: ListViewProps): JSX.Element | null { const normalizedItems = useMemo( () => normalizeItemList(children), [children] @@ -96,20 +98,43 @@ export function ListView({ const scrollRef = useOnScrollRef(onScroll, extractSpectrumHTMLElement); + // Spectrum ListView crashes when it has zero height. Trac the contentRect + // of the parent container and only render the ListView when it has a height. + const { ref: contentRectRef, contentRect } = useContentRect( + extractSpectrumHTMLElement + ); + return ( - - {renderNormalizedItem} - + {contentRect.height === 0 ? ( + // Ensure content has a non-zero height so that the container has a height + // whenever it is visible. This helps differentiate when the container + // height has been set to zero (e.g. when a tab is not visible) vs when + // the container height has not been constrained but has not yet rendered + // the ListView. + <>  + ) : ( + + {renderNormalizedItem} + + )} + ); } diff --git a/packages/jsapi-components/package.json b/packages/jsapi-components/package.json index f849061bd6..ab301c3b9a 100644 --- a/packages/jsapi-components/package.json +++ b/packages/jsapi-components/package.json @@ -22,6 +22,7 @@ "build:sass": "sass --embed-sources --load-path=../../node_modules ./src:./dist" }, "dependencies": { + "@adobe/react-spectrum": "^3.34.1", "@deephaven/components": "file:../components", "@deephaven/jsapi-bootstrap": "file:../jsapi-bootstrap", "@deephaven/jsapi-types": "1.0.0-dev0.33.1", diff --git a/packages/react-hooks/src/index.ts b/packages/react-hooks/src/index.ts index bec624de9e..a3d98b2498 100644 --- a/packages/react-hooks/src/index.ts +++ b/packages/react-hooks/src/index.ts @@ -4,6 +4,7 @@ export * from './SpectrumUtils'; export * from './useAsyncInterval'; export * from './useCallbackWithAction'; export * from './useCheckOverflow'; +export * from './useContentRect'; export { default as useContextOrThrow } from './useContextOrThrow'; export * from './useDebouncedCallback'; export * from './useDelay'; diff --git a/packages/react-hooks/src/useContentRect.ts b/packages/react-hooks/src/useContentRect.ts new file mode 100644 index 0000000000..2018e2a3ab --- /dev/null +++ b/packages/react-hooks/src/useContentRect.ts @@ -0,0 +1,44 @@ +import { identityExtractHTMLElement } from '@deephaven/utils'; +import { useCallback, useRef, useState } from 'react'; +import useMappedRef from './useMappedRef'; +import useResizeObserver from './useResizeObserver'; + +export interface UseContentRectResult { + contentRect: DOMRectReadOnly; + ref: (refValue: T) => void; +} + +/** + * Returns a callback ref that will track the `contentRect` of a given refValue. + * If the `contentRect` is undefined, it will be set to a new `DOMRect` with + * zeros for all dimensions. + * @param map Optional mapping function to extract an HTMLElement from the given + * refValue + * @returns Content rect and a ref callback + */ +export function useContentRect( + map: (ref: T) => HTMLElement | null = identityExtractHTMLElement +): UseContentRectResult { + const [contentRect, setContentRect] = useState( + new DOMRect() + ); + + const handleResize = useCallback( + ([firstEntry]: ResizeObserverEntry[], _observer: ResizeObserver): void => { + setContentRect(firstEntry?.contentRect ?? new DOMRect()); + }, + [] + ); + + const observerRef = useRef(null); + useResizeObserver(observerRef.current, handleResize); + + const ref = useMappedRef(observerRef, map); + + return { + ref, + contentRect, + }; +} + +export default useContentRect; From dac492f35b07c09f879f6775eb815c2bd9868adf Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 9 Apr 2024 08:59:14 -0500 Subject: [PATCH 21/41] Removed minHeight (#1909) --- packages/components/src/spectrum/listView/ListView.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/components/src/spectrum/listView/ListView.tsx b/packages/components/src/spectrum/listView/ListView.tsx index 723394f219..d15442f3f7 100644 --- a/packages/components/src/spectrum/listView/ListView.tsx +++ b/packages/components/src/spectrum/listView/ListView.tsx @@ -123,7 +123,6 @@ export function ListView({ Date: Tue, 9 Apr 2024 08:59:56 -0500 Subject: [PATCH 22/41] Removed unused arg (#1909) --- packages/react-hooks/src/useContentRect.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-hooks/src/useContentRect.ts b/packages/react-hooks/src/useContentRect.ts index 2018e2a3ab..be26a1c6f6 100644 --- a/packages/react-hooks/src/useContentRect.ts +++ b/packages/react-hooks/src/useContentRect.ts @@ -24,7 +24,7 @@ export function useContentRect( ); const handleResize = useCallback( - ([firstEntry]: ResizeObserverEntry[], _observer: ResizeObserver): void => { + ([firstEntry]: ResizeObserverEntry[]): void => { setContentRect(firstEntry?.contentRect ?? new DOMRect()); }, [] From c7ecd14d72da2eb72e22504b0c59274a3f2c5960 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 9 Apr 2024 10:19:09 -0500 Subject: [PATCH 23/41] useContentRect tests (#1909) --- jest.setup.ts | 15 ++++ .../react-hooks/src/useContentRect.test.ts | 70 +++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 packages/react-hooks/src/useContentRect.test.ts diff --git a/jest.setup.ts b/jest.setup.ts index 4bb196107b..8db731f817 100644 --- a/jest.setup.ts +++ b/jest.setup.ts @@ -50,6 +50,21 @@ Object.defineProperty(window, 'ResizeObserver', { }, }); +Object.defineProperty(window, 'DOMRect', { + value: function (x: number = 0, y: number = 0, width = 0, height = 0) { + return TestUtils.createMockProxy({ + x, + y, + width, + height, + top: y, + bottom: y + height, + left: x, + right: x + width, + }); + }, +}); + Object.defineProperty(window, 'TextDecoder', { value: TextDecoder, }); diff --git a/packages/react-hooks/src/useContentRect.test.ts b/packages/react-hooks/src/useContentRect.test.ts new file mode 100644 index 0000000000..7dbf903b8b --- /dev/null +++ b/packages/react-hooks/src/useContentRect.test.ts @@ -0,0 +1,70 @@ +import { act, renderHook } from '@testing-library/react-hooks'; +import { TestUtils } from '@deephaven/utils'; +import { useContentRect } from './useContentRect'; +import useResizeObserver from './useResizeObserver'; + +jest.mock('./useResizeObserver'); + +const { asMock, createMockProxy } = TestUtils; + +beforeEach(() => { + jest.clearAllMocks(); + expect.hasAssertions(); + asMock(useResizeObserver).mockName('useResizeObserver'); +}); + +describe.each([true, false])('useContentRect - explicitMap:%s', explicitMap => { + const mock = { + refValue: document.createElement('div'), + mappedValue: document.createElement('span'), + resizeEntry: createMockProxy({ + contentRect: new DOMRect(0, 0, 100, 100), + }), + observer: createMockProxy(), + }; + + const mockMap = explicitMap ? jest.fn(() => mock.mappedValue) : undefined; + + it('should initially return zero size contentRect', () => { + const { result } = renderHook(() => useContentRect(mockMap)); + expect(useResizeObserver).toHaveBeenCalledWith(null, expect.any(Function)); + expect(result.current.contentRect).toEqual(new DOMRect()); + }); + + it('should pass expected value to resize observer based on presence of map function', () => { + const { result, rerender } = renderHook(() => useContentRect(mockMap)); + + result.current.ref(mock.refValue); + rerender(); + + if (mockMap != null) { + expect(mockMap).toHaveBeenCalledWith(mock.refValue); + } + expect(useResizeObserver).toHaveBeenCalledWith( + mockMap == null ? mock.refValue : mock.mappedValue, + expect.any(Function) + ); + expect(result.current.contentRect).toEqual(new DOMRect()); + }); + + it.each([ + [[], new DOMRect()], + [[mock.resizeEntry], mock.resizeEntry.contentRect], + ])( + 'should update contentRect when resize observer triggers: %s', + (entries, expected) => { + const { result, rerender } = renderHook(() => useContentRect(mockMap)); + + result.current.ref(mock.refValue); + rerender(); + + const handleResize = asMock(useResizeObserver).mock.calls.at(-1)?.[1]; + + act(() => { + handleResize?.(entries, mock.observer); + }); + + expect(result.current.contentRect).toEqual(expected); + } + ); +}); From 663a965b3ce4302d29e66868fdeda3240d15c6ed Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 9 Apr 2024 10:36:05 -0500 Subject: [PATCH 24/41] Updated comments (#1909) --- .../src/spectrum/listView/ListView.tsx | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/components/src/spectrum/listView/ListView.tsx b/packages/components/src/spectrum/listView/ListView.tsx index d15442f3f7..60668c2752 100644 --- a/packages/components/src/spectrum/listView/ListView.tsx +++ b/packages/components/src/spectrum/listView/ListView.tsx @@ -98,8 +98,9 @@ export function ListView({ const scrollRef = useOnScrollRef(onScroll, extractSpectrumHTMLElement); - // Spectrum ListView crashes when it has zero height. Trac the contentRect - // of the parent container and only render the ListView when it has a height. + // Spectrum ListView crashes when it has zero height. Track the contentRect + // of the parent container and only render the ListView when it has a non-zero + // height. const { ref: contentRectRef, contentRect } = useContentRect( extractSpectrumHTMLElement ); @@ -113,11 +114,17 @@ export function ListView({ UNSAFE_className={cl('dh-list-view', UNSAFE_className)} > {contentRect.height === 0 ? ( - // Ensure content has a non-zero height so that the container has a height - // whenever it is visible. This helps differentiate when the container - // height has been set to zero (e.g. when a tab is not visible) vs when - // the container height has not been constrained but has not yet rendered - // the ListView. + // Use   to ensure content has a non-zero height. This ensures the + // container will also have a non-zero height unless its height is + // explicitly set to zero. Example use case: + // 1. Tab containing ListView is visible. Height is non-zero. ListView is + // rendered. + // 2. Tab is hidden. Height of container is explicitly constrained to zero. + // ListView is not rendered. + // 3. Tab is shown again. Height constraint is removed. Resize observer + // fires and shows non-zero height due to the   (without this, + // the height would remain zero forever) + // 4. ListView is rendered again. <>  ) : ( Date: Tue, 9 Apr 2024 10:38:55 -0500 Subject: [PATCH 25/41] Comments (#1909) --- packages/components/src/spectrum/listView/ListView.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/components/src/spectrum/listView/ListView.tsx b/packages/components/src/spectrum/listView/ListView.tsx index 60668c2752..183f6e13bb 100644 --- a/packages/components/src/spectrum/listView/ListView.tsx +++ b/packages/components/src/spectrum/listView/ListView.tsx @@ -117,13 +117,13 @@ export function ListView({ // Use   to ensure content has a non-zero height. This ensures the // container will also have a non-zero height unless its height is // explicitly set to zero. Example use case: - // 1. Tab containing ListView is visible. Height is non-zero. ListView is - // rendered. - // 2. Tab is hidden. Height of container is explicitly constrained to zero. - // ListView is not rendered. + // 1. Tab containing ListView is visible. Container height is non-zero. + // ListView is rendered. + // 2. Tab is hidden. Container height is explicitly constrained to zero. + // ListView is not rendered. // 3. Tab is shown again. Height constraint is removed. Resize observer // fires and shows non-zero height due to the   (without this, - // the height would remain zero forever) + // the height would remain zero forever since ListView hasn't rendered yet) // 4. ListView is rendered again. <>  ) : ( From 055f9532d7019f0d690db9dbaa2bb86a4e47e501 Mon Sep 17 00:00:00 2001 From: Brian Ingles Date: Tue, 9 Apr 2024 10:46:01 -0500 Subject: [PATCH 26/41] Comments (#1909) --- .../src/spectrum/utils/useRenderNormalizedItem.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx index 52a70f62dc..2904bbb558 100644 --- a/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx +++ b/packages/components/src/spectrum/utils/useRenderNormalizedItem.tsx @@ -3,6 +3,12 @@ import { ItemContent } from '../ItemContent'; import { Item } from '../shared'; import { getItemKey, NormalizedItem, TooltipOptions } from './itemUtils'; +/** + * Returns a render function that can be used to render a normalized item in + * collection components. + * @param tooltipOptions Tooltip options to use when rendering the item + * @returns Render function for normalized items + */ export function useRenderNormalizedItem( tooltipOptions: TooltipOptions | null ): (normalizedItem: NormalizedItem) => JSX.Element { @@ -20,7 +26,7 @@ export function useRenderNormalizedItem( // `onSelectionChange` handlers` regardless of the actual type of the // key. We can't really get around setting in order to support Windowed // data, so we'll need to do some manual conversion of keys to strings - // in other places of this component. + // in other components that use this hook. key={key as Key} // The `textValue` prop gets used to provide the content of `