diff --git a/packages/code-studio/src/styleguide/Inputs.tsx b/packages/code-studio/src/styleguide/Inputs.tsx index f0d91234f3..2dac837aa0 100644 --- a/packages/code-studio/src/styleguide/Inputs.tsx +++ b/packages/code-studio/src/styleguide/Inputs.tsx @@ -14,6 +14,7 @@ import { UISwitch, Select, Option, + Item, } from '@deephaven/components'; import SampleSection from './SampleSection'; @@ -33,6 +34,10 @@ const EXAMPLES = [ { title: 'Title 12', value: 'Value 12' }, ]; +const items = EXAMPLES.map(({ title, value }) => ( + {title} +)); + const TIMEOUTS = [ { title: '1 minute', value: 1 * 60 * 1000 }, { title: '5 minutes', value: 5 * 60 * 1000 }, @@ -283,20 +288,15 @@ function Inputs(): React.ReactElement {
Input with Select
- + + {items} +

- + + {items} +
diff --git a/packages/code-studio/src/styleguide/Pickers.tsx b/packages/code-studio/src/styleguide/Pickers.tsx index c41f03861f..739ff8d512 100644 --- a/packages/code-studio/src/styleguide/Pickers.tsx +++ b/packages/code-studio/src/styleguide/Pickers.tsx @@ -8,6 +8,7 @@ import { Text, PickerNormalized, Checkbox, + ComboBox, } from '@deephaven/components'; import { vsPerson } from '@deephaven/icons'; import { Icon } from '@adobe/react-spectrum'; @@ -75,7 +76,7 @@ export function Pickers(): JSX.Element { [selectedKey] ); - const onChange = useCallback((key: ItemKey): void => { + const onChange = useCallback((key: ItemKey | null): void => { setSelectedKey(key); }, []); @@ -85,53 +86,66 @@ export function Pickers(): JSX.Element {

Pickers

- - - Aaa - - - - {mixedItemsWithIconsNoDescriptions} - - - - {/* eslint-disable react/jsx-curly-brace-presence */} - {'String 1'} - {'String 2'} - {'String 3'} -
- Item Aaa - Item Bbb - - - Complex Ccc - -
-
- Item Ddd - Item Eee - - - Complex Fff - - - - Label - Description - - - - Label that causes overflow - Description that causes overflow - -
-
{itemElementsA}
-
{itemElementsB}
-
{itemElementsC}
-
{itemElementsD}
-
{itemElementsE}
-
-
+ {[Picker, ComboBox].map(Component => { + const label = (suffix: string) => + `${Component === Picker ? 'Picker' : 'ComboBox'} (${suffix})`; + return ( + + + Aaa + + + {mixedItemsWithIconsNoDescriptions} + + + {/* eslint-disable react/jsx-curly-brace-presence */} + {'String 1'} + {'String 2'} + {'String 3'} +
+ Item Aaa + Item Bbb + + + Complex Ccc + +
+
+ Item Ddd + Item Eee + + + Complex Fff + + + + Label + Description + + + + Label that causes overflow + + Description that causes overflow + + +
+
{itemElementsA}
+
{itemElementsB}
+
{itemElementsC}
+
{itemElementsD}
+
{itemElementsE}
+
+
+ ); + })} Bootstrap - {[false, true].map(isDisabled => ( - -
- - -
- - - One - Two - Three - -
- ))} - {[false, true].map(isDisabled => (
diff --git a/packages/components/src/ComboBox.test.tsx b/packages/components/src/ComboBox.test.tsx deleted file mode 100644 index 861e1e8545..0000000000 --- a/packages/components/src/ComboBox.test.tsx +++ /dev/null @@ -1,11 +0,0 @@ -import React from 'react'; -import { render } from '@testing-library/react'; -import ComboBox from './ComboBox'; - -function makeWrapper(options = []) { - return render(); -} - -it('mounts and unmounts without failing', () => { - makeWrapper(); -}); diff --git a/packages/components/src/ComboBox.tsx b/packages/components/src/ComboBox.tsx deleted file mode 100644 index 0b89fd0973..0000000000 --- a/packages/components/src/ComboBox.tsx +++ /dev/null @@ -1,515 +0,0 @@ -/** - * Combobox that combines a input box with a searchable dropdown menu - * - * props: - * @param {array} options :[{ - * title: 'option title for display', - * value: 'option value' //option value - * }] - * @param {string} inputPlaceholder place holder for the input box - * @param {string} searchPlaceholder place holder for the search box in drop down search box - * @param {boolean} disabled disable both input & drop down - * - * - */ -import React, { Component } from 'react'; -import PropTypes from 'prop-types'; -import memoize from 'memoizee'; -import classNames from 'classnames'; -import { PopperOptions } from 'popper.js'; -import SearchInput from './SearchInput'; -import { Popper } from './popper'; - -import './ComboBox.scss'; - -interface ComboBoxOption { - title: string; - value: string; -} - -interface ComboBoxProps { - options: ComboBoxOption[]; - popperOptions: PopperOptions; - onChange: (value: string) => void; - inputPlaceholder: string; - searchPlaceholder: string; - disabled: boolean; - className: string; - defaultValue: string; - spellCheck: boolean; - onEnter: () => void; - 'data-testid'?: string; -} - -interface ComboBoxState { - value: string; - filter: string; - filteredOptions: ComboBoxOption[]; - keyboardOptionIndex: number; - menuIsOpen: boolean; - inputWidth: number; -} - -enum MENU_NAVIGATION_DIRECTION { - UP = 'UP', - DOWN = 'DOWN', -} - -class ComboBox extends Component { - static MENU_NAVIGATION_DIRECTION = MENU_NAVIGATION_DIRECTION; - - static DROP_DOWN_MENU_HEIGHT = 200; - - static propTypes = { - options: PropTypes.arrayOf( - PropTypes.shape({ - title: PropTypes.string.isRequired, - value: PropTypes.string.isRequired, - }) - ).isRequired, - popperOptions: PropTypes.shape({ - title: PropTypes.string.isRequired, - value: PropTypes.string.isRequired, - }), - onChange: PropTypes.func, - inputPlaceholder: PropTypes.string, - searchPlaceholder: PropTypes.string, - disabled: PropTypes.bool, - className: PropTypes.string, - defaultValue: PropTypes.string, - spellCheck: PropTypes.bool, - onEnter: PropTypes.func, - 'data-testid': PropTypes.string, - }; - - static defaultProps = { - onChange(): void { - // no-op - }, - inputPlaceholder: '', - searchPlaceholder: 'Search', - disabled: false, - className: '', - defaultValue: '', - popperOptions: null, - spellCheck: true, - onEnter(): void { - // no-op - }, - 'data-testid': undefined, - }; - - constructor(props: ComboBoxProps) { - super(props); - this.state = { - value: '', - filter: '', - filteredOptions: props.options, - keyboardOptionIndex: -1, - menuIsOpen: false, - inputWidth: 100, - }; - - this.toggleMenu = this.toggleMenu.bind(this); - this.handleMenuKeyDown = this.handleMenuKeyDown.bind(this); - this.handleMenuBlur = this.handleMenuBlur.bind(this); - this.closeMenu = this.closeMenu.bind(this); - - this.handleInputChange = this.handleInputChange.bind(this); - this.handleInputKeyDown = this.handleInputKeyDown.bind(this); - this.handleInputBlur = this.handleInputBlur.bind(this); - this.handleFilterChange = this.handleFilterChange.bind(this); - - this.handleOptionClick = this.handleOptionClick.bind(this); - this.handleOptionFocus = this.handleOptionFocus.bind(this); - - this.handleMenuOpened = this.handleMenuOpened.bind(this); - this.handleMenuExited = this.handleMenuExited.bind(this); - - this.popper = React.createRef(); - this.cbContainer = React.createRef(); - this.toggleButton = React.createRef(); - this.menuContainer = React.createRef(); - this.input = React.createRef(); - this.searchInput = React.createRef(); - } - - componentDidUpdate(): void { - const { menuIsOpen, keyboardOptionIndex } = this.state; - if (menuIsOpen && keyboardOptionIndex >= 0) { - this.scrollOptionIntoView(); - } - } - - popper: React.RefObject; - - cbContainer: React.RefObject; - - toggleButton: React.RefObject; - - menuContainer: React.RefObject; - - input: React.RefObject; - - searchInput: React.RefObject; - - setInputWidth(): void { - if (this.cbContainer.current) { - this.setState({ - inputWidth: this.cbContainer.current.getBoundingClientRect().width, - }); - } - } - - getCachedFilteredOptions = memoize( - (options: ComboBoxOption[], input: string) => - options.filter( - option => - option.title.toLowerCase().indexOf(input.toLowerCase()) >= 0 || - option.value.toLowerCase().indexOf(input.toLowerCase()) >= 0 - ) - ); - - focus(): void { - this.input.current?.focus(); - } - - resetValue(): void { - this.setState({ value: '' }); - } - - updateInputValue(value: string): void { - const { onChange } = this.props; - this.setState({ value }); - onChange(value); - } - - handleResize(): void { - this.setInputWidth(); - } - - handleMenuKeyDown(event: React.KeyboardEvent): void { - const { filter, filteredOptions, keyboardOptionIndex } = this.state; - const { options } = this.props; - const menuOptions = filter ? filteredOptions : options; - - switch (event.key) { - case 'Enter': - if (menuOptions[keyboardOptionIndex]?.value != null) { - this.updateInputValue(menuOptions[keyboardOptionIndex].value); - } - this.closeMenu(); - this.input.current?.focus(); - event.stopPropagation(); - event.preventDefault(); - break; - case 'ArrowUp': - this.handleMenuNavigation(ComboBox.MENU_NAVIGATION_DIRECTION.UP); - event.stopPropagation(); - event.preventDefault(); - break; - case 'ArrowDown': - this.handleMenuNavigation(ComboBox.MENU_NAVIGATION_DIRECTION.DOWN); - event.stopPropagation(); - event.preventDefault(); - break; - case 'Escape': - if (filter !== '') { - this.setState({ filter: '' }); - event.stopPropagation(); // Don't trigger blur on input element - } else { - this.closeMenu(); - } - break; - case 'Tab': - if (!event.shiftKey && keyboardOptionIndex === menuOptions.length - 1) { - this.closeMenu(); - } - break; - default: - break; - } - } - - handleMenuNavigation(direction: MENU_NAVIGATION_DIRECTION): void { - const { filter, filteredOptions, keyboardOptionIndex } = this.state; - const { options } = this.props; - const menuOptions = filter ? filteredOptions : options; - const menuOptionsLength = menuOptions.length; - let delta = 0; - switch (direction) { - case ComboBox.MENU_NAVIGATION_DIRECTION.UP: - delta = -1; - break; - case ComboBox.MENU_NAVIGATION_DIRECTION.DOWN: - delta = 1; - break; - } - - if (delta !== 0) { - this.setState({ - keyboardOptionIndex: - (keyboardOptionIndex + delta + menuOptionsLength) % menuOptionsLength, - }); - } - } - - handleInputKeyDown(event: React.KeyboardEvent): void { - const { onEnter } = this.props; - const { menuIsOpen } = this.state; - if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { - if (!menuIsOpen) { - this.openMenu(); - } - } else if (event.key === 'Enter') { - onEnter(); - } - } - - handleInputChange(event: React.ChangeEvent): void { - this.updateInputValue(event.target.value); - } - - handleOptionClick(event: React.MouseEvent): void { - const optionIndex = Number(event.currentTarget.value); - const { filter, filteredOptions } = this.state; - const { options } = this.props; - const menuOptions = filter ? filteredOptions : options; - - this.updateInputValue(menuOptions[optionIndex].value); - this.closeMenu(); - this.input.current?.focus(); - } - - handleOptionFocus(event: React.FocusEvent): void { - this.setState({ keyboardOptionIndex: Number(event.target.value) }); - } - - handleFilterChange(event: React.ChangeEvent): void { - const { options } = this.props; - const filter = event.target.value; - const filteredOptions = this.getCachedFilteredOptions(options, filter); - - this.setState({ - filter, - filteredOptions, - keyboardOptionIndex: 0, - }); - this.popper.current?.scheduleUpdate(); - } - - handleMenuBlur(event: React.FocusEvent): void { - // close if menu blurs, unless its an internal option or the toggleButton which triggers close via click - if ( - (event.relatedTarget instanceof Element && - this.popper.current != null && - this.popper.current.element.contains(event.relatedTarget)) || - event.relatedTarget === this.toggleButton.current - ) { - return; - } - this.closeMenu(false); - } - - handleInputBlur(event: React.FocusEvent): void { - // if blur event is caused by focusing on search input or open menu by keyboard, don't close the menu - const { menuIsOpen } = this.state; - if ( - menuIsOpen && - event.relatedTarget instanceof Element && - this.popper.current != null && - this.popper.current.element.contains(event.relatedTarget) - ) { - return; - } - this.closeMenu(false); - } - - handleMenuOpened(): void { - this.scrollOptionIntoView(); - this.searchInput.current?.focus(); - } - - handleMenuExited(): void { - const { menuIsOpen } = this.state; - if (menuIsOpen) { - this.setState({ menuIsOpen: false }); - this.popper.current?.hide(); - } - this.setState({ filter: '' }); - } - - toggleMenu(event: React.MouseEvent): void { - const { menuIsOpen } = this.state; - if (menuIsOpen) { - this.closeMenu(); - } else { - this.openMenu(); - } - event.stopPropagation(); - } - - openMenu(): void { - this.updateKeyboardIndex(); - this.setInputWidth(); - this.setState({ menuIsOpen: true }); - - window.requestAnimationFrame(() => { - this.popper.current?.show(); - }); - } - - closeMenu(focusInput = true): void { - this.setState({ menuIsOpen: false }); - if (focusInput) { - this.input.current?.focus(); - } - this.popper.current?.hide(); - } - - updateKeyboardIndex(): void { - const { value, filter, filteredOptions } = this.state; - const { options } = this.props; - const menuOptions = filter ? filteredOptions : options; - const valueIndex = menuOptions.findIndex(option => option.value === value); - this.setState({ keyboardOptionIndex: valueIndex }); - } - - scrollOptionIntoView(): void { - const activeOption = this.menuContainer.current?.querySelector( - '.cb-option-btn.keyboard-active' - ); - if (activeOption instanceof HTMLElement) { - activeOption.scrollIntoView({ - block: 'nearest', - inline: 'nearest', - }); - } - } - - renderMenuElement(): JSX.Element { - const { searchPlaceholder } = this.props; - const { filter, inputWidth } = this.state; - return ( -
{ - event.stopPropagation(); - }} - style={{ width: inputWidth }} - onBlur={this.handleMenuBlur} - > -
- -
-
-
{this.renderOptions()}
-
-
- ); - } - - renderOptions(): React.ReactNode { - const { options } = this.props; - const { keyboardOptionIndex, filter, filteredOptions } = this.state; - const menuOptions = filter ? filteredOptions : options; - - return menuOptions.map((option, index) => { - const key = `option-${index}-${option.value}`; - return ( - - ); - }); - } - - render(): JSX.Element { - const { - options, - inputPlaceholder, - disabled, - className, - defaultValue, - spellCheck, - 'data-testid': dataTestId, - } = this.props; - const { value } = this.state; - let { popperOptions } = this.props; - popperOptions = { - placement: 'bottom-end', - modifiers: { - preventOverflow: { enabled: false }, - }, - ...popperOptions, - }; - - return ( -
- -
- -
-
- ); - } -} - -export default ComboBox; diff --git a/packages/components/src/index.ts b/packages/components/src/index.ts index aeba2b3097..4101363d10 100644 --- a/packages/components/src/index.ts +++ b/packages/components/src/index.ts @@ -9,7 +9,6 @@ export { default as CardFlip } from './CardFlip'; export * from './context-actions'; export { default as Collapse } from './Collapse'; export { default as Checkbox } from './Checkbox'; -export { default as ComboBox } from './ComboBox'; export { default as CopyButton } from './CopyButton'; export { default as CustomTimeSelect } from './CustomTimeSelect'; export * from './DateTimeInput'; diff --git a/packages/components/src/spectrum/comboBox/ComboBox.tsx b/packages/components/src/spectrum/comboBox/ComboBox.tsx new file mode 100644 index 0000000000..6b4647fb49 --- /dev/null +++ b/packages/components/src/spectrum/comboBox/ComboBox.tsx @@ -0,0 +1,44 @@ +import { + ComboBox as SpectrumComboBox, + SpectrumComboBoxProps, +} from '@adobe/react-spectrum'; +import type { FocusableRef } from '@react-types/shared'; +import cl from 'classnames'; +import type { NormalizedItem } from '../utils'; +import { PickerPropsT, usePickerProps } from '../picker'; + +export type ComboBoxProps = PickerPropsT>; + +export function ComboBox({ + UNSAFE_className, + ...props +}: ComboBoxProps): JSX.Element { + const { + defaultSelectedKey, + disabledKeys, + selectedKey, + scrollRef, + ...comboBoxProps + } = usePickerProps(props); + + return ( + } + UNSAFE_className={cl('dh-combobox', UNSAFE_className)} + // Type assertions are necessary here since Spectrum types don't account + // for number and boolean key values even though they are valid runtime + // values. + defaultSelectedKey={ + defaultSelectedKey as SpectrumComboBoxProps['defaultSelectedKey'] + } + disabledKeys={ + disabledKeys as SpectrumComboBoxProps['disabledKeys'] + } + selectedKey={ + selectedKey as SpectrumComboBoxProps['selectedKey'] + } + /> + ); +} diff --git a/packages/components/src/spectrum/comboBox/index.ts b/packages/components/src/spectrum/comboBox/index.ts new file mode 100644 index 0000000000..d122f03ba6 --- /dev/null +++ b/packages/components/src/spectrum/comboBox/index.ts @@ -0,0 +1 @@ +export * from './ComboBox'; diff --git a/packages/components/src/spectrum/index.ts b/packages/components/src/spectrum/index.ts index 3ba45b3e04..28630b2da9 100644 --- a/packages/components/src/spectrum/index.ts +++ b/packages/components/src/spectrum/index.ts @@ -10,7 +10,6 @@ export * from './icons'; export * from './layout'; export * from './navigation'; export * from './overlays'; -export * from './pickers'; export * from './shared'; export * from './status'; @@ -19,6 +18,7 @@ export * from './status'; */ export * from './ActionMenu'; export * from './ActionGroup'; +export * from './comboBox'; export * from './ListActionGroup'; export * from './ListActionMenu'; export * from './listView'; diff --git a/packages/components/src/spectrum/picker/Picker.tsx b/packages/components/src/spectrum/picker/Picker.tsx index 3e32c95fed..d6a85ce873 100644 --- a/packages/components/src/spectrum/picker/Picker.tsx +++ b/packages/components/src/spectrum/picker/Picker.tsx @@ -1,68 +1,12 @@ -import { useCallback, useMemo, useState } from 'react'; +import { + Picker as SpectrumPicker, + SpectrumPickerProps, +} from '@adobe/react-spectrum'; import type { DOMRef } from '@react-types/shared'; -import { Picker as SpectrumPicker } from '@adobe/react-spectrum'; import cl from 'classnames'; -import { - EMPTY_FUNCTION, - ensureArray, - PICKER_ITEM_HEIGHTS, - PICKER_TOP_OFFSET, -} from '@deephaven/utils'; -import { - NormalizedSpectrumPickerProps, - ItemOrSection, - getPositionOfSelectedItemElement, - ItemKey, - normalizeTooltipOptions, - TooltipOptions, - isItemElementWithDescription, - isSectionElement, -} from '../utils/itemUtils'; -import { wrapItemChildren } from '../utils/itemWrapperUtils'; -import usePickerScrollOnOpen from './usePickerScrollOnOpen'; -import { useSpectrumThemeProvider } from '../../theme'; - -export type PickerProps = { - children: ItemOrSection | ItemOrSection[]; - - /** Can be set to true or a TooltipOptions to enable item tooltips */ - tooltip?: boolean | TooltipOptions; - - /** The currently selected key in the collection (controlled). */ - selectedKey?: ItemKey | null; - - /** The initial selected key in the collection (uncontrolled). */ - defaultSelectedKey?: ItemKey; - - /** - * Handler that is called when the selection change. - * Note that under the hood, this is just an alias for Spectrum's - * `onSelectionChange`. We are renaming for better consistency with other - * components. - */ - onChange?: (key: ItemKey) => void; - - /** Handler that is called when the picker is scrolled. */ - onScroll?: (event: Event) => void; - - /** - * Handler that is called when the selection changes. - * @deprecated Use `onChange` instead - */ - onSelectionChange?: (key: ItemKey) => void; -} /* - * Support remaining SpectrumPickerProps. - * Note that `selectedKey`, `defaultSelectedKey`, and `onSelectionChange` are - * re-defined above to account for boolean types which aren't included in the - * React `Key` type, but are actually supported by the Spectrum Picker component. - */ & Omit< - NormalizedSpectrumPickerProps, - | 'children' - | 'items' - | 'onSelectionChange' - | 'selectedKey' - | 'defaultSelectedKey' ->; +import type { NormalizedItem } from '../utils'; +import type { PickerProps } from './PickerProps'; +import { usePickerProps } from './usePickerProps'; /** * Picker component for selecting items from a list of items. Items can be @@ -72,103 +16,36 @@ export type PickerProps = { * See https://react-spectrum.adobe.com/react-spectrum/Picker.html */ export function Picker({ - children, - tooltip = true, - defaultSelectedKey, - selectedKey, - onChange, - onOpenChange, - onScroll = EMPTY_FUNCTION, - onSelectionChange, - // eslint-disable-next-line camelcase UNSAFE_className, - ...spectrumPickerProps + ...props }: PickerProps): JSX.Element { - const { scale } = useSpectrumThemeProvider(); - const itemHeight = PICKER_ITEM_HEIGHTS[scale]; - - const tooltipOptions = useMemo( - () => normalizeTooltipOptions(tooltip), - [tooltip] - ); - - // `null` is a valid value for `selectedKey` in controlled mode, so we check - // for explicit `undefined` to identify uncontrolled mode. - const isUncontrolled = selectedKey === undefined; - const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = - useState(defaultSelectedKey); - - const wrappedItems = useMemo( - () => ensureArray(wrapItemChildren(children, tooltipOptions)), - [children, tooltipOptions] - ); - - // Item descriptions and Section elements introduce variable item heights. - // This throws off scroll position calculations, so we disable auto scrolling - // if either of these are found. - const disableScrollOnOpen = useMemo( - () => - wrappedItems.some( - item => isSectionElement(item) || isItemElementWithDescription(item) - ), - [wrappedItems] - ); - - const getInitialScrollPosition = useCallback( - async () => - disableScrollOnOpen - ? null - : getPositionOfSelectedItemElement({ - items: wrappedItems, - itemHeight, - selectedKey: isUncontrolled ? uncontrolledSelectedKey : selectedKey, - topOffset: PICKER_TOP_OFFSET, - }), - [ - disableScrollOnOpen, - isUncontrolled, - itemHeight, - selectedKey, - uncontrolledSelectedKey, - wrappedItems, - ] - ); - - const onSelectionChangeInternal = useCallback( - (key: ItemKey): void => { - // If our component is uncontrolled, track the selected key internally - // so that we can scroll to the selected item if the user re-opens - if (isUncontrolled) { - setUncontrolledSelectedKey(key); - } - - (onChange ?? onSelectionChange)?.(key); - }, - [isUncontrolled, onChange, onSelectionChange] - ); - - const { ref: scrollRef, onOpenChange: onOpenChangeInternal } = - usePickerScrollOnOpen({ - getInitialScrollPosition, - onScroll, - onOpenChange, - }); + const { + defaultSelectedKey, + disabledKeys, + selectedKey, + scrollRef, + ...pickerProps + } = usePickerProps(props); return ( } UNSAFE_className={cl('dh-picker', UNSAFE_className)} - selectedKey={selectedKey as NormalizedSpectrumPickerProps['selectedKey']} + // Type assertions are necessary here since Spectrum types don't account + // for number and boolean key values even though they are valid runtime + // values. defaultSelectedKey={ - defaultSelectedKey as NormalizedSpectrumPickerProps['defaultSelectedKey'] + defaultSelectedKey as SpectrumPickerProps['defaultSelectedKey'] + } + disabledKeys={ + disabledKeys as SpectrumPickerProps['disabledKeys'] + } + selectedKey={ + selectedKey as SpectrumPickerProps['selectedKey'] } - onSelectionChange={onSelectionChangeInternal} - onOpenChange={onOpenChangeInternal} - > - {wrappedItems} - + /> ); } diff --git a/packages/components/src/spectrum/picker/PickerNormalized.tsx b/packages/components/src/spectrum/picker/PickerNormalized.tsx index c330fc289b..4e4409b922 100644 --- a/packages/components/src/spectrum/picker/PickerNormalized.tsx +++ b/packages/components/src/spectrum/picker/PickerNormalized.tsx @@ -4,27 +4,17 @@ import type { DOMRef } from '@react-types/shared'; import cl from 'classnames'; import { EMPTY_FUNCTION } from '@deephaven/utils'; import { Section } from '../shared'; -import type { PickerProps as PickerBaseProps } from './Picker'; +import type { PickerNormalizedProps } from './PickerProps'; import { getItemKey, isNormalizedSection, - NormalizedItem, - NormalizedSection, normalizeTooltipOptions, useRenderNormalizedItem, useStringifiedSelection, } from '../utils'; import usePickerScrollOnOpen from './usePickerScrollOnOpen'; -export interface PickerNormalizedProps - extends Omit { - normalizedItems: (NormalizedItem | NormalizedSection)[]; - showItemIcons: boolean; - getInitialScrollPosition?: () => Promise; - onScroll?: (event: Event) => void; -} - /** * Picker that takes an array of `NormalizedItem` or `NormalizedSection` items * as children and uses a render item function to render the items. This is diff --git a/packages/components/src/spectrum/picker/PickerProps.ts b/packages/components/src/spectrum/picker/PickerProps.ts new file mode 100644 index 0000000000..981f47390d --- /dev/null +++ b/packages/components/src/spectrum/picker/PickerProps.ts @@ -0,0 +1,98 @@ +import type { SpectrumPickerProps } from '@adobe/react-spectrum'; +import type { + ItemKey, + ItemOrSection, + NormalizedItem, + NormalizedSection, + TooltipOptions, +} from '../utils'; + +/** + * Extend Spectrum Picker props (also other components that adhere to the same + * apis such as ComboBox). + * - `children` is extended to include primitive types and to exclude render function + * - `items` and `defaultItems` are excluded since we are not currently supporting + * render functions as `children` + * - selection key types are extended to include number + boolean primitive types + * - remaining props from the original type are passed through + */ +export type PickerPropsT = Omit< + TProps, + // These props are all re-defined below + | 'children' + | 'onSelectionChange' + | 'selectedKey' + | 'defaultSelectedKey' + | 'disabledKeys' + // Excluding `defaultItems` and `items` since we are not currently supporting + // a render function as `children`. This simplifies the API for determining + // initial scroll position and wrapping items with tooltips. + | 'defaultItems' + | 'items' +> & { + children: ItemOrSection | ItemOrSection[]; + + /** Can be set to true or a TooltipOptions to enable item tooltips */ + tooltip?: boolean | TooltipOptions; + + /** The currently selected key in the collection (controlled). */ + selectedKey?: ItemKey | null; + + /** The initial selected key in the collection (uncontrolled). */ + defaultSelectedKey?: ItemKey; + + /** + * The item keys that are disabled. These items cannot be selected, focused, + * or otherwise interacted with. + */ + 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?: (key: ItemKey | null) => void; + + /** + * Method that is called when the open state of the menu changes. + */ + onOpenChange?: (isOpen: boolean) => void; + + /** Handler that is called when the picker is scrolled. */ + onScroll?: (event: Event) => void; + + /** + * Handler that is called when the selection changes. + * @deprecated Use `onChange` instead + */ + onSelectionChange?: (key: ItemKey | null) => void; +}; + +/** + * Extend Picker props for usage with normalized items list instead of React + * `children` elements. + */ +export type PickerNormalizedPropsT = Omit< + PickerPropsT, + 'children' +> & { + /** + * Normalized format for items and sections instead React elements. + */ + normalizedItems: (NormalizedItem | NormalizedSection)[]; + + /** + * Whether to show icons in items. + */ + showItemIcons: boolean; + + /** + * Get the initial scroll position to use when picker is opened. + */ + getInitialScrollPosition?: () => Promise; +}; + +export type PickerProps = PickerPropsT>; +export type PickerNormalizedProps = PickerNormalizedPropsT; diff --git a/packages/components/src/spectrum/picker/index.ts b/packages/components/src/spectrum/picker/index.ts index 2ce75f0446..f1180f5214 100644 --- a/packages/components/src/spectrum/picker/index.ts +++ b/packages/components/src/spectrum/picker/index.ts @@ -1,3 +1,6 @@ export * from './Picker'; export * from './PickerNormalized'; +export * from './PickerProps'; +export * from './usePickerItemScale'; +export * from './usePickerProps'; export * from './usePickerScrollOnOpen'; diff --git a/packages/components/src/spectrum/picker/usePickerItemScale.test.ts b/packages/components/src/spectrum/picker/usePickerItemScale.test.ts new file mode 100644 index 0000000000..f57cde548d --- /dev/null +++ b/packages/components/src/spectrum/picker/usePickerItemScale.test.ts @@ -0,0 +1,30 @@ +import { PICKER_ITEM_HEIGHTS, TestUtils } from '@deephaven/utils'; +import type { ProviderContext } from '@react-types/provider'; +import { renderHook } from '@testing-library/react-hooks'; +import { useSpectrumThemeProvider } from '../../theme'; +import { usePickerItemScale } from './usePickerItemScale'; + +const { asMock } = TestUtils; + +jest.mock('../../theme'); + +beforeEach(() => { + jest.clearAllMocks(); + asMock(useSpectrumThemeProvider).mockName('useSpectrumThemeProvider'); +}); + +describe('usePickerItemScale', () => { + it.each([ + ['medium', PICKER_ITEM_HEIGHTS.medium], + ['large', PICKER_ITEM_HEIGHTS.large], + ] as const)('should return itemHeight for scale: %s', (scale, itemHeight) => { + asMock(useSpectrumThemeProvider).mockReturnValue({ + scale, + } as ProviderContext); + + const { result } = renderHook(() => usePickerItemScale()); + expect(result.current).toEqual({ + itemHeight, + }); + }); +}); diff --git a/packages/components/src/spectrum/picker/usePickerItemScale.ts b/packages/components/src/spectrum/picker/usePickerItemScale.ts new file mode 100644 index 0000000000..1847c5c104 --- /dev/null +++ b/packages/components/src/spectrum/picker/usePickerItemScale.ts @@ -0,0 +1,15 @@ +import { PICKER_ITEM_HEIGHTS } from '@deephaven/utils'; +import { useSpectrumThemeProvider } from '../../theme'; + +/** + * Get Picker Item height for current scale. + * @returns Picker Item height + */ +export function usePickerItemScale(): { itemHeight: number } { + const { scale } = useSpectrumThemeProvider(); + const itemHeight = PICKER_ITEM_HEIGHTS[scale]; + + return { itemHeight }; +} + +export default usePickerItemScale; diff --git a/packages/components/src/spectrum/picker/usePickerProps.ts b/packages/components/src/spectrum/picker/usePickerProps.ts new file mode 100644 index 0000000000..2d7535d1ca --- /dev/null +++ b/packages/components/src/spectrum/picker/usePickerProps.ts @@ -0,0 +1,106 @@ +import { + EMPTY_FUNCTION, + ensureArray, + PICKER_TOP_OFFSET, +} from '@deephaven/utils'; +import { DOMRef } from '@react-types/shared'; +import { useMemo } from 'react'; +import { + normalizeTooltipOptions, + wrapItemChildren, + useOnChangeTrackUncontrolled, + useStaticItemInitialScrollPosition, + ItemKey, + SectionElement, + ItemElement, +} from '../utils'; +import type { PickerPropsT } from './PickerProps'; +import usePickerItemScale from './usePickerItemScale'; +import usePickerScrollOnOpen from './usePickerScrollOnOpen'; + +/** Props that are derived. */ +export type UsePickerDerivedProps = { + children: (SectionElement | ItemElement)[]; + defaultSelectedKey?: ItemKey | undefined; + selectedKey?: ItemKey | null | undefined; + scrollRef: DOMRef; + onOpenChange: (isOpen: boolean) => void; + onSelectionChange: ((key: ItemKey | null) => void) | undefined; +}; + +/** Props that are passed through untouched. */ +export type UsePickerPassthroughProps = Omit< + PickerPropsT, + | 'children' + | 'defaultSelectedKey' + | 'selectedKey' + | 'tooltip' + | 'onChange' + | 'onOpenChange' + | 'onScroll' + | 'onSelectionChange' +>; + +export type UsePickerProps = UsePickerDerivedProps & + UsePickerPassthroughProps; + +/** + * Derive props for Picker components (e.g. Picker and ComboBox). Specifically + * handles wrapping children items and initial scroll position when the picker + * is opened. + */ +export function usePickerProps({ + children, + defaultSelectedKey, + selectedKey, + tooltip = true, + onChange: onChangeHandler, + onOpenChange: onOpenChangeHandler, + onScroll = EMPTY_FUNCTION, + onSelectionChange: onSelectionChangeHandler, + ...props +}: PickerPropsT): UsePickerProps { + const { itemHeight } = usePickerItemScale(); + + const tooltipOptions = useMemo( + () => normalizeTooltipOptions(tooltip), + [tooltip] + ); + + const items = useMemo( + () => ensureArray(wrapItemChildren(children, tooltipOptions)), + [children, tooltipOptions] + ); + + const { selectedKeyMaybeUncontrolled, onChangeMaybeUncontrolled } = + useOnChangeTrackUncontrolled({ + defaultSelectedKey, + selectedKey, + onChange: onChangeHandler ?? onSelectionChangeHandler, + }); + + const getInitialScrollPosition = useStaticItemInitialScrollPosition({ + itemHeight, + items, + selectedKey: selectedKeyMaybeUncontrolled, + topOffset: PICKER_TOP_OFFSET, + }); + + const { ref: scrollRef, onOpenChange } = usePickerScrollOnOpen({ + getInitialScrollPosition, + onScroll, + onOpenChange: onOpenChangeHandler, + }); + + return { + ...props, + defaultSelectedKey, + selectedKey, + children: items, + scrollRef, + onOpenChange, + onSelectionChange: onChangeMaybeUncontrolled, + }; +} + +export default usePickerProps; diff --git a/packages/components/src/spectrum/pickers.ts b/packages/components/src/spectrum/pickers.ts deleted file mode 100644 index bb285fca18..0000000000 --- a/packages/components/src/spectrum/pickers.ts +++ /dev/null @@ -1,4 +0,0 @@ -export { - ComboBox, - type SpectrumComboBoxProps as ComboBoxProps, -} from '@adobe/react-spectrum'; diff --git a/packages/components/src/spectrum/utils/index.ts b/packages/components/src/spectrum/utils/index.ts index 2b35c6afdf..e2ef9e3ffa 100644 --- a/packages/components/src/spectrum/utils/index.ts +++ b/packages/components/src/spectrum/utils/index.ts @@ -2,6 +2,8 @@ export * from './itemUtils'; export * from './itemWrapperUtils'; export * from './propsUtils'; export * from './themeUtils'; +export * from './useOnChangeTrackUncontrolled'; export * from './useRenderNormalizedItem'; +export * from './useStaticItemInitialScrollPosition'; export * from './useStringifiedMultiSelection'; export * from './useStringifiedSelection'; diff --git a/packages/components/src/spectrum/utils/itemUtils.test.tsx b/packages/components/src/spectrum/utils/itemUtils.test.tsx index 3389b8c905..51c6932a91 100644 --- a/packages/components/src/spectrum/utils/itemUtils.test.tsx +++ b/packages/components/src/spectrum/utils/itemUtils.test.tsx @@ -14,6 +14,8 @@ import { itemSelectionToStringSet, getPositionOfSelectedItemElement, isItemElementWithDescription, + getItemTextValue, + ITEM_EMPTY_STRING_TEXT_VALUE, } from './itemUtils'; import { Item, ItemElementOrPrimitive, Section } from '../shared'; import { Text } from '../Text'; @@ -37,6 +39,39 @@ describe('getItemKey', () => { ); }); +describe('getItemTextValue', () => { + it.each([ + [string, 'string'], + [{4}, '4'], + [{true}, 'true'], + [{false}, 'false'], + [ + + string + , + 'textValue', + ], + [ + + string + , + ITEM_EMPTY_STRING_TEXT_VALUE, + ], + [ + + object + , + undefined, + ], + ])( + 'should return the expected `textValue`: %s, %s', + (item, expectedValue) => { + const actual = getItemTextValue(item); + expect(actual).toBe(expectedValue); + } + ); +}); + describe('getPositionOfSelectedItemElement', () => { const items = [ A, diff --git a/packages/components/src/spectrum/utils/itemUtils.ts b/packages/components/src/spectrum/utils/itemUtils.ts index 515f182bea..49895787c9 100644 --- a/packages/components/src/spectrum/utils/itemUtils.ts +++ b/packages/components/src/spectrum/utils/itemUtils.ts @@ -1,5 +1,4 @@ import { Key, ReactElement, ReactNode } from 'react'; -import { SpectrumPickerProps } from '@adobe/react-spectrum'; import type { ItemRenderer } from '@react-types/shared'; import { isElementOfType } from '@deephaven/react-hooks'; import { ensureArray, KeyedItem, SelectionT } from '@deephaven/utils'; @@ -108,8 +107,6 @@ export type NormalizedSection = KeyedItem; export type NormalizedItemOrSection = TItemOrSection extends SectionElement ? NormalizedSection : NormalizedItem; -export type NormalizedSpectrumPickerProps = SpectrumPickerProps; - export type TooltipOptions = { placement: PopperOptions['placement'] }; /** @@ -132,6 +129,24 @@ export function getItemKey< return (item?.item?.key ?? item?.key) as TKey; } +/** + * Determine Item `textValue` based on the `textValue` prop or primitive children + * value. + * @param item The item to get the text value for + * @returns The text value of the item + */ +export function getItemTextValue(item: ItemElement): string | undefined { + if (item.props.textValue == null) { + return ['string', 'boolean', 'number'].includes(typeof item.props.children) + ? String(item.props.children) + : undefined; + } + + return item.props.textValue === '' + ? ITEM_EMPTY_STRING_TEXT_VALUE + : item.props.textValue; +} + /** * Get the position of the item with the given selected key in a list of items. * @param items The items to search diff --git a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx index 489ba26ff0..0435f06501 100644 --- a/packages/components/src/spectrum/utils/itemWrapperUtils.tsx +++ b/packages/components/src/spectrum/utils/itemWrapperUtils.tsx @@ -5,6 +5,7 @@ import { dh as dhIcons } from '@deephaven/icons'; import { isElementOfType } from '@deephaven/react-hooks'; import { ensureArray, NON_BREAKING_SPACE } from '@deephaven/utils'; import { + getItemTextValue, isItemElement, isSectionElement, ItemElement, @@ -66,10 +67,7 @@ export function wrapItemChildren( } const key = item.key ?? item.props.textValue; - const textValue = - item.props.textValue === '' - ? ITEM_EMPTY_STRING_TEXT_VALUE - : item.props.textValue; + const textValue = getItemTextValue(item); // Wrap in `ItemContent` so we can support tooltips and handle text // overflow diff --git a/packages/components/src/spectrum/utils/useOnChangeTrackUncontrolled.test.ts b/packages/components/src/spectrum/utils/useOnChangeTrackUncontrolled.test.ts new file mode 100644 index 0000000000..5815e201d3 --- /dev/null +++ b/packages/components/src/spectrum/utils/useOnChangeTrackUncontrolled.test.ts @@ -0,0 +1,56 @@ +import { act, renderHook } from '@testing-library/react-hooks'; +import { ItemKey } from './itemUtils'; +import { + useOnChangeTrackUncontrolled, + UseOnChangeTrackUncontrolledOptions, +} from './useOnChangeTrackUncontrolled'; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +afterEach(() => { + expect.hasAssertions(); +}); + +describe('useOnChangeTrackUncontrolled', () => { + const defaultSelectedKey = 'default.selectedKey'; + const changedKey = 'changed.key'; + const controlledKey = 'controlled.key'; + + const props = { + controlled: { selectedKey: controlledKey }, + controlledWithDefault: { + defaultSelectedKey, + selectedKey: controlledKey, + }, + uncontrolled: {}, + uncontrolledWithDefault: { defaultSelectedKey }, + } satisfies Record>; + + it.each([ + [props.uncontrolled, [undefined, changedKey]], + [props.uncontrolledWithDefault, [defaultSelectedKey, changedKey]], + [props.controlled, [controlledKey, controlledKey]], + [props.controlledWithDefault, [controlledKey, controlledKey]], + ] as const)( + 'should track controlled / uncontrolled state: %s, %s', + (initialProps, [expectedInitial, expectedChanged]) => { + const { result } = renderHook(() => + useOnChangeTrackUncontrolled(initialProps) + ); + + expect(result.current.selectedKeyMaybeUncontrolled).toEqual( + expectedInitial + ); + + act(() => { + result.current.onChangeMaybeUncontrolled(changedKey); + }); + + expect(result.current.selectedKeyMaybeUncontrolled).toEqual( + expectedChanged + ); + } + ); +}); diff --git a/packages/components/src/spectrum/utils/useOnChangeTrackUncontrolled.ts b/packages/components/src/spectrum/utils/useOnChangeTrackUncontrolled.ts new file mode 100644 index 0000000000..9cf62ec6bd --- /dev/null +++ b/packages/components/src/spectrum/utils/useOnChangeTrackUncontrolled.ts @@ -0,0 +1,53 @@ +import { useCallback, useState } from 'react'; +import { ItemKey } from './itemUtils'; + +export interface UseOnChangeTrackUncontrolledOptions { + defaultSelectedKey?: ItemKey; + selectedKey?: ItemKey | null; + onChange?: (key: ItemKey | null) => void; +} + +export interface UseOnChangeTrackUncontrolledResult { + selectedKeyMaybeUncontrolled?: ItemKey | null; + onChangeMaybeUncontrolled: (key: ItemKey | null) => void; +} + +/** + * Returns a selectedKey and onChange handler that can manage selection state + * for both controlled and uncontrolled components. Useful for cases where a + * component needs to always track its selection state regardless of its + * controlled / uncontrolled status. + */ +export function useOnChangeTrackUncontrolled({ + defaultSelectedKey, + selectedKey, + onChange: onChangeHandler, +}: UseOnChangeTrackUncontrolledOptions): UseOnChangeTrackUncontrolledResult { + // `null` is a valid value for `selectedKey` in controlled mode, so we check + // for explicit `undefined` to identify uncontrolled mode. + const isUncontrolled = selectedKey === undefined; + const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = useState< + ItemKey | null | undefined + >(defaultSelectedKey); + + const onChangeMaybeUncontrolled = useCallback( + (key: ItemKey | null): void => { + // If our component is uncontrolled, track the selected key internally + if (isUncontrolled) { + setUncontrolledSelectedKey(key); + } + + onChangeHandler?.(key); + }, + [isUncontrolled, onChangeHandler] + ); + + return { + selectedKeyMaybeUncontrolled: isUncontrolled + ? uncontrolledSelectedKey + : selectedKey, + onChangeMaybeUncontrolled, + }; +} + +export default useOnChangeTrackUncontrolled; diff --git a/packages/components/src/spectrum/utils/useStaticItemInitialScrollPosition.test.tsx b/packages/components/src/spectrum/utils/useStaticItemInitialScrollPosition.test.tsx new file mode 100644 index 0000000000..dab7444f3d --- /dev/null +++ b/packages/components/src/spectrum/utils/useStaticItemInitialScrollPosition.test.tsx @@ -0,0 +1,57 @@ +import React from 'react'; +import { renderHook } from '@testing-library/react-hooks'; +import { Item, Section } from '../shared'; +import { ItemElement } from './itemUtils'; +import { useStaticItemInitialScrollPosition } from './useStaticItemInitialScrollPosition'; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +afterEach(() => { + expect.hasAssertions(); +}); + +const mockSelectedKey = 'selected.key'; + +describe('useStaticItemInitialScrollPosition: selectedKey: %s', () => { + const itemHeight = 32; + const topOffset = 20; + + const items = { + empty: [], + mixed: [Item,
Test
], + only: [ + Item, + Item, + Item, + Item, + Item, + ], + } satisfies Record[]>; + + it.each([ + [items.empty, undefined, topOffset], + [items.mixed, undefined, topOffset], + [items.only, undefined, topOffset], + [items.empty, mockSelectedKey, topOffset], + [items.mixed, mockSelectedKey, topOffset], + [items.only, mockSelectedKey, topOffset + 3 * itemHeight], + ])( + 'should return a function that returns the initial scroll position for item only collections: %s, %s', + async (givenItems, selectedKey, expected) => { + const { result } = renderHook(() => + useStaticItemInitialScrollPosition({ + itemHeight, + selectedKey, + topOffset, + items: givenItems, + }) + ); + + const actual = await result.current(); + + expect(actual).toEqual(expected); + } + ); +}); diff --git a/packages/components/src/spectrum/utils/useStaticItemInitialScrollPosition.ts b/packages/components/src/spectrum/utils/useStaticItemInitialScrollPosition.ts new file mode 100644 index 0000000000..8f0f390de5 --- /dev/null +++ b/packages/components/src/spectrum/utils/useStaticItemInitialScrollPosition.ts @@ -0,0 +1,51 @@ +import { useCallback, useMemo } from 'react'; +import { + getPositionOfSelectedItemElement, + isItemElementWithDescription, + isSectionElement, + ItemElement, + ItemKey, + SectionElement, +} from './itemUtils'; + +export interface UseStaticItemInitialScrollPositionOptions { + itemHeight: number; + items: (ItemElement | SectionElement)[]; + selectedKey?: ItemKey | null; + topOffset: number; +} + +export function useStaticItemInitialScrollPosition({ + itemHeight, + selectedKey, + topOffset, + items, +}: UseStaticItemInitialScrollPositionOptions): () => Promise { + // Item descriptions and Section elements introduce variable item heights. + // This throws off scroll position calculations, so we disable auto scrolling + // if either of these are found. + const disableScrollOnOpen = useMemo( + () => + items.some( + item => isSectionElement(item) || isItemElementWithDescription(item) + ), + [items] + ); + + const getInitialScrollPosition = useCallback( + async () => + disableScrollOnOpen + ? topOffset + : getPositionOfSelectedItemElement({ + items, + itemHeight, + selectedKey, + topOffset, + }), + [disableScrollOnOpen, itemHeight, items, selectedKey, topOffset] + ); + + return getInitialScrollPosition; +} + +export default useStaticItemInitialScrollPosition; diff --git a/packages/iris-grid/src/sidebar/conditional-formatting/ColumnFormatEditor.tsx b/packages/iris-grid/src/sidebar/conditional-formatting/ColumnFormatEditor.tsx index 2467046ea6..1417ff4622 100644 --- a/packages/iris-grid/src/sidebar/conditional-formatting/ColumnFormatEditor.tsx +++ b/packages/iris-grid/src/sidebar/conditional-formatting/ColumnFormatEditor.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import Log from '@deephaven/log'; import { ComboBox } from '@deephaven/components'; import type { dh as DhType } from '@deephaven/jsapi-types'; @@ -115,23 +115,19 @@ function ColumnFormatEditor(props: ColumnFormatEditorProps): JSX.Element { [onChange, selectedColumn, selectedStyle, conditionConfig, conditionValid] ); - const columnInputOptions = columns.map(({ name }) => ({ - title: name, - value: name, - })); + const columnNames = useMemo(() => columns.map(({ name }) => name), [columns]); return (
+ > + {columnNames} +
{selectedColumn !== undefined && ( diff --git a/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormatEditor.scss b/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormatEditor.scss index 6481f965e6..0781dade6c 100644 --- a/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormatEditor.scss +++ b/packages/iris-grid/src/sidebar/conditional-formatting/ConditionalFormatEditor.scss @@ -22,4 +22,8 @@ .color-select { padding: 0 2em 0 0.25em; } + + .dh-combobox { + width: 100%; + } } diff --git a/packages/iris-grid/src/sidebar/conditional-formatting/RowFormatEditor.tsx b/packages/iris-grid/src/sidebar/conditional-formatting/RowFormatEditor.tsx index 0ecd9f3113..ad2e438444 100644 --- a/packages/iris-grid/src/sidebar/conditional-formatting/RowFormatEditor.tsx +++ b/packages/iris-grid/src/sidebar/conditional-formatting/RowFormatEditor.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import Log from '@deephaven/log'; import type { dh as DhType } from '@deephaven/jsapi-types'; import { ComboBox } from '@deephaven/components'; @@ -108,23 +108,19 @@ function RowFormatEditor(props: RowFormatEditorProps): JSX.Element { [onChange, selectedColumn, selectedStyle, conditionConfig, conditionValid] ); - const columnInputOptions = columns.map(({ name }) => ({ - title: name, - value: name, - })); + const columnNames = useMemo(() => columns.map(({ name }) => name), [columns]); return (
+ > + {columnNames} +
{selectedColumn !== undefined && ( diff --git a/packages/jsapi-components/src/spectrum/Picker.tsx b/packages/jsapi-components/src/spectrum/Picker.tsx index 4d5b39016f..2e9872d042 100644 --- a/packages/jsapi-components/src/spectrum/Picker.tsx +++ b/packages/jsapi-components/src/spectrum/Picker.tsx @@ -52,9 +52,9 @@ export function Picker({ // `null` is a valid value for `selectedKey` in controlled mode, so we check // for explicit `undefined` to identify uncontrolled mode. const isUncontrolled = props.selectedKey === undefined; - const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = useState( - props.defaultSelectedKey - ); + const [uncontrolledSelectedKey, setUncontrolledSelectedKey] = useState< + ItemKey | null | undefined + >(props.defaultSelectedKey); const keyColumn = useMemo( () => getItemKeyColumn(table, keyColumnName), @@ -126,7 +126,7 @@ export function Picker({ ); const onSelectionChangeInternal = useCallback( - (key: ItemKey): void => { + (key: ItemKey | null): void => { // If our component is uncontrolled, track the selected key internally // so that we can scroll to the selected item if the user re-opens if (isUncontrolled) { diff --git a/packages/jsapi-components/src/usePickerWithSelectedValues.test.ts b/packages/jsapi-components/src/usePickerWithSelectedValues.test.ts index 6c2a80bbc0..97166b4143 100644 --- a/packages/jsapi-components/src/usePickerWithSelectedValues.test.ts +++ b/packages/jsapi-components/src/usePickerWithSelectedValues.test.ts @@ -1,5 +1,6 @@ import { act, renderHook } from '@testing-library/react-hooks'; -import type { FilterCondition, Table } from '@deephaven/jsapi-types'; +import type { dh as DhType } from '@deephaven/jsapi-types'; +import { usePickerItemScale } from '@deephaven/components'; import { createSearchTextFilter, createSelectedValuesFilter, @@ -21,6 +22,10 @@ jest.mock('./useFilterConditionFactories'); jest.mock('./useTableUtils'); jest.mock('./useViewportData'); jest.mock('./useViewportFilter'); +jest.mock('@deephaven/components', () => ({ + ...jest.requireActual('@deephaven/components'), + usePickerItemScale: jest.fn(), +})); jest.mock('@deephaven/jsapi-utils', () => ({ ...jest.requireActual('@deephaven/jsapi-utils'), createSearchTextFilter: jest.fn(), @@ -48,8 +53,8 @@ const mock = { typeof useDebouncedValue >, filter: [ - createMockProxy(), - createMockProxy(), + createMockProxy(), + createMockProxy(), ], filterConditionFactories: [jest.fn(), jest.fn()] as FilterConditionFactory[], keyedItem: createMockProxy>({ @@ -67,12 +72,14 @@ const mock = { }; const mockTable = { - usersAndGroups: createMockProxy(), - list: createMockProxy
(), + usersAndGroups: createMockProxy(), + list: createMockProxy(), }; function mockUseViewportData(size: number) { - const viewportData = createMockProxy>({ + const viewportData = createMockProxy< + UseViewportDataResult + >({ table: mockTable.list, viewportData: mock.viewportData, size, @@ -80,7 +87,9 @@ function mockUseViewportData(size: number) { asMock(useViewportData) .mockName('useViewportData') - .mockReturnValue(viewportData as UseViewportDataResult); + .mockReturnValue( + viewportData as UseViewportDataResult + ); } async function renderOnceAndWait( @@ -104,6 +113,9 @@ async function renderOnceAndWait( beforeEach(() => { jest.clearAllMocks(); + asMock(usePickerItemScale).mockName('usePickerItemScale').mockReturnValue({ + itemHeight: 32, + }); asMock(useTableUtils).mockName('useTableUtils').mockReturnValue(tableUtils); asMock(mock.mapItemToValue) diff --git a/packages/jsapi-components/src/usePickerWithSelectedValues.ts b/packages/jsapi-components/src/usePickerWithSelectedValues.ts index 1e4cc192f5..d63b2ef78a 100644 --- a/packages/jsapi-components/src/usePickerWithSelectedValues.ts +++ b/packages/jsapi-components/src/usePickerWithSelectedValues.ts @@ -10,8 +10,8 @@ import { useDebouncedValue, usePromiseFactory, } from '@deephaven/react-hooks'; +import { usePickerItemScale } from '@deephaven/components'; import { - COMBO_BOX_ITEM_HEIGHT, KeyedItem, SEARCH_DEBOUNCE_MS, SelectionT, @@ -62,6 +62,8 @@ export function usePickerWithSelectedValues({ filterConditionFactories?: FilterConditionFactory[]; trimSearchText?: boolean; }): UsePickerWithSelectedValuesResult { + const { itemHeight } = usePickerItemScale(); + const tableUtils = useTableUtils(); // `searchText` should always be up to date for controlled search input. @@ -141,7 +143,7 @@ export function usePickerWithSelectedValues({ const list = useViewportData({ table: listTable, - itemHeight: COMBO_BOX_ITEM_HEIGHT, + itemHeight, viewportSize: VIEWPORT_SIZE, viewportPadding: VIEWPORT_PADDING, }); diff --git a/packages/utils/src/UIConstants.ts b/packages/utils/src/UIConstants.ts index 2a5d7b962a..3181d311a8 100644 --- a/packages/utils/src/UIConstants.ts +++ b/packages/utils/src/UIConstants.ts @@ -1,6 +1,4 @@ export const ACTION_ICON_HEIGHT = 24; -export const COMBO_BOX_ITEM_HEIGHT = 32; -export const COMBO_BOX_TOP_OFFSET = 4; export const ITEM_KEY_PREFIX = 'DH_ITEM_KEY'; // https://github.com/adobe/react-spectrum/blob/main/packages/%40react-spectrum/listbox/src/ListBoxBase.tsx#L56 export const PICKER_ITEM_HEIGHTS = { diff --git a/tests/styleguide.spec.ts-snapshots/inputs-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-chromium-linux.png index 5e7fd2661f..b81d68614f 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-firefox-linux.png index aca566546f..7f65e44fa1 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-chromium-linux.png index e41cb510be..c3eb0fe318 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-firefox-linux.png index 2192caab50..084b2b3282 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-webkit-linux.png index 2e862cba42..d6f8024661 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row0-select-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-chromium-linux.png index 8bd50b4e45..733d6eceb3 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-firefox-linux.png index 0ea6fe6237..265ed5b1c8 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-webkit-linux.png index e08ea21cc1..e64c04d533 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row1-select-disabled-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-chromium-linux.png index 865a0bb39d..5dcb1462d5 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-firefox-linux.png index 65def74edc..d4f090d238 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-webkit-linux.png index 738beca1e4..3e5051843e 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row2-select-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-chromium-linux.png index cfb83bf7a1..8a76b712c6 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-firefox-linux.png index ecce293651..2da7726b0a 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-webkit-linux.png index c366cbeb2b..771e608fe6 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row3-text-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-chromium-linux.png index c37f303fb4..b233209738 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-firefox-linux.png index a43bae5bf5..83f27b2076 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-webkit-linux.png index 144069b750..aa634fa190 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row4-button-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-chromium-linux.png index a7584bd318..3e521016af 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-firefox-linux.png index 0ea6fe6237..265ed5b1c8 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-webkit-linux.png index e08ea21cc1..e64c04d533 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row5-text-disabled-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-chromium-linux.png index a7584bd318..3e521016af 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-firefox-linux.png index 0ea6fe6237..265ed5b1c8 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-webkit-linux.png index e08ea21cc1..e64c04d533 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-button-disabled-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/inputs-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/inputs-webkit-linux.png index ca38958d4f..73f6ebba73 100644 Binary files a/tests/styleguide.spec.ts-snapshots/inputs-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/inputs-webkit-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/pickers-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/pickers-chromium-linux.png index a313191681..10724bfe64 100644 Binary files a/tests/styleguide.spec.ts-snapshots/pickers-chromium-linux.png and b/tests/styleguide.spec.ts-snapshots/pickers-chromium-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/pickers-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/pickers-firefox-linux.png index d278c8cf68..c2d689247e 100644 Binary files a/tests/styleguide.spec.ts-snapshots/pickers-firefox-linux.png and b/tests/styleguide.spec.ts-snapshots/pickers-firefox-linux.png differ diff --git a/tests/styleguide.spec.ts-snapshots/pickers-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/pickers-webkit-linux.png index 174c78f47c..984f2d4ddb 100644 Binary files a/tests/styleguide.spec.ts-snapshots/pickers-webkit-linux.png and b/tests/styleguide.spec.ts-snapshots/pickers-webkit-linux.png differ