From 9c9c886925a480971fb22e457dc563d879da943d Mon Sep 17 00:00:00 2001 From: gzh2003 Date: Tue, 21 Apr 2026 11:46:54 -0400 Subject: [PATCH 01/15] multiselect plugin wrapper --- .../src/deephaven/ui/components/combo_box.py | 94 +++++++++++++++++-- .../ui/src/js/src/elements/MultiSelect.tsx | 71 ++++++++++++++ .../src/elements/hooks/useMultiSelectProps.ts | 63 +++++++++++++ plugins/ui/src/js/src/elements/index.ts | 1 + .../js/src/elements/model/ElementConstants.ts | 1 + plugins/ui/src/js/src/widget/WidgetUtils.tsx | 2 + 6 files changed, 226 insertions(+), 6 deletions(-) create mode 100644 plugins/ui/src/js/src/elements/MultiSelect.tsx create mode 100644 plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index 58b3da335..cbb389df8 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Callable, Any +from typing import Callable, Any, Literal from .types import ( FocusEventCallable, @@ -29,7 +29,7 @@ from .item_table_source import ItemTableSource from ..elements import BaseElement, Element, NodeType from .._internal.utils import create_props, unpack_item_table_source -from ..types import Key, Undefined, UndefinedType +from ..types import Key, Selection, Undefined, UndefinedType from .basic import component_element ComboBoxElement = BaseElement @@ -44,9 +44,56 @@ _NULLABLE_PROPS = ["selected_key"] +# Props that only apply to single-select ComboBox. +_SINGLE_ONLY_PROPS = { + "selected_key", + "default_selected_key", +} + +# Props that only apply to multi-select mode. +_MULTI_ONLY_PROPS = { + "selected_keys", + "default_selected_keys", +} + +# Props that raise a ValueError if explicitly set in the wrong mode. +_SINGLE_ONLY_VALIDATED = { + "selected_key": Undefined, + "default_selected_key": None, +} + +_MULTI_ONLY_VALIDATED = { + "selected_keys": None, + "default_selected_keys": None, +} + + +def _validate_selection_mode(props: dict[str, Any], mode: str) -> None: + """Validate and strip props that conflict with the given selection mode. + + Raises ValueError for props that conflict with the active mode, + and removes props that only apply to the other mode. + + Args: + props: The props to validate. + mode: The active selection mode. + """ + if mode == "multiple": + validated, strip = _SINGLE_ONLY_VALIDATED, _SINGLE_ONLY_PROPS + else: + validated, strip = _MULTI_ONLY_VALIDATED, _MULTI_ONLY_PROPS + + for prop, default in validated.items(): + val = props.get(prop) + if val is not default: + raise ValueError(f"'{prop}' is not supported when selection_mode='{mode}'.") + for prop in strip: + props.pop(prop, None) + def combo_box( *children: Item | SectionElement | Table | PartitionedTable | ItemTableSource, + selection_mode: Literal["single", "multiple"] = "single", menu_trigger: MenuTriggerAction | None = "input", is_quiet: bool | None = None, align: Align | None = "end", @@ -62,6 +109,8 @@ def combo_box( disabled_keys: list[Key] | None = None, selected_key: Key | None | UndefinedType = Undefined, default_selected_key: Key | None = None, + selected_keys: Selection | None = None, + default_selected_keys: Selection | None = None, is_disabled: bool | None = None, is_read_only: bool | None = None, is_required: bool | None = None, @@ -131,7 +180,13 @@ def combo_box( key: str | None = None, ) -> ComboBoxElement: """ - A combo box that can be used to search or select from a list. Children should be one of five types: + A combo box that can be used to search or select from a list. + + When `selection_mode="single"` (default), behaves as a standard ComboBox with a single + selected value. When `selection_mode="multiple"`, displays selected items as tags inside the + input area and presents a filterable dropdown list for multi-selection. + + Children should be one of five types: 1. If children are of type `Item`, they are the dropdown options. 2. If children are of type `SectionElement`, they are the dropdown sections. @@ -148,6 +203,8 @@ def combo_box( Args: *children: The options to render within the combo box. + selection_mode: Whether the combo box allows single or multiple selection. + Defaults to `"single"`. menu_trigger: The interaction required to display the ComboBox menu. is_quiet: Whether the ComboBox should be displayed with a quiet style. align: Alignment of the menu relative to the input target. @@ -157,16 +214,26 @@ def combo_box( should_flip: Whether the menu should automatically flip direction when space is limited. menu_width: Width of the menu. By default, matches width of the combobox. Note that the minimum width of the dropdown is always equal to the combobox's width. - form_value: Whether the text or key of the selected item is submitted as part of an HTML form. - When allowsCustomValue is true, this option does not apply and the text is always submitted. + form_value: Whether the text or key of the selected item(s) is submitted as part of an HTML form. + In single-select mode, when `allows_custom_value` is true, this option does not apply and the + text is always submitted. In multi-select mode, controls whether comma-joined keys or labels + are submitted via the hidden form input. should_focus_wrap: Whether keyboard navigation is circular. input_value: The value of the search input (controlled). default_input_value: The default value of the search input (uncontrolled). allows_custom_value: Whether the ComboBox allows a non-item matching input value to be set. + In multi-select mode, pressing Enter when no item is focused adds the typed text as a custom tag. + If the typed text matches an existing item's label, that item's key is used instead. disabled_keys: The item keys that are disabled. These items cannot be selected, focused, or otherwise interacted with. selected_key: The currently selected key in the collection (controlled). + Only applies in single-select mode. default_selected_key: The initial selected key in the collection (uncontrolled). + Only applies in single-select mode. + selected_keys: The currently selected keys in the collection (controlled). + Only applies in multi-select mode. + default_selected_keys: The initial selected keys in the collection (uncontrolled). + Only applies in multi-select mode. is_disabled: Whether the input is disabled. is_read_only: Whether the input can be selected but not changed by the user. is_required: Whether user input is required on the input before form submission. @@ -185,6 +252,8 @@ def combo_box( on_open_change: Method that is called when the open state of the menu changes. Returns the new open state and the action that caused the opening of the menu. on_selection_change: Handler that is called when the selection changes. + In single-select mode, receives the selected key. + In multi-select mode, receives the full selection (set of keys). on_change: Alias of `on_selection_change`. Handler that is called when the selection changes. on_input_change: Handler that is called when the ComboBox input value changes. on_focus: Handler that is called when the element receives focus. @@ -237,13 +306,26 @@ def combo_box( UNSAFE_style: A CSS style to apply to the element. key: A unique identifier used by React to render elements in a list. + Raises: + ValueError: If `selected_key` or `default_selected_key` is set when + `selection_mode="multiple"`. + ValueError: If `selected_keys` or `default_selected_keys` + is set when `selection_mode="single"`. + Returns: The rendered ComboBox. """ children, props = create_props(locals()) + is_multiple = props.pop("selection_mode", "single") == "multiple" + + _validate_selection_mode(props, "multiple" if is_multiple else "single") + children, props = unpack_item_table_source(children, props, SUPPORTED_SOURCE_ARGS) return component_element( - "ComboBox", *children, _nullable_props=_NULLABLE_PROPS, **props + "MultiSelect" if is_multiple else "ComboBox", + *children, + _nullable_props=[] if is_multiple else _NULLABLE_PROPS, + **props, ) diff --git a/plugins/ui/src/js/src/elements/MultiSelect.tsx b/plugins/ui/src/js/src/elements/MultiSelect.tsx new file mode 100644 index 000000000..44f136ba9 --- /dev/null +++ b/plugins/ui/src/js/src/elements/MultiSelect.tsx @@ -0,0 +1,71 @@ +import { useSelector } from 'react-redux'; +import { MultiSelect as DHMultiSelect } from '@deephaven/components'; +import { MultiSelect as DHMultiSelectJSApi } from '@deephaven/jsapi-components'; +import { isElementOfType } from '@deephaven/react-hooks'; +import type { dh } from '@deephaven/jsapi-types'; +import { ApiContext } from '@deephaven/jsapi-bootstrap'; +import { getSettings, RootState } from '@deephaven/redux'; +import { + SerializedMultiSelectProps, + useMultiSelectProps, +} from './hooks/useMultiSelectProps'; +import ObjectView from './ObjectView'; +import { useObjectViewObject } from './hooks/useObjectViewObject'; +import UriObjectView from './UriObjectView'; +import { getErrorShortMessage } from '../widget/WidgetErrorUtils'; + +export function MultiSelect( + props: SerializedMultiSelectProps +): JSX.Element | null { + const settings = useSelector(getSettings); + const { children, ...pickerProps } = useMultiSelectProps(props); + + const isObjectView = + isElementOfType(children, ObjectView) || + isElementOfType(children, UriObjectView); + const { + widget: table, + api, + isLoading, + error, + } = useObjectViewObject(children); + + if (isObjectView) { + if (error != null) { + const message = getErrorShortMessage(error); + return ( + + {[]} + + ); + } + if (isLoading || table == null || api == null) { + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + + {[]} + + ); + } + return ( + + + + ); + } + + // eslint-disable-next-line react/jsx-props-no-spreading + return {children}; +} + +export default MultiSelect; diff --git a/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts b/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts new file mode 100644 index 000000000..aeddff9f4 --- /dev/null +++ b/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts @@ -0,0 +1,63 @@ +import { MultiSelectProps as DHMultiSelectProps } from '@deephaven/components'; +import { MultiSelectProps as DHMultiSelectJSApiProps } from '@deephaven/jsapi-components'; +import { + SerializedSelectionProps, + useSelectionProps, +} from './useSelectionProps'; +import { + SerializedPickerEventProps, + WrappedDHPickerJSApiProps, +} from './usePickerProps'; +import { useFocusEventCallback } from './useFocusEventCallback'; +import { useKeyboardEventCallback } from './useKeyboardEventCallback'; + +type WrappedDHMultiSelectJSApiProps = + WrappedDHPickerJSApiProps; + +export type SerializedMultiSelectProps = ( + | DHMultiSelectProps + | WrappedDHMultiSelectJSApiProps +) & + SerializedSelectionProps & + SerializedPickerEventProps; + +/** + * Wrap MultiSelect props with the appropriate serialized event callbacks. + * @param props Props to wrap + * @returns Wrapped props + */ +export function useMultiSelectProps({ + onChange: serializedOnChange, + onSelectionChange: serializedOnSelectionChange, + onFocus, + onBlur, + onKeyDown, + onKeyUp, + ...otherProps +}: SerializedMultiSelectProps): + | DHMultiSelectProps + | WrappedDHMultiSelectJSApiProps { + const { onChange, onSelectionChange } = useSelectionProps({ + onChange: serializedOnChange, + onSelectionChange: serializedOnSelectionChange, + }); + + const deserializedOnFocus = useFocusEventCallback(onFocus); + const deserializedOnBlur = useFocusEventCallback(onBlur); + const deserializedOnKeyDown = useKeyboardEventCallback(onKeyDown); + const deserializedOnKeyUp = useKeyboardEventCallback(onKeyUp); + + return { + onChange, + onSelectionChange, + onFocus: deserializedOnFocus, + onBlur: deserializedOnBlur, + onKeyDown: deserializedOnKeyDown, + onKeyUp: deserializedOnKeyUp, + // The @deephaven/components `MultiSelect` has its own normalization logic + // that handles primitive children types (string, number, boolean). It also + // handles nested children inside of `Item` and `Section` components, so + // we are intentionally not wrapping `otherProps` in `mapSpectrumProps` + ...otherProps, + }; +} diff --git a/plugins/ui/src/js/src/elements/index.ts b/plugins/ui/src/js/src/elements/index.ts index 2ea1c76a0..0fbd38e34 100644 --- a/plugins/ui/src/js/src/elements/index.ts +++ b/plugins/ui/src/js/src/elements/index.ts @@ -28,6 +28,7 @@ export * from './LogicButton'; export * from './Markdown'; export * from './Menu'; export * from './Meter'; +export * from './MultiSelect'; export * from './model'; export * from './ObjectView'; export * from './Picker'; diff --git a/plugins/ui/src/js/src/elements/model/ElementConstants.ts b/plugins/ui/src/js/src/elements/model/ElementConstants.ts index 928da2331..c8447b82b 100644 --- a/plugins/ui/src/js/src/elements/model/ElementConstants.ts +++ b/plugins/ui/src/js/src/elements/model/ElementConstants.ts @@ -71,6 +71,7 @@ export const ELEMENT_NAME = { menu: uiComponentName('Menu'), menuTrigger: uiComponentName('MenuTrigger'), meter: uiComponentName('Meter'), + multiSelect: uiComponentName('MultiSelect'), numberField: uiComponentName('NumberField'), picker: uiComponentName('Picker'), progressBar: uiComponentName('ProgressBar'), diff --git a/plugins/ui/src/js/src/widget/WidgetUtils.tsx b/plugins/ui/src/js/src/widget/WidgetUtils.tsx index 00b00b1d0..a0c1106a7 100644 --- a/plugins/ui/src/js/src/widget/WidgetUtils.tsx +++ b/plugins/ui/src/js/src/widget/WidgetUtils.tsx @@ -88,6 +88,7 @@ import { Markdown, Menu, Meter, + MultiSelect, Picker, ProgressBar, ProgressCircle, @@ -186,6 +187,7 @@ export const elementComponentMap: Record, unknown> = { [ELEMENT_NAME.menu]: Menu, [ELEMENT_NAME.menuTrigger]: MenuTrigger, [ELEMENT_NAME.meter]: Meter, + [ELEMENT_NAME.multiSelect]: MultiSelect, [ELEMENT_NAME.numberField]: NumberField, [ELEMENT_NAME.picker]: Picker, [ELEMENT_NAME.progressBar]: ProgressBar, From db5a635a01a38a02fe2e73e49439d40926636e5d Mon Sep 17 00:00:00 2001 From: gzh2003 Date: Tue, 21 Apr 2026 12:00:53 -0400 Subject: [PATCH 02/15] fix docs error --- plugins/ui/src/deephaven/ui/components/combo_box.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index cbb389df8..865f2a185 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -307,10 +307,8 @@ def combo_box( key: A unique identifier used by React to render elements in a list. Raises: - ValueError: If `selected_key` or `default_selected_key` is set when - `selection_mode="multiple"`. - ValueError: If `selected_keys` or `default_selected_keys` - is set when `selection_mode="single"`. + ValueError: If `selected_key` or `default_selected_key` is set when `selection_mode="multiple"`. + ValueError: If `selected_keys` or `default_selected_keys` is set when `selection_mode="single"`. Returns: The rendered ComboBox. From 086748b22215260ab3915843e262972cc453fbd3 Mon Sep 17 00:00:00 2001 From: gzh2003 Date: Tue, 21 Apr 2026 12:07:12 -0400 Subject: [PATCH 03/15] Issue with raise entries in autodoc --- plugins/ui/src/deephaven/ui/components/combo_box.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index 865f2a185..ef332539c 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -204,7 +204,8 @@ def combo_box( Args: *children: The options to render within the combo box. selection_mode: Whether the combo box allows single or multiple selection. - Defaults to `"single"`. + Defaults to `"single"`. When `"multiple"`, use `selected_keys`/`default_selected_keys` + instead of `selected_key`/`default_selected_key`. menu_trigger: The interaction required to display the ComboBox menu. is_quiet: Whether the ComboBox should be displayed with a quiet style. align: Alignment of the menu relative to the input target. @@ -306,10 +307,6 @@ def combo_box( UNSAFE_style: A CSS style to apply to the element. key: A unique identifier used by React to render elements in a list. - Raises: - ValueError: If `selected_key` or `default_selected_key` is set when `selection_mode="multiple"`. - ValueError: If `selected_keys` or `default_selected_keys` is set when `selection_mode="single"`. - Returns: The rendered ComboBox. """ From 83107fe5aaf74749e645eaee70a5e1536216f7af Mon Sep 17 00:00:00 2001 From: gzh2003 Date: Sat, 25 Apr 2026 21:03:25 -0400 Subject: [PATCH 04/15] should be using hand rolled props --- plugins/ui/src/js/src/elements/MultiSelect.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/ui/src/js/src/elements/MultiSelect.tsx b/plugins/ui/src/js/src/elements/MultiSelect.tsx index 44f136ba9..f94b941ce 100644 --- a/plugins/ui/src/js/src/elements/MultiSelect.tsx +++ b/plugins/ui/src/js/src/elements/MultiSelect.tsx @@ -18,7 +18,7 @@ export function MultiSelect( props: SerializedMultiSelectProps ): JSX.Element | null { const settings = useSelector(getSettings); - const { children, ...pickerProps } = useMultiSelectProps(props); + const { children, ...multiSelectProps } = useMultiSelectProps(props); const isObjectView = isElementOfType(children, ObjectView) || @@ -36,7 +36,7 @@ export function MultiSelect( return ( @@ -47,7 +47,7 @@ export function MultiSelect( if (isLoading || table == null || api == null) { return ( // eslint-disable-next-line react/jsx-props-no-spreading - + {[]} ); @@ -56,7 +56,7 @@ export function MultiSelect( @@ -64,8 +64,10 @@ export function MultiSelect( ); } - // eslint-disable-next-line react/jsx-props-no-spreading - return {children}; + return ( + // eslint-disable-next-line react/jsx-props-no-spreading + {children} + ); } export default MultiSelect; From 23417ed8d76b7b1f847ed616eab80fdb6ff24286 Mon Sep 17 00:00:00 2001 From: gzh2003 Date: Mon, 27 Apr 2026 01:15:38 -0400 Subject: [PATCH 05/15] fix round trip issue --- plugins/ui/src/js/src/elements/MultiSelect.tsx | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/plugins/ui/src/js/src/elements/MultiSelect.tsx b/plugins/ui/src/js/src/elements/MultiSelect.tsx index f94b941ce..3f961d25a 100644 --- a/plugins/ui/src/js/src/elements/MultiSelect.tsx +++ b/plugins/ui/src/js/src/elements/MultiSelect.tsx @@ -23,12 +23,7 @@ export function MultiSelect( const isObjectView = isElementOfType(children, ObjectView) || isElementOfType(children, UriObjectView); - const { - widget: table, - api, - isLoading, - error, - } = useObjectViewObject(children); + const { widget: table, api, error } = useObjectViewObject(children); if (isObjectView) { if (error != null) { @@ -44,7 +39,10 @@ export function MultiSelect( ); } - if (isLoading || table == null || api == null) { + // Don't gate on `isLoading` as it flips true on server round-trips and + // would unmount/remount the spectrum MultiSelect, closing any open + // popover. + if (table == null || api == null) { return ( // eslint-disable-next-line react/jsx-props-no-spreading From 343d65136fe771ae60d5076b0b0b5bf394cbf68c Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Thu, 14 May 2026 10:30:50 -0500 Subject: [PATCH 06/15] deprecation --- .../src/deephaven/ui/components/combo_box.py | 82 ++++++++++++++++--- 1 file changed, 72 insertions(+), 10 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index ef332539c..c36cbfe60 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -1,5 +1,6 @@ from __future__ import annotations +import warnings from typing import Callable, Any, Literal from .types import ( @@ -91,9 +92,34 @@ def _validate_selection_mode(props: dict[str, Any], mode: str) -> None: props.pop(prop, None) +def _wrap_callback_as_selection( + callback: Callable[..., None] | None, +) -> Callable[..., None] | None: + """ + Wrap a callback so it always receives a Selection instead of a single Key. + + Args: + callback: The callback to wrap. + + Returns: + A wrapped callback that always receives a Selection. + """ + if callback is None: + return None + + def wrapper(value: Any) -> None: + if isinstance(value, (str, int, float, bool)): + callback([value]) + else: + callback(value) + + return wrapper + + def combo_box( *children: Item | SectionElement | Table | PartitionedTable | ItemTableSource, selection_mode: Literal["single", "multiple"] = "single", + selection_event: bool = False, menu_trigger: MenuTriggerAction | None = "input", is_quiet: bool | None = None, align: Align | None = "end", @@ -126,8 +152,8 @@ def combo_box( necessity_indicator: NecessityIndicator | None = None, contextual_help: Element | None = None, on_open_change: Callable[[bool, MenuTriggerAction], None] | None = None, - on_selection_change: Callable[[Key | None], None] | None = None, - on_change: Callable[[Key], None] | None = None, + on_selection_change: Callable[[Selection | None], None] | None = None, + on_change: Callable[[Selection], None] | None = None, on_input_change: Callable[[str], None] | None = None, on_focus: Callable[[FocusEventCallable], None] | None = None, on_blur: Callable[[FocusEventCallable], None] | None = None, @@ -206,6 +232,11 @@ def combo_box( selection_mode: Whether the combo box allows single or multiple selection. Defaults to `"single"`. When `"multiple"`, use `selected_keys`/`default_selected_keys` instead of `selected_key`/`default_selected_key`. + selection_event: When True, `on_selection_change` and `on_change` receive a + `Selection` (list of keys) instead of a single `Key` in single-select mode. + Defaults to False for backwards compatibility. Set to True to opt in to the + new behavior. In a future version, this will become the default and this + prop will be deprecated. menu_trigger: The interaction required to display the ComboBox menu. is_quiet: Whether the ComboBox should be displayed with a quiet style. align: Alignment of the menu relative to the input target. @@ -227,14 +258,10 @@ def combo_box( If the typed text matches an existing item's label, that item's key is used instead. disabled_keys: The item keys that are disabled. These items cannot be selected, focused, or otherwise interacted with. - selected_key: The currently selected key in the collection (controlled). - Only applies in single-select mode. - default_selected_key: The initial selected key in the collection (uncontrolled). - Only applies in single-select mode. + selected_key: Deprecated. Use `selected_keys` instead. + default_selected_key: Deprecated. Use `default_selected_keys` instead. selected_keys: The currently selected keys in the collection (controlled). - Only applies in multi-select mode. default_selected_keys: The initial selected keys in the collection (uncontrolled). - Only applies in multi-select mode. is_disabled: Whether the input is disabled. is_read_only: Whether the input can be selected but not changed by the user. is_required: Whether user input is required on the input before form submission. @@ -253,9 +280,11 @@ def combo_box( on_open_change: Method that is called when the open state of the menu changes. Returns the new open state and the action that caused the opening of the menu. on_selection_change: Handler that is called when the selection changes. - In single-select mode, receives the selected key. - In multi-select mode, receives the full selection (set of keys). + When `selection_event=True`, always receives a `Selection` (list of keys). + Otherwise, receives a single `Key` in single-select mode (deprecated). on_change: Alias of `on_selection_change`. Handler that is called when the selection changes. + When `selection_event=True`, always receives a `Selection` (list of keys). + Otherwise, receives a single `Key` in single-select mode (deprecated). on_input_change: Handler that is called when the ComboBox input value changes. on_focus: Handler that is called when the element receives focus. on_blur: Handler that is called when the element loses focus. @@ -312,7 +341,40 @@ def combo_box( """ children, props = create_props(locals()) + if selected_key is not Undefined: + warnings.warn( + "'selected_key' is deprecated. Use 'selected_keys' instead.", + DeprecationWarning, + stacklevel=2, + ) + if default_selected_key is not None: + warnings.warn( + "'default_selected_key' is deprecated. Use 'default_selected_keys' instead.", + DeprecationWarning, + stacklevel=2, + ) + is_multiple = props.pop("selection_mode", "single") == "multiple" + use_selection_event = props.pop("selection_event", False) + + if not is_multiple: + if use_selection_event: + for cb_name in ("on_selection_change", "on_change"): + cb = props.get(cb_name) + if cb is not None: + props[cb_name] = _wrap_callback_as_selection(cb) + else: + for cb_name in ("on_selection_change", "on_change"): + if props.get(cb_name) is not None: + warnings.warn( + f"'{cb_name}' currently receives a single Key in " + "single-select mode. In a future version, it will " + "receive a Selection (list of keys). Set " + "selection_event=True to opt in to the new behavior " + "and suppress this warning.", + DeprecationWarning, + stacklevel=2, + ) _validate_selection_mode(props, "multiple" if is_multiple else "single") From 665b17897ce5bb33f1981a5ba85d81a62c53f27b Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Thu, 14 May 2026 10:33:18 -0500 Subject: [PATCH 07/15] fixed --- plugins/ui/src/deephaven/ui/components/combo_box.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index c36cbfe60..5a0f9cbb9 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -230,8 +230,7 @@ def combo_box( Args: *children: The options to render within the combo box. selection_mode: Whether the combo box allows single or multiple selection. - Defaults to `"single"`. When `"multiple"`, use `selected_keys`/`default_selected_keys` - instead of `selected_key`/`default_selected_key`. + Defaults to `"single"`. selection_event: When True, `on_selection_change` and `on_change` receive a `Selection` (list of keys) instead of a single `Key` in single-select mode. Defaults to False for backwards compatibility. Set to True to opt in to the From 29ac4d251eaf6b1f6d936f07b85033d73e1785cd Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Thu, 14 May 2026 13:24:40 -0500 Subject: [PATCH 08/15] refactor and tests --- plugins/ui/docs/components/combo_box.md | 123 +++------- .../src/deephaven/ui/components/combo_box.py | 140 +++++------ .../src/js/src/elements/MultiSelect.test.tsx | 157 ++++++++++++ .../hooks/useMultiSelectProps.test.ts | 127 ++++++++++ .../ui/test/deephaven/ui/test_combo_box.py | 232 ++++++++++++++++++ 5 files changed, 613 insertions(+), 166 deletions(-) create mode 100644 plugins/ui/src/js/src/elements/MultiSelect.test.tsx create mode 100644 plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.test.ts create mode 100644 plugins/ui/test/deephaven/ui/test_combo_box.py diff --git a/plugins/ui/docs/components/combo_box.md b/plugins/ui/docs/components/combo_box.md index 65e4e9c20..9029035a0 100644 --- a/plugins/ui/docs/components/combo_box.md +++ b/plugins/ui/docs/components/combo_box.md @@ -10,7 +10,7 @@ from deephaven import ui @ui.component def ui_combo_box_basic(): - option, set_option = ui.use_state("") + option, set_option = ui.use_state([]) return ui.combo_box( ui.item("red panda"), @@ -21,7 +21,7 @@ def ui_combo_box_basic(): ui.item("snake"), ui.item("ant"), label="Favorite Animal", - selected_key=option, + selected_keys=option, on_change=set_option, ) @@ -262,9 +262,11 @@ my_combo_box_required_examples = ui_combo_box_required_examples() ## Selection -In a combo box, the `default_selected_key` or `selected_key` props set a selected option. +Use `selected_keys` or `default_selected_keys` to set the selected option(s). -The `default_selected_key` is useful for simpler scenarios where you don't need to control the state externally. The `selected_key` is used for scenarios where the state should be managed by the parent component, providing control and flexibility over the selection of the combo box. +`default_selected_keys` is useful for simpler scenarios where you don't need to control the state externally. `selected_keys` is used for scenarios where the state should be managed by the parent component, providing control and flexibility over the selection of the combo box. + +> [!NOTE] > `selected_key` and `default_selected_key` are deprecated. Use `selected_keys` and `default_selected_keys` instead. When the deprecated props are used, `on_selection_change` and `on_change` continue to receive a single key for backwards compatibility. When using the new props, callbacks always receive a list of keys. ```python from deephaven import ui @@ -272,7 +274,7 @@ from deephaven import ui @ui.component def ui_combo_box_selected_key_examples(): - option, set_option = ui.use_state("Option 1") + option, set_option = ui.use_state(["Option 1"]) return [ ui.combo_box( ui.item("Option 1"), @@ -284,7 +286,7 @@ def ui_combo_box_selected_key_examples(): ui.item("Option 7"), ui.item("Option 8"), ui.item("Option 9"), - default_selected_key="Option 2", + default_selected_keys=["Option 2"], label="Pick an option (uncontrolled)", ), ui.combo_box( @@ -297,7 +299,7 @@ def ui_combo_box_selected_key_examples(): ui.item("Option 7"), ui.item("Option 8"), ui.item("Option 9"), - selected_key=option, + selected_keys=option, on_change=set_option, label="Pick an option (controlled)", ), @@ -347,12 +349,14 @@ my_combo_box_section_example = ui.combo_box( ## Events -Combo boxes support selection via mouse, keyboard, and touch. You can handle all these via the `on_change` prop, which receives the selected key as an argument. Additionally, combo boxes accept an `on_input_change` prop, which is triggered whenever the search value is edited by the user, whether through typing or option selection. +Combo boxes support selection via mouse, keyboard, and touch. You can handle all these via the `on_change` prop. Additionally, combo boxes accept an `on_input_change` prop, which is triggered whenever the search value is edited by the user, whether through typing or option selection. Each interaction done in the combo box will trigger its associated event handler. For instance, typing in the input field will only trigger the `on_input_change`, not the `on_change`. Note, this is not the case for selections; when a selection is made, both the `on_change` and `on_input_change` are triggered. +> [!NOTE] > `on_change` and `on_selection_change` receive a `Selection` (list of keys) by default. When the deprecated `selected_key` or `default_selected_key` props are used, callbacks receive a single `Key` instead for backwards compatibility. Eventually the single key props will be removed and callbacks will always receive a list of keys. + ```python from deephaven import ui @@ -360,17 +364,15 @@ from deephaven import ui @ui.component def ui_combo_box_control_example(): input_value, set_input_value = ui.use_state("") - selection_state, set_selection_state = ui.use_state("") + selection_state, set_selection_state = ui.use_state([]) def handle_input_change(new_value): - set_selection_state("") set_input_value(new_value) - print(f"Text changed to {input_value}") + print(f"Text changed to {new_value}") def handle_selection_change(new_value): - set_input_value(new_value) set_selection_state(new_value) - print(f"Selection changed to {selection_state}") + print(f"Selection changed to {new_value}") return [ ui.combo_box( @@ -383,11 +385,10 @@ def ui_combo_box_control_example(): ui.item("Option 7"), ui.item("Option 8"), ui.item("Option 9"), - input_value=input_value, on_input_change=handle_input_change, - selected_key=selection_state, + selected_keys=selection_state, on_change=handle_selection_change, - ) + ), ] @@ -600,7 +601,7 @@ my_combo_box_is_read_only_example = ui.combo_box( ui.item("Option 6", key="Option 6"), ui.item("Option 7", key="Option 7"), ui.item("Option 8", key="Option 8"), - default_selected_key="Option 1", + default_selected_keys=["Option 1"], is_read_only=True, ) ``` @@ -765,84 +766,36 @@ def ui_combo_box_alignment_direction_examples(): my_combo_box_alignment_direction_examples = ui_combo_box_alignment_direction_examples() ``` -## How to create a multi-select component +## Multi-select -By leveraging the `on_change` handler of `ui.combo_box` to dynamically generate items, you can pair it with `ui.tag_group` to build a multi-select component. +Set `selection_mode="multiple"` to allow selecting multiple items. Selected items appear as tags inside the input area, and the dropdown list can be filtered by typing. ```python from deephaven import ui @ui.component -def ui_combo_box_multi_select_example( - options, on_input_change_callback=None, on_selection_change_callback=None -): - input_value, set_input_value = ui.use_state("") - selection_state, set_selection_state = ui.use_state("") - items, set_items = ui.use_state([]) - - def handle_input_change(new_value): - set_selection_state("") - set_input_value(new_value) - print(f"Text changed to {new_value}") - if on_input_change_callback: - on_input_change_callback(new_value) - - def handle_selection_change(new_value): - set_input_value("") - set_selection_state(new_value) - set_items( - lambda prev_items: prev_items + [new_value] - if new_value not in prev_items and new_value is not None - else prev_items - ) - print(f"Selection changed to {items}") - if on_selection_change_callback: - on_selection_change_callback(new_value, items) - - return [ - ui.flex( - ui.flex( - ui.combo_box( - *[ui.item(option) for option in options], - input_value=input_value, - on_input_change=handle_input_change, - selected_key=selection_state, - on_change=handle_selection_change, - ), - ui.tag_group( - *[ui.item(item, key=item.lower()) for item in items], - on_remove=lambda keys: set_items( - [item for item in items if item.lower() not in keys] - ), - ), - direction="row", - align_items="center", - ), - align_items="start", - ) - ] +def ui_combo_box_multi_select_example(): + selected, set_selected = ui.use_state([]) + return ui.combo_box( + ui.item("Option 1"), + ui.item("Option 2"), + ui.item("Option 3"), + ui.item("Option 4"), + ui.item("Option 5"), + ui.item("Option 6"), + ui.item("Option 7"), + ui.item("Option 8"), + ui.item("Option 9"), + selection_mode="multiple", + selected_keys=selected, + on_change=set_selected, + label="Pick options", + ) -my_options = [ - "Option 1", - "Option 2", - "Option 3", - "Option 4", - "Option 5", - "Option 6", - "Option 7", - "Option 8", - "Option 9", -] -my_combo_box_multi_select_example = ui_combo_box_multi_select_example( - options=my_options, - on_input_change_callback=lambda value: print(f"Custom input handler: {value}"), - on_selection_change_callback=lambda value, items: print( - f"Custom selection handler: {value}, {items}" - ), -) +my_combo_box_multi_select_example = ui_combo_box_multi_select_example() ``` ## API Reference diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index 5a0f9cbb9..f99eb358b 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -45,52 +45,18 @@ _NULLABLE_PROPS = ["selected_key"] -# Props that only apply to single-select ComboBox. +# Props that only apply to single-select ComboBox and are stripped in multi mode. _SINGLE_ONLY_PROPS = { "selected_key", "default_selected_key", } -# Props that only apply to multi-select mode. +# Props that only apply to multi-select mode and are stripped in single mode. _MULTI_ONLY_PROPS = { "selected_keys", "default_selected_keys", } -# Props that raise a ValueError if explicitly set in the wrong mode. -_SINGLE_ONLY_VALIDATED = { - "selected_key": Undefined, - "default_selected_key": None, -} - -_MULTI_ONLY_VALIDATED = { - "selected_keys": None, - "default_selected_keys": None, -} - - -def _validate_selection_mode(props: dict[str, Any], mode: str) -> None: - """Validate and strip props that conflict with the given selection mode. - - Raises ValueError for props that conflict with the active mode, - and removes props that only apply to the other mode. - - Args: - props: The props to validate. - mode: The active selection mode. - """ - if mode == "multiple": - validated, strip = _SINGLE_ONLY_VALIDATED, _SINGLE_ONLY_PROPS - else: - validated, strip = _MULTI_ONLY_VALIDATED, _MULTI_ONLY_PROPS - - for prop, default in validated.items(): - val = props.get(prop) - if val is not default: - raise ValueError(f"'{prop}' is not supported when selection_mode='{mode}'.") - for prop in strip: - props.pop(prop, None) - def _wrap_callback_as_selection( callback: Callable[..., None] | None, @@ -116,10 +82,60 @@ def wrapper(value: Any) -> None: return wrapper +def _process_selection_props( + props: dict[str, Any], + is_multiple: bool, + *, + stacklevel: int = 3, +) -> None: + """Process selection-related props: emit deprecation warnings, strip + inapplicable props, and wrap callbacks when needed. + + When the deprecated ``selected_key`` / ``default_selected_key`` props are + used, callbacks continue to receive a single ``Key``. When only the new + ``selected_keys`` / ``default_selected_keys`` props are used, callbacks in + single-select mode are wrapped so they always receive a ``Selection``. + + Args: + props: Mutable props dict (modified in place). + is_multiple: Whether multi-select mode is active. + stacklevel: Stack level passed to warnings.warn to point to the caller's code. + """ + uses_deprecated = ( + props.get("selected_key") is not Undefined + or props.get("default_selected_key") is not None + ) + + # warn about deprecated single-select props if they are set + if props.get("selected_key") is not Undefined: + warnings.warn( + "'selected_key' is deprecated. Use 'selected_keys' instead.", + DeprecationWarning, + stacklevel=stacklevel, + ) + if props.get("default_selected_key") is not None: + warnings.warn( + "'default_selected_key' is deprecated. Use 'default_selected_keys' instead.", + DeprecationWarning, + stacklevel=stacklevel, + ) + + # strip props that don't apply to the active mode + for prop in _SINGLE_ONLY_PROPS if is_multiple else _MULTI_ONLY_PROPS: + props.pop(prop, None) + + # When not using deprecated key props in single-select mode, wrap + # callbacks so they receive a Selection instead of a single Key. + if not is_multiple and not uses_deprecated: + for cb_name in ("on_selection_change", "on_change"): + cb = props.get(cb_name) + if cb is not None: + props[cb_name] = _wrap_callback_as_selection(cb) + + def combo_box( *children: Item | SectionElement | Table | PartitionedTable | ItemTableSource, selection_mode: Literal["single", "multiple"] = "single", - selection_event: bool = False, menu_trigger: MenuTriggerAction | None = "input", is_quiet: bool | None = None, align: Align | None = "end", @@ -231,11 +247,6 @@ def combo_box( *children: The options to render within the combo box. selection_mode: Whether the combo box allows single or multiple selection. Defaults to `"single"`. - selection_event: When True, `on_selection_change` and `on_change` receive a - `Selection` (list of keys) instead of a single `Key` in single-select mode. - Defaults to False for backwards compatibility. Set to True to opt in to the - new behavior. In a future version, this will become the default and this - prop will be deprecated. menu_trigger: The interaction required to display the ComboBox menu. is_quiet: Whether the ComboBox should be displayed with a quiet style. align: Alignment of the menu relative to the input target. @@ -279,11 +290,11 @@ def combo_box( on_open_change: Method that is called when the open state of the menu changes. Returns the new open state and the action that caused the opening of the menu. on_selection_change: Handler that is called when the selection changes. - When `selection_event=True`, always receives a `Selection` (list of keys). - Otherwise, receives a single `Key` in single-select mode (deprecated). + Receives a `Selection` (list of keys). When the deprecated `selected_key` + or `default_selected_key` props are used, receives a single `Key` instead. on_change: Alias of `on_selection_change`. Handler that is called when the selection changes. - When `selection_event=True`, always receives a `Selection` (list of keys). - Otherwise, receives a single `Key` in single-select mode (deprecated). + Receives a `Selection` (list of keys). When the deprecated `selected_key` + or `default_selected_key` props are used, receives a single `Key` instead. on_input_change: Handler that is called when the ComboBox input value changes. on_focus: Handler that is called when the element receives focus. on_blur: Handler that is called when the element loses focus. @@ -340,42 +351,9 @@ def combo_box( """ children, props = create_props(locals()) - if selected_key is not Undefined: - warnings.warn( - "'selected_key' is deprecated. Use 'selected_keys' instead.", - DeprecationWarning, - stacklevel=2, - ) - if default_selected_key is not None: - warnings.warn( - "'default_selected_key' is deprecated. Use 'default_selected_keys' instead.", - DeprecationWarning, - stacklevel=2, - ) - is_multiple = props.pop("selection_mode", "single") == "multiple" - use_selection_event = props.pop("selection_event", False) - - if not is_multiple: - if use_selection_event: - for cb_name in ("on_selection_change", "on_change"): - cb = props.get(cb_name) - if cb is not None: - props[cb_name] = _wrap_callback_as_selection(cb) - else: - for cb_name in ("on_selection_change", "on_change"): - if props.get(cb_name) is not None: - warnings.warn( - f"'{cb_name}' currently receives a single Key in " - "single-select mode. In a future version, it will " - "receive a Selection (list of keys). Set " - "selection_event=True to opt in to the new behavior " - "and suppress this warning.", - DeprecationWarning, - stacklevel=2, - ) - - _validate_selection_mode(props, "multiple" if is_multiple else "single") + + _process_selection_props(props, is_multiple) children, props = unpack_item_table_source(children, props, SUPPORTED_SOURCE_ARGS) diff --git a/plugins/ui/src/js/src/elements/MultiSelect.test.tsx b/plugins/ui/src/js/src/elements/MultiSelect.test.tsx new file mode 100644 index 000000000..b2ec7130a --- /dev/null +++ b/plugins/ui/src/js/src/elements/MultiSelect.test.tsx @@ -0,0 +1,157 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import { MultiSelect } from './MultiSelect'; +import type { SerializedMultiSelectProps } from './hooks/useMultiSelectProps'; + +// Mock ObjectView and UriObjectView before they trigger deep dependency chains +jest.mock('./ObjectView', () => jest.fn(() => null)); +jest.mock('./UriObjectView', () => jest.fn(() => null)); +jest.mock('../widget/WidgetErrorUtils', () => ({ + getErrorShortMessage: jest.fn((e: Error) => e.message), +})); + +// Mock all heavy dependencies +jest.mock('react-redux', () => ({ + useSelector: jest.fn(() => ({})), +})); + +jest.mock('./hooks/useMultiSelectProps', () => ({ + useMultiSelectProps: jest.fn((props: Record) => { + const { + onChange, + onSelectionChange, + onFocus, + onBlur, + onKeyDown, + onKeyUp, + ...rest + } = props; + return rest; + }), +})); + +jest.mock('./hooks/useObjectViewObject', () => ({ + useObjectViewObject: jest.fn(() => ({ + widget: null, + api: null, + isLoading: false, + error: null, + })), +})); + +jest.mock('@deephaven/components', () => ({ + MultiSelect: jest.fn( + ({ + children, + ...props + }: { + children?: React.ReactNode; + [key: string]: unknown; + }) => ( +
+ {children} +
+ ) + ), +})); + +jest.mock('@deephaven/jsapi-components', () => ({ + MultiSelect: jest.fn(() =>
), +})); + +jest.mock('@deephaven/react-hooks', () => ({ + isElementOfType: jest.fn(() => false), +})); + +jest.mock('@deephaven/jsapi-bootstrap', () => ({ + ApiContext: { + Provider: ({ children }: { children: React.ReactNode }) => <>{children}, + }, +})); + +jest.mock('@deephaven/redux', () => ({ + getSettings: jest.fn(() => ({})), +})); + +describe('MultiSelect', () => { + it('renders DHMultiSelect with children when not an ObjectView', () => { + const props = { + children: ['Option A', 'Option B'], + label: 'Test', + } as unknown as SerializedMultiSelectProps; + + const { getByTestId } = render(); + expect(getByTestId('dh-multi-select')).toBeTruthy(); + }); + + it('renders loading state when ObjectView with no table', () => { + const { isElementOfType } = jest.requireMock('@deephaven/react-hooks'); + isElementOfType.mockReturnValue(true); + + const { useObjectViewObject } = jest.requireMock( + './hooks/useObjectViewObject' + ); + useObjectViewObject.mockReturnValue({ + widget: null, + api: null, + isLoading: true, + error: null, + }); + + const props = { + children: React.createElement('div'), + label: 'Loading test', + } as unknown as SerializedMultiSelectProps; + + const { getByTestId } = render(); + const el = getByTestId('dh-multi-select'); + expect(el).toBeTruthy(); + }); + + it('renders error state when ObjectView has error', () => { + const { isElementOfType } = jest.requireMock('@deephaven/react-hooks'); + isElementOfType.mockReturnValue(true); + + const { useObjectViewObject } = jest.requireMock( + './hooks/useObjectViewObject' + ); + useObjectViewObject.mockReturnValue({ + widget: null, + api: null, + isLoading: false, + error: new Error('Test error'), + }); + + const props = { + children: React.createElement('div'), + label: 'Error test', + } as unknown as SerializedMultiSelectProps; + + const { getByTestId } = render(); + const el = getByTestId('dh-multi-select'); + expect(el).toBeTruthy(); + }); + + it('renders JSApi MultiSelect when ObjectView has table and api', () => { + const { isElementOfType } = jest.requireMock('@deephaven/react-hooks'); + isElementOfType.mockReturnValue(true); + + const { useObjectViewObject } = jest.requireMock( + './hooks/useObjectViewObject' + ); + useObjectViewObject.mockReturnValue({ + widget: {}, + api: {}, + isLoading: false, + error: null, + }); + + const props = { + children: React.createElement('div'), + label: 'JSApi test', + } as unknown as SerializedMultiSelectProps; + + const { getByTestId } = render(); + expect(getByTestId('dh-multi-select-jsapi')).toBeTruthy(); + }); +}); diff --git a/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.test.ts b/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.test.ts new file mode 100644 index 000000000..54c1540b9 --- /dev/null +++ b/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.test.ts @@ -0,0 +1,127 @@ +import { renderHook, act } from '@testing-library/react'; +import { useMultiSelectProps } from './useMultiSelectProps'; +import type { SerializedMultiSelectProps } from './useMultiSelectProps'; + +describe('useMultiSelectProps', () => { + it('passes through other props unchanged', () => { + const props = { + label: 'Test Label', + isDisabled: true, + selectedKeys: ['a', 'b'], + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + expect(result.current).toMatchObject({ + label: 'Test Label', + isDisabled: true, + selectedKeys: ['a', 'b'], + }); + }); + + it('deserializes onChange into a function', () => { + const onChange = jest.fn(); + const props = { + onChange, + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + expect(result.current.onChange).toBeDefined(); + expect(typeof result.current.onChange).toBe('function'); + }); + + it('deserializes onSelectionChange into a function', () => { + const onSelectionChange = jest.fn(); + const props = { + onSelectionChange, + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + expect(result.current.onSelectionChange).toBeDefined(); + expect(typeof result.current.onSelectionChange).toBe('function'); + }); + + it('serializes Set selection to array when onChange fires', () => { + const onChange = jest.fn(); + const props = { + onChange, + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + act(() => { + result.current.onChange?.(new Set(['a', 'b'])); + }); + + expect(onChange).toHaveBeenCalledWith(['a', 'b']); + }); + + it('serializes Set selection to array when onSelectionChange fires', () => { + const onSelectionChange = jest.fn(); + const props = { + onSelectionChange, + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + act(() => { + result.current.onSelectionChange?.(new Set(['x', 'y'])); + }); + + expect(onSelectionChange).toHaveBeenCalledWith(['x', 'y']); + }); + + it('passes "all" selection through unchanged', () => { + const onChange = jest.fn(); + const props = { + onChange, + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + act(() => { + result.current.onChange?.('all'); + }); + + expect(onChange).toHaveBeenCalledWith('all'); + }); + + it('deserializes focus and blur callbacks', () => { + const onFocus = jest.fn(); + const onBlur = jest.fn(); + const props = { + onFocus, + onBlur, + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + expect(result.current.onFocus).toBeDefined(); + expect(result.current.onBlur).toBeDefined(); + }); + + it('deserializes keyboard callbacks', () => { + const onKeyDown = jest.fn(); + const onKeyUp = jest.fn(); + const props = { + onKeyDown, + onKeyUp, + } as unknown as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + expect(result.current.onKeyDown).toBeDefined(); + expect(result.current.onKeyUp).toBeDefined(); + }); + + it('returns undefined for omitted callbacks', () => { + const props = {} as SerializedMultiSelectProps; + + const { result } = renderHook(() => useMultiSelectProps(props)); + + expect(result.current.onChange).toBeUndefined(); + expect(result.current.onSelectionChange).toBeUndefined(); + }); +}); diff --git a/plugins/ui/test/deephaven/ui/test_combo_box.py b/plugins/ui/test/deephaven/ui/test_combo_box.py new file mode 100644 index 000000000..03fb4f8b9 --- /dev/null +++ b/plugins/ui/test/deephaven/ui/test_combo_box.py @@ -0,0 +1,232 @@ +import unittest +import warnings + +from .BaseTest import BaseTestCase + + +class ComboBoxProcessSelectionPropsTest(BaseTestCase): + def setUp(self): + from deephaven.ui.types import Undefined + + self.Undefined = Undefined + + def _process(self, props, is_multiple): + from deephaven.ui.components.combo_box import _process_selection_props + + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + _process_selection_props(props, is_multiple) + + def test_single_mode_strips_multi_props(self): + props = { + "selected_keys": ["a", "b"], + "default_selected_keys": ["c"], + "other": "value", + } + self._process(props, is_multiple=False) + self.assertNotIn("selected_keys", props) + self.assertNotIn("default_selected_keys", props) + self.assertEqual(props["other"], "value") + + def test_multiple_mode_strips_single_props(self): + props = { + "selected_key": self.Undefined, + "default_selected_key": None, + "other": "value", + } + self._process(props, is_multiple=True) + self.assertNotIn("selected_key", props) + self.assertNotIn("default_selected_key", props) + self.assertEqual(props["other"], "value") + + def test_multiple_mode_strips_set_single_props(self): + props = { + "selected_key": "some_key", + "default_selected_key": "other", + } + self._process(props, is_multiple=True) + self.assertNotIn("selected_key", props) + self.assertNotIn("default_selected_key", props) + + +class ComboBoxWrapCallbackTest(BaseTestCase): + def _wrap(self, callback): + from deephaven.ui.components.combo_box import _wrap_callback_as_selection + + return _wrap_callback_as_selection(callback) + + def test_none_returns_none(self): + self.assertIsNone(self._wrap(None)) + + def test_wraps_string_key(self): + received = [] + wrapped = self._wrap(lambda v: received.append(v)) + wrapped("my_key") + self.assertEqual(received, [["my_key"]]) + + def test_wraps_int_key(self): + received = [] + wrapped = self._wrap(lambda v: received.append(v)) + wrapped(42) + self.assertEqual(received, [[42]]) + + def test_wraps_float_key(self): + received = [] + wrapped = self._wrap(lambda v: received.append(v)) + wrapped(3.14) + self.assertEqual(received, [[3.14]]) + + def test_wraps_bool_key(self): + received = [] + wrapped = self._wrap(lambda v: received.append(v)) + wrapped(True) + self.assertEqual(received, [[True]]) + + def test_passes_list_through(self): + received = [] + wrapped = self._wrap(lambda v: received.append(v)) + wrapped(["a", "b"]) + self.assertEqual(received, [["a", "b"]]) + + def test_passes_none_through(self): + received = [] + wrapped = self._wrap(lambda v: received.append(v)) + wrapped(None) + self.assertEqual(received, [None]) + + +class ComboBoxDeprecationTest(BaseTestCase): + def test_selected_key_warns(self): + from deephaven.ui import combo_box + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + combo_box(selected_key="a") + dep_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)] + messages = [str(x.message) for x in dep_warnings] + self.assertTrue( + any("selected_key" in m and "selected_keys" in m for m in messages), + f"Expected selected_key deprecation warning, got: {messages}", + ) + + def test_default_selected_key_warns(self): + from deephaven.ui import combo_box + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + combo_box(default_selected_key="a") + dep_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)] + messages = [str(x.message) for x in dep_warnings] + self.assertTrue( + any( + "default_selected_key" in m and "default_selected_keys" in m + for m in messages + ), + f"Expected default_selected_key deprecation warning, got: {messages}", + ) + + def test_no_warning_when_defaults(self): + from deephaven.ui import combo_box + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + combo_box() + dep_warnings = [ + x + for x in w + if issubclass(x.category, DeprecationWarning) + and ("selected_key" in str(x.message)) + ] + self.assertEqual( + len(dep_warnings), + 0, + f"Unexpected deprecation warning: {[str(x.message) for x in dep_warnings]}", + ) + + +class ComboBoxCallbackWrappingTest(BaseTestCase): + """Callbacks are wrapped to receive Selection when deprecated key props are NOT used.""" + + def _process(self, props, is_multiple): + from deephaven.ui.components.combo_box import _process_selection_props + + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + _process_selection_props(props, is_multiple) + + def test_single_wraps_when_no_deprecated_props(self): + from deephaven.ui.types import Undefined + + received = [] + handler = lambda v: received.append(v) + props = { + "selected_key": Undefined, + "default_selected_key": None, + "on_change": handler, + } + self._process(props, is_multiple=False) + props["on_change"]("my_key") + self.assertEqual(received, [["my_key"]]) + + def test_single_no_wrap_when_selected_key_used(self): + received = [] + handler = lambda v: received.append(v) + props = { + "selected_key": "a", + "default_selected_key": None, + "on_change": handler, + } + self._process(props, is_multiple=False) + props["on_change"]("my_key") + self.assertEqual(received, ["my_key"]) + + def test_single_no_wrap_when_default_selected_key_used(self): + from deephaven.ui.types import Undefined + + received = [] + handler = lambda v: received.append(v) + props = { + "selected_key": Undefined, + "default_selected_key": "b", + "on_change": handler, + } + self._process(props, is_multiple=False) + props["on_change"]("my_key") + self.assertEqual(received, ["my_key"]) + + def test_multiple_no_wrap_regardless(self): + received = [] + handler = lambda v: received.append(v) + props = { + "selected_key": "x", + "default_selected_key": None, + "on_change": handler, + } + self._process(props, is_multiple=True) + # single props are stripped, callback untouched + props["on_change"](["a", "b"]) + self.assertEqual(received, [["a", "b"]]) + + +class ComboBoxSelectionModeTest(BaseTestCase): + def test_single_mode_renders_combo_box(self): + from deephaven.ui import combo_box + + result = combo_box(label="Test") + self.assertEqual(result.name, "deephaven.ui.components.ComboBox") + + def test_multiple_mode_renders_multi_select(self): + from deephaven.ui import combo_box + + result = combo_box(selection_mode="multiple", label="Test") + self.assertEqual(result.name, "deephaven.ui.components.MultiSelect") + + def test_multiple_mode_accepts_selected_keys(self): + from deephaven.ui import combo_box + + result = combo_box(selection_mode="multiple", selected_keys=["a", "b"]) + self.assertEqual(result.name, "deephaven.ui.components.MultiSelect") + + +if __name__ == "__main__": + unittest.main() From ce954faa855c6c4001bceca0ca1890babb47000d Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Thu, 14 May 2026 14:05:56 -0500 Subject: [PATCH 09/15] fixing issues --- .../src/js/src/elements/MultiSelect.test.tsx | 42 ++++++++++++------- .../ui/src/js/src/elements/MultiSelect.tsx | 4 +- .../src/elements/hooks/useMultiSelectProps.ts | 8 ++-- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/plugins/ui/src/js/src/elements/MultiSelect.test.tsx b/plugins/ui/src/js/src/elements/MultiSelect.test.tsx index b2ec7130a..7018b8a0c 100644 --- a/plugins/ui/src/js/src/elements/MultiSelect.test.tsx +++ b/plugins/ui/src/js/src/elements/MultiSelect.test.tsx @@ -41,16 +41,8 @@ jest.mock('./hooks/useObjectViewObject', () => ({ jest.mock('@deephaven/components', () => ({ MultiSelect: jest.fn( - ({ - children, - ...props - }: { - children?: React.ReactNode; - [key: string]: unknown; - }) => ( -
- {children} -
+ ({ children }: { children?: React.ReactNode; [key: string]: unknown }) => ( +
{children}
) ), })); @@ -65,7 +57,7 @@ jest.mock('@deephaven/react-hooks', () => ({ jest.mock('@deephaven/jsapi-bootstrap', () => ({ ApiContext: { - Provider: ({ children }: { children: React.ReactNode }) => <>{children}, + Provider: ({ children }: { children: React.ReactNode }) => children, }, })); @@ -80,7 +72,12 @@ describe('MultiSelect', () => { label: 'Test', } as unknown as SerializedMultiSelectProps; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); expect(getByTestId('dh-multi-select')).toBeTruthy(); }); @@ -103,7 +100,12 @@ describe('MultiSelect', () => { label: 'Loading test', } as unknown as SerializedMultiSelectProps; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const el = getByTestId('dh-multi-select'); expect(el).toBeTruthy(); }); @@ -127,7 +129,12 @@ describe('MultiSelect', () => { label: 'Error test', } as unknown as SerializedMultiSelectProps; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); const el = getByTestId('dh-multi-select'); expect(el).toBeTruthy(); }); @@ -151,7 +158,12 @@ describe('MultiSelect', () => { label: 'JSApi test', } as unknown as SerializedMultiSelectProps; - const { getByTestId } = render(); + const { getByTestId } = render( + + ); expect(getByTestId('dh-multi-select-jsapi')).toBeTruthy(); }); }); diff --git a/plugins/ui/src/js/src/elements/MultiSelect.tsx b/plugins/ui/src/js/src/elements/MultiSelect.tsx index 3f961d25a..ee8cdf2b1 100644 --- a/plugins/ui/src/js/src/elements/MultiSelect.tsx +++ b/plugins/ui/src/js/src/elements/MultiSelect.tsx @@ -4,9 +4,9 @@ import { MultiSelect as DHMultiSelectJSApi } from '@deephaven/jsapi-components'; import { isElementOfType } from '@deephaven/react-hooks'; import type { dh } from '@deephaven/jsapi-types'; import { ApiContext } from '@deephaven/jsapi-bootstrap'; -import { getSettings, RootState } from '@deephaven/redux'; +import { getSettings, type RootState } from '@deephaven/redux'; import { - SerializedMultiSelectProps, + type SerializedMultiSelectProps, useMultiSelectProps, } from './hooks/useMultiSelectProps'; import ObjectView from './ObjectView'; diff --git a/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts b/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts index aeddff9f4..62119083d 100644 --- a/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts +++ b/plugins/ui/src/js/src/elements/hooks/useMultiSelectProps.ts @@ -1,10 +1,10 @@ -import { MultiSelectProps as DHMultiSelectProps } from '@deephaven/components'; -import { MultiSelectProps as DHMultiSelectJSApiProps } from '@deephaven/jsapi-components'; +import type { MultiSelectProps as DHMultiSelectProps } from '@deephaven/components'; +import type { MultiSelectProps as DHMultiSelectJSApiProps } from '@deephaven/jsapi-components'; import { - SerializedSelectionProps, + type SerializedSelectionProps, useSelectionProps, } from './useSelectionProps'; -import { +import type { SerializedPickerEventProps, WrappedDHPickerJSApiProps, } from './usePickerProps'; From 0498ace7d8cc8fae3a206cbee282fa9378634391 Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Thu, 14 May 2026 14:50:56 -0500 Subject: [PATCH 10/15] fixed screenshots --- plugins/ui/docs/snapshots/0761e766ac976e28061e143e034b23bf.json | 1 - plugins/ui/docs/snapshots/09532531c03ea0d5591a4a03b09161a1.json | 1 + plugins/ui/docs/snapshots/2747a4e6e572a96106716910e764ee1d.json | 1 + plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json | 1 + plugins/ui/docs/snapshots/87f46b40b82cb2efc19f5bdbab3ac2a3.json | 1 + plugins/ui/docs/snapshots/94ab46014a1ec4241a826e7b08b163ef.json | 1 - plugins/ui/docs/snapshots/a3924153476e117f57e563dacb3b31d5.json | 1 + plugins/ui/docs/snapshots/ec0ed3d95653ca3d2062db0a5ffcbc68.json | 1 - plugins/ui/docs/snapshots/f1891462b585b31dabf2e7d96395c968.json | 1 - plugins/ui/docs/snapshots/fa4c431cba8e02a50fb02b9cbf6dd6de.json | 1 - 10 files changed, 5 insertions(+), 5 deletions(-) delete mode 100644 plugins/ui/docs/snapshots/0761e766ac976e28061e143e034b23bf.json create mode 100644 plugins/ui/docs/snapshots/09532531c03ea0d5591a4a03b09161a1.json create mode 100644 plugins/ui/docs/snapshots/2747a4e6e572a96106716910e764ee1d.json create mode 100644 plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json create mode 100644 plugins/ui/docs/snapshots/87f46b40b82cb2efc19f5bdbab3ac2a3.json delete mode 100644 plugins/ui/docs/snapshots/94ab46014a1ec4241a826e7b08b163ef.json create mode 100644 plugins/ui/docs/snapshots/a3924153476e117f57e563dacb3b31d5.json delete mode 100644 plugins/ui/docs/snapshots/ec0ed3d95653ca3d2062db0a5ffcbc68.json delete mode 100644 plugins/ui/docs/snapshots/f1891462b585b31dabf2e7d96395c968.json delete mode 100644 plugins/ui/docs/snapshots/fa4c431cba8e02a50fb02b9cbf6dd6de.json diff --git a/plugins/ui/docs/snapshots/0761e766ac976e28061e143e034b23bf.json b/plugins/ui/docs/snapshots/0761e766ac976e28061e143e034b23bf.json deleted file mode 100644 index 62b851727..000000000 --- a/plugins/ui/docs/snapshots/0761e766ac976e28061e143e034b23bf.json +++ /dev/null @@ -1 +0,0 @@ -{"file":"components/combo_box.md","objects":{"my_combo_box_selected_key_examples":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":[{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","defaultSelectedKey":"Option 2","validationBehavior":"aria","label":"Pick an option (uncontrolled)","labelPosition":"top","children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}},{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","selectedKey":"Option 1","validationBehavior":"aria","label":"Pick an option (controlled)","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}}]},"__dhElemName":"__main__.ui_combo_box_selected_key_examples"},"state":"{\"state\": {\"0\": \"Option 1\"}}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/09532531c03ea0d5591a4a03b09161a1.json b/plugins/ui/docs/snapshots/09532531c03ea0d5591a4a03b09161a1.json new file mode 100644 index 000000000..6026f6711 --- /dev/null +++ b/plugins/ui/docs/snapshots/09532531c03ea0d5591a4a03b09161a1.json @@ -0,0 +1 @@ +{"file":"components/combo_box.md","objects":{"my_combo_box_selected_key_examples":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":[{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","validationBehavior":"aria","label":"Pick an option (uncontrolled)","labelPosition":"top","children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}},{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","validationBehavior":"aria","label":"Pick an option (controlled)","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}}]},"__dhElemName":"__main__.ui_combo_box_selected_key_examples"},"state":"{}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/2747a4e6e572a96106716910e764ee1d.json b/plugins/ui/docs/snapshots/2747a4e6e572a96106716910e764ee1d.json new file mode 100644 index 000000000..d857abb1f --- /dev/null +++ b/plugins/ui/docs/snapshots/2747a4e6e572a96106716910e764ee1d.json @@ -0,0 +1 @@ +{"file":"components/combo_box.md","objects":{"my_combo_box_basic":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","validationBehavior":"aria","label":"Favorite Animal","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"red panda"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"cat"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"dog"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"aardvark"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"kangaroo"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"snake"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"ant"}}]}}},"__dhElemName":"__main__.ui_combo_box_basic"},"state":"{}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json b/plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json new file mode 100644 index 000000000..0c43e13ce --- /dev/null +++ b/plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json @@ -0,0 +1 @@ +{"file":"components/combo_box.md","objects":{"my_combo_box_control_example":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":[{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","validationBehavior":"aria","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"onInputChange":{"__dhCbid":"cb1"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}}]},"__dhElemName":"__main__.ui_combo_box_control_example"},"state":"{\"state\": {\"0\": \"\"}}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/87f46b40b82cb2efc19f5bdbab3ac2a3.json b/plugins/ui/docs/snapshots/87f46b40b82cb2efc19f5bdbab3ac2a3.json new file mode 100644 index 000000000..b409f9fe6 --- /dev/null +++ b/plugins/ui/docs/snapshots/87f46b40b82cb2efc19f5bdbab3ac2a3.json @@ -0,0 +1 @@ +{"file":"components/combo_box.md","objects":{"my_combo_box_multi_select_example":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":{"__dhElemName":"deephaven.ui.components.MultiSelect","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","selectedKeys":[],"validationBehavior":"aria","label":"Pick options","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}}},"__dhElemName":"__main__.ui_combo_box_multi_select_example"},"state":"{}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/94ab46014a1ec4241a826e7b08b163ef.json b/plugins/ui/docs/snapshots/94ab46014a1ec4241a826e7b08b163ef.json deleted file mode 100644 index cc1802ac2..000000000 --- a/plugins/ui/docs/snapshots/94ab46014a1ec4241a826e7b08b163ef.json +++ /dev/null @@ -1 +0,0 @@ -{"file":"components/combo_box.md","objects":{"my_combo_box_is_read_only_example":{"type":"deephaven.ui.Element","data":{"document":{"props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","defaultSelectedKey":"Option 1","isReadOnly":true,"validationBehavior":"aria","labelPosition":"top","children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 1","children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 2","children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 3","children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 4","children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 5","children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 6","children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 7","children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 8","children":"Option 8"}}]},"__dhElemName":"deephaven.ui.components.ComboBox"},"state":"{}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/a3924153476e117f57e563dacb3b31d5.json b/plugins/ui/docs/snapshots/a3924153476e117f57e563dacb3b31d5.json new file mode 100644 index 000000000..c58bff1d2 --- /dev/null +++ b/plugins/ui/docs/snapshots/a3924153476e117f57e563dacb3b31d5.json @@ -0,0 +1 @@ +{"file":"components/combo_box.md","objects":{"my_combo_box_is_read_only_example":{"type":"deephaven.ui.Element","data":{"document":{"props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","isReadOnly":true,"validationBehavior":"aria","labelPosition":"top","children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 1","children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 2","children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 3","children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 4","children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 5","children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 6","children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 7","children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"key":"Option 8","children":"Option 8"}}]},"__dhElemName":"deephaven.ui.components.ComboBox"},"state":"{}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/ec0ed3d95653ca3d2062db0a5ffcbc68.json b/plugins/ui/docs/snapshots/ec0ed3d95653ca3d2062db0a5ffcbc68.json deleted file mode 100644 index bffbf7716..000000000 --- a/plugins/ui/docs/snapshots/ec0ed3d95653ca3d2062db0a5ffcbc68.json +++ /dev/null @@ -1 +0,0 @@ -{"file":"components/combo_box.md","objects":{"my_combo_box_multi_select_example":{"type":"deephaven.ui.Element","data":{"document":{"__dhElemName":"__main__.ui_combo_box_multi_select_example","props":{"children":[{"__dhElemName":"deephaven.ui.components.Flex","props":{"alignItems":"start","gap":"size-100","flex":"auto","children":{"__dhElemName":"deephaven.ui.components.Flex","props":{"direction":"row","alignItems":"center","gap":"size-100","flex":"auto","children":[{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","inputValue":"","selectedKey":"","validationBehavior":"aria","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"onInputChange":{"__dhCbid":"cb1"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}},{"__dhElemName":"deephaven.ui.components.TagGroup","props":{"labelPosition":"top","labelAlign":"start","onRemove":{"__dhCbid":"cb2"}}}]}}}}]}},"state":"{\"state\": {\"0\": \"\", \"1\": \"\"}}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/f1891462b585b31dabf2e7d96395c968.json b/plugins/ui/docs/snapshots/f1891462b585b31dabf2e7d96395c968.json deleted file mode 100644 index 861e7edad..000000000 --- a/plugins/ui/docs/snapshots/f1891462b585b31dabf2e7d96395c968.json +++ /dev/null @@ -1 +0,0 @@ -{"file":"components/combo_box.md","objects":{"my_combo_box_control_example":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":[{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","inputValue":"","selectedKey":"","validationBehavior":"aria","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"onInputChange":{"__dhCbid":"cb1"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}}]},"__dhElemName":"__main__.ui_combo_box_control_example"},"state":"{\"state\": {\"0\": \"\", \"1\": \"\"}}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/fa4c431cba8e02a50fb02b9cbf6dd6de.json b/plugins/ui/docs/snapshots/fa4c431cba8e02a50fb02b9cbf6dd6de.json deleted file mode 100644 index 626eb4f37..000000000 --- a/plugins/ui/docs/snapshots/fa4c431cba8e02a50fb02b9cbf6dd6de.json +++ /dev/null @@ -1 +0,0 @@ -{"file":"components/combo_box.md","objects":{"my_combo_box_basic":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","selectedKey":"","validationBehavior":"aria","label":"Favorite Animal","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"red panda"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"cat"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"dog"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"aardvark"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"kangaroo"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"snake"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"ant"}}]}}},"__dhElemName":"__main__.ui_combo_box_basic"},"state":"{\"state\": {\"0\": \"\"}}"}}}} \ No newline at end of file From ac827c2f83fb96f453102128dd61a05ae2c10b01 Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Fri, 15 May 2026 10:34:46 -0500 Subject: [PATCH 11/15] fixed comments --- plugins/ui/docs/components/combo_box.md | 19 +++++-- .../src/deephaven/ui/components/combo_box.py | 55 +++++++++++++------ .../ui/test/deephaven/ui/test_combo_box.py | 46 +++++++++++++++- 3 files changed, 96 insertions(+), 24 deletions(-) diff --git a/plugins/ui/docs/components/combo_box.md b/plugins/ui/docs/components/combo_box.md index 9029035a0..a6c3e7bf6 100644 --- a/plugins/ui/docs/components/combo_box.md +++ b/plugins/ui/docs/components/combo_box.md @@ -10,7 +10,7 @@ from deephaven import ui @ui.component def ui_combo_box_basic(): - option, set_option = ui.use_state([]) + option, set_option = ui.use_state([""]) return ui.combo_box( ui.item("red panda"), @@ -266,7 +266,9 @@ Use `selected_keys` or `default_selected_keys` to set the selected option(s). `default_selected_keys` is useful for simpler scenarios where you don't need to control the state externally. `selected_keys` is used for scenarios where the state should be managed by the parent component, providing control and flexibility over the selection of the combo box. -> [!NOTE] > `selected_key` and `default_selected_key` are deprecated. Use `selected_keys` and `default_selected_keys` instead. When the deprecated props are used, `on_selection_change` and `on_change` continue to receive a single key for backwards compatibility. When using the new props, callbacks always receive a list of keys. + +> [!NOTE] +> `selected_key` and `default_selected_key` are deprecated. Use `selected_keys` and `default_selected_keys` instead. When the deprecated props are used, `on_selection_change` and `on_change` continue to receive a single key for backwards compatibility. When using the new props, callbacks always receive a list of keys. ```python from deephaven import ui @@ -355,7 +357,9 @@ Each interaction done in the combo box will trigger its associated event handler Note, this is not the case for selections; when a selection is made, both the `on_change` and `on_input_change` are triggered. -> [!NOTE] > `on_change` and `on_selection_change` receive a `Selection` (list of keys) by default. When the deprecated `selected_key` or `default_selected_key` props are used, callbacks receive a single `Key` instead for backwards compatibility. Eventually the single key props will be removed and callbacks will always receive a list of keys. + +> [!NOTE] +> `on_change` and `on_selection_change` receive a `Selection` (list of keys) by default. When the deprecated `selected_key` or `default_selected_key` props are used, callbacks receive a single `Key` instead for backwards compatibility. Eventually the single key props will be removed and callbacks will always receive a list of keys. ```python from deephaven import ui @@ -364,13 +368,15 @@ from deephaven import ui @ui.component def ui_combo_box_control_example(): input_value, set_input_value = ui.use_state("") - selection_state, set_selection_state = ui.use_state([]) + selection_state, set_selection_state = ui.use_state([""]) def handle_input_change(new_value): + set_selection_state([""]) set_input_value(new_value) print(f"Text changed to {new_value}") def handle_selection_change(new_value): + set_input_value(new_value[0] if new_value else "") set_selection_state(new_value) print(f"Selection changed to {new_value}") @@ -385,10 +391,11 @@ def ui_combo_box_control_example(): ui.item("Option 7"), ui.item("Option 8"), ui.item("Option 9"), + input_value=input_value, on_input_change=handle_input_change, selected_keys=selection_state, on_change=handle_selection_change, - ), + ) ] @@ -776,7 +783,7 @@ from deephaven import ui @ui.component def ui_combo_box_multi_select_example(): - selected, set_selected = ui.use_state([]) + selected, set_selected = ui.use_state([""]) return ui.combo_box( ui.item("Option 1"), diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index f99eb358b..23aa335cc 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -29,7 +29,7 @@ from .item import Item from .item_table_source import ItemTableSource from ..elements import BaseElement, Element, NodeType -from .._internal.utils import create_props, unpack_item_table_source +from .._internal.utils import create_props, unpack_item_table_source, wrap_callable from ..types import Key, Selection, Undefined, UndefinedType from .basic import component_element @@ -57,12 +57,15 @@ "default_selected_keys", } +_SELECTION_CALLBACKS = {"on_selection_change", "on_change"} + def _wrap_callback_as_selection( callback: Callable[..., None] | None, ) -> Callable[..., None] | None: """ Wrap a callback so it always receives a Selection instead of a single Key. + Uses wrap_callable to handle user callbacks with varying argument counts. Args: callback: The callback to wrap. @@ -73,11 +76,13 @@ def _wrap_callback_as_selection( if callback is None: return None + wrapped = wrap_callable(callback) + def wrapper(value: Any) -> None: if isinstance(value, (str, int, float, bool)): - callback([value]) + wrapped([value]) else: - callback(value) + wrapped(value) return wrapper @@ -88,13 +93,15 @@ def _process_selection_props( *, stacklevel: int = 3, ) -> None: - """Process selection-related props: emit deprecation warnings, strip - inapplicable props, and wrap callbacks when needed. + """Process selection-related props: emit deprecation warnings, convert or + strip inapplicable props, and wrap callbacks when needed. - When the deprecated ``selected_key`` / ``default_selected_key`` props are - used, callbacks continue to receive a single ``Key``. When only the new - ``selected_keys`` / ``default_selected_keys`` props are used, callbacks in - single-select mode are wrapped so they always receive a ``Selection``. + In single-select mode with the new selected_keys / default_selected_keys + props, converts them to selected_key / default_selected_key (which the + JS ComboBox understands) and wraps callbacks so they receive a Selection. + + When the deprecated selected_key / default_selected_key props are + used, callbacks continue to receive a single Key. Args: props: Mutable props dict (modified in place). @@ -120,14 +127,28 @@ def _process_selection_props( stacklevel=stacklevel, ) - # strip props that don't apply to the active mode - for prop in _SINGLE_ONLY_PROPS if is_multiple else _MULTI_ONLY_PROPS: - props.pop(prop, None) - - # When not using deprecated key props in single-select mode, wrap - # callbacks so they receive a Selection instead of a single Key. - if not is_multiple and not uses_deprecated: - for cb_name in ("on_selection_change", "on_change"): + if is_multiple: + # Multi-select: strip deprecated single-select props + for prop in _SINGLE_ONLY_PROPS: + props.pop(prop, None) + elif uses_deprecated: + # Single-select with deprecated props: strip the new multi props + for prop in _MULTI_ONLY_PROPS: + props.pop(prop, None) + else: + # Single-select but using new multi props + # Convert to single for ComboBox and wrap callbacks + sel_keys = props.pop("selected_keys", None) + def_sel_keys = props.pop("default_selected_keys", None) + props.pop("selected_key", None) + props.pop("default_selected_key", None) + + if sel_keys is not None: + props["selected_key"] = sel_keys[0] if sel_keys else None + if def_sel_keys is not None: + props["default_selected_key"] = def_sel_keys[0] if def_sel_keys else None + + for cb_name in _SELECTION_CALLBACKS: cb = props.get(cb_name) if cb is not None: props[cb_name] = _wrap_callback_as_selection(cb) diff --git a/plugins/ui/test/deephaven/ui/test_combo_box.py b/plugins/ui/test/deephaven/ui/test_combo_box.py index 03fb4f8b9..5bf40ccc0 100644 --- a/plugins/ui/test/deephaven/ui/test_combo_box.py +++ b/plugins/ui/test/deephaven/ui/test_combo_box.py @@ -17,8 +17,10 @@ def _process(self, props, is_multiple): warnings.simplefilter("always") _process_selection_props(props, is_multiple) - def test_single_mode_strips_multi_props(self): + def test_single_mode_converts_selected_keys_to_selected_key(self): props = { + "selected_key": self.Undefined, + "default_selected_key": None, "selected_keys": ["a", "b"], "default_selected_keys": ["c"], "other": "value", @@ -26,8 +28,46 @@ def test_single_mode_strips_multi_props(self): self._process(props, is_multiple=False) self.assertNotIn("selected_keys", props) self.assertNotIn("default_selected_keys", props) + self.assertEqual(props["selected_key"], "a") + self.assertEqual(props["default_selected_key"], "c") self.assertEqual(props["other"], "value") + def test_single_mode_converts_empty_selected_keys_to_none(self): + props = { + "selected_key": self.Undefined, + "default_selected_key": None, + "selected_keys": [], + "default_selected_keys": [], + } + self._process(props, is_multiple=False) + self.assertIsNone(props["selected_key"]) + self.assertIsNone(props["default_selected_key"]) + + def test_single_mode_no_conversion_when_keys_none(self): + props = { + "selected_key": self.Undefined, + "default_selected_key": None, + "selected_keys": None, + "default_selected_keys": None, + } + self._process(props, is_multiple=False) + self.assertNotIn("selected_key", props) + self.assertNotIn("default_selected_key", props) + self.assertNotIn("selected_keys", props) + self.assertNotIn("default_selected_keys", props) + + def test_single_mode_deprecated_strips_multi_props(self): + props = { + "selected_key": "a", + "default_selected_key": None, + "selected_keys": ["x"], + "default_selected_keys": ["y"], + } + self._process(props, is_multiple=False) + self.assertNotIn("selected_keys", props) + self.assertNotIn("default_selected_keys", props) + self.assertEqual(props["selected_key"], "a") + def test_multiple_mode_strips_single_props(self): props = { "selected_key": self.Undefined, @@ -162,9 +202,13 @@ def test_single_wraps_when_no_deprecated_props(self): props = { "selected_key": Undefined, "default_selected_key": None, + "selected_keys": ["a"], "on_change": handler, } self._process(props, is_multiple=False) + # selected_keys converted to selected_key + self.assertEqual(props["selected_key"], "a") + # callback wrapped props["on_change"]("my_key") self.assertEqual(received, [["my_key"]]) From d4a7edce7f4a8c3c899ec0b24f5cd5b13e701be4 Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Fri, 15 May 2026 12:16:44 -0500 Subject: [PATCH 12/15] a few more comments --- plugins/ui/docs/components/combo_box.md | 10 +++---- .../src/deephaven/ui/components/combo_box.py | 30 ++++++++++++++----- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/plugins/ui/docs/components/combo_box.md b/plugins/ui/docs/components/combo_box.md index a6c3e7bf6..438dbbde6 100644 --- a/plugins/ui/docs/components/combo_box.md +++ b/plugins/ui/docs/components/combo_box.md @@ -10,7 +10,7 @@ from deephaven import ui @ui.component def ui_combo_box_basic(): - option, set_option = ui.use_state([""]) + option, set_option = ui.use_state([]) return ui.combo_box( ui.item("red panda"), @@ -268,7 +268,7 @@ Use `selected_keys` or `default_selected_keys` to set the selected option(s). > [!NOTE] -> `selected_key` and `default_selected_key` are deprecated. Use `selected_keys` and `default_selected_keys` instead. When the deprecated props are used, `on_selection_change` and `on_change` continue to receive a single key for backwards compatibility. When using the new props, callbacks always receive a list of keys. + > `selected_key` and `default_selected_key` are deprecated. Use `selected_keys` and `default_selected_keys` instead. When the deprecated props are used, `on_selection_change` and `on_change` continue to receive a single key for backwards compatibility. When using the new props, callbacks generally receive a list of keys (unless `None` in the case of `on_change`). ```python from deephaven import ui @@ -359,7 +359,7 @@ Note, this is not the case for selections; when a selection is made, both the `o > [!NOTE] -> `on_change` and `on_selection_change` receive a `Selection` (list of keys) by default. When the deprecated `selected_key` or `default_selected_key` props are used, callbacks receive a single `Key` instead for backwards compatibility. Eventually the single key props will be removed and callbacks will always receive a list of keys. + > `selected_key` and `default_selected_key` are deprecated. Use `selected_keys` and `default_selected_keys` instead. When the deprecated props are used, `on_selection_change` and `on_change` continue to receive a single key for backwards compatibility. When using the new props, callbacks generally receive a list of keys (unless `None` in the case of `on_change`). ```python from deephaven import ui @@ -368,7 +368,7 @@ from deephaven import ui @ui.component def ui_combo_box_control_example(): input_value, set_input_value = ui.use_state("") - selection_state, set_selection_state = ui.use_state([""]) + selection_state, set_selection_state = ui.use_state([]) def handle_input_change(new_value): set_selection_state([""]) @@ -783,7 +783,7 @@ from deephaven import ui @ui.component def ui_combo_box_multi_select_example(): - selected, set_selected = ui.use_state([""]) + selected, set_selected = ui.use_state([]) return ui.combo_box( ui.item("Option 1"), diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index 23aa335cc..f91818a0a 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -62,6 +62,7 @@ def _wrap_callback_as_selection( callback: Callable[..., None] | None, + callback_name: str | None = None, ) -> Callable[..., None] | None: """ Wrap a callback so it always receives a Selection instead of a single Key. @@ -81,6 +82,9 @@ def _wrap_callback_as_selection( def wrapper(value: Any) -> None: if isinstance(value, (str, int, float, bool)): wrapped([value]) + elif value is None and callback_name is "on_change": + # on_change with None means no selection + wrapped([]) else: wrapped(value) @@ -108,9 +112,9 @@ def _process_selection_props( is_multiple: Whether multi-select mode is active. stacklevel: Stack level passed to warnings.warn to point to the caller's code. """ - uses_deprecated = ( - props.get("selected_key") is not Undefined - or props.get("default_selected_key") is not None + uses_keys = ( + props.get("selected_keys") is not Undefined + or props.get("default_selected_keys") is not None ) # warn about deprecated single-select props if they are set @@ -131,8 +135,8 @@ def _process_selection_props( # Multi-select: strip deprecated single-select props for prop in _SINGLE_ONLY_PROPS: props.pop(prop, None) - elif uses_deprecated: - # Single-select with deprecated props: strip the new multi props + elif not uses_keys: + # Doesn't use multi props: strip them for prop in _MULTI_ONLY_PROPS: props.pop(prop, None) else: @@ -144,14 +148,24 @@ def _process_selection_props( props.pop("default_selected_key", None) if sel_keys is not None: + if not isinstance(sel_keys, list) or len(sel_keys) > 1: + warnings.warn( + f"'selected_keys' should be a list with at most one key when 'selection_mode' is 'single'. Got: {sel_keys}", + stacklevel=3, + ) props["selected_key"] = sel_keys[0] if sel_keys else None if def_sel_keys is not None: + if not isinstance(def_sel_keys, list) or len(def_sel_keys) > 1: + warnings.warn( + f"'default_selected_keys' should be a list with at most one key when 'selection_mode' is 'single'. Got: {def_sel_keys}", + stacklevel=3, + ) props["default_selected_key"] = def_sel_keys[0] if def_sel_keys else None for cb_name in _SELECTION_CALLBACKS: cb = props.get(cb_name) if cb is not None: - props[cb_name] = _wrap_callback_as_selection(cb) + props[cb_name] = _wrap_callback_as_selection(cb, cb_name) def combo_box( @@ -189,8 +203,8 @@ def combo_box( necessity_indicator: NecessityIndicator | None = None, contextual_help: Element | None = None, on_open_change: Callable[[bool, MenuTriggerAction], None] | None = None, - on_selection_change: Callable[[Selection | None], None] | None = None, - on_change: Callable[[Selection], None] | None = None, + on_selection_change: Callable[[Key | Selection | None], None] | None = None, + on_change: Callable[[Key | Selection], None] | None = None, on_input_change: Callable[[str], None] | None = None, on_focus: Callable[[FocusEventCallable], None] | None = None, on_blur: Callable[[FocusEventCallable], None] | None = None, From 50db8889c877f4999fa1dca89172b5c18d0fd679 Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Fri, 15 May 2026 12:45:41 -0500 Subject: [PATCH 13/15] snapshots --- plugins/ui/docs/snapshots/0cf5572d36b4c8bdf11d666c2afd41c5.json | 1 + plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 plugins/ui/docs/snapshots/0cf5572d36b4c8bdf11d666c2afd41c5.json delete mode 100644 plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json diff --git a/plugins/ui/docs/snapshots/0cf5572d36b4c8bdf11d666c2afd41c5.json b/plugins/ui/docs/snapshots/0cf5572d36b4c8bdf11d666c2afd41c5.json new file mode 100644 index 000000000..f350b5108 --- /dev/null +++ b/plugins/ui/docs/snapshots/0cf5572d36b4c8bdf11d666c2afd41c5.json @@ -0,0 +1 @@ +{"file":"components/combo_box.md","objects":{"my_combo_box_control_example":{"type":"deephaven.ui.Element","data":{"document":{"__dhElemName":"__main__.ui_combo_box_control_example","props":{"children":[{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","inputValue":"","validationBehavior":"aria","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"onInputChange":{"__dhCbid":"cb1"},"selectedKey":null,"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}}]}},"state":"{\"state\": {\"0\": \"\"}}"}}}} \ No newline at end of file diff --git a/plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json b/plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json deleted file mode 100644 index 0c43e13ce..000000000 --- a/plugins/ui/docs/snapshots/639e179ae0580408f236edba3dd1d3c3.json +++ /dev/null @@ -1 +0,0 @@ -{"file":"components/combo_box.md","objects":{"my_combo_box_control_example":{"type":"deephaven.ui.Element","data":{"document":{"props":{"children":[{"__dhElemName":"deephaven.ui.components.ComboBox","props":{"menuTrigger":"input","align":"end","direction":"bottom","shouldFlip":true,"formValue":"text","validationBehavior":"aria","labelPosition":"top","onChange":{"__dhCbid":"cb0"},"onInputChange":{"__dhCbid":"cb1"},"children":[{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 1"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 2"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 3"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 4"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 5"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 6"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 7"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 8"}},{"__dhElemName":"deephaven.ui.components.Item","props":{"children":"Option 9"}}]}}]},"__dhElemName":"__main__.ui_combo_box_control_example"},"state":"{\"state\": {\"0\": \"\"}}"}}}} \ No newline at end of file From 933b76022f949e8dfd38fda9f8e0f80325f16b51 Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Mon, 18 May 2026 10:57:25 -0500 Subject: [PATCH 14/15] comments --- .../src/deephaven/ui/components/combo_box.py | 138 ++++++++++-------- .../ui/test/deephaven/ui/test_combo_box.py | 40 +++-- 2 files changed, 107 insertions(+), 71 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index f91818a0a..40a6c409f 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -1,7 +1,7 @@ from __future__ import annotations import warnings -from typing import Callable, Any, Literal +from typing import Callable, Any, Literal, Sequence from .types import ( FocusEventCallable, @@ -65,8 +65,7 @@ def _wrap_callback_as_selection( callback_name: str | None = None, ) -> Callable[..., None] | None: """ - Wrap a callback so it always receives a Selection instead of a single Key. - Uses wrap_callable to handle user callbacks with varying argument counts. + Wrap a callback so it always receives a Selection instead of (possibly) a single Key. Args: callback: The callback to wrap. @@ -82,8 +81,8 @@ def _wrap_callback_as_selection( def wrapper(value: Any) -> None: if isinstance(value, (str, int, float, bool)): wrapped([value]) - elif value is None and callback_name is "on_change": - # on_change with None means no selection + elif value is None and callback_name == "on_change": + # on_change with None means an empty list to be consistent with the typing wrapped([]) else: wrapped(value) @@ -91,77 +90,98 @@ def wrapper(value: Any) -> None: return wrapper +def _convert_selection_prop( + props: dict[str, Any], + multi_prop: str, + single_prop: str, + is_multiple: bool, + default_val: Any, +) -> bool: + """ + Convert between single and multi select props based on the selection mode, emitting warnings as needed. + + Args: + props: The props dict to modify in place. + multi_prop: The name of the multi-select prop (e.g. "selected_keys"). + single_prop: The name of the single-select prop (e.g. "selected_key"). + is_multiple: Whether multi-select mode is active. + default_val: The default value to use (for the single prop only). + + Returns: + True if callbacks should always return a Key + """ + multi_val = props.pop(multi_prop) + single_val = props.pop(single_prop) + + if single_val is not default_val: + if is_multiple: + # Throw an error if the user is trying to use the single prop in multi-select mode since it shouldn't work + raise ValueError( + f"'{single_prop}' cannot be used when 'selection_mode' is 'multiple'. Use '{multi_prop}' instead." + ) + # Otherwise use the single prop value + # Warn and don't convert callbacks since the user is using the deprecated single prop which expects a single Key + warnings.warn( + f"'{single_prop}' is deprecated. Use '{multi_prop}' instead.", + FutureWarning, + stacklevel=2, + ) + props[single_prop] = single_val + return True + + if is_multiple: + # In multi-select mode, multi_prop is expected + props[multi_prop] = multi_val + return False + + if multi_val is not None: + # multi_prop is provided in single-select mode, so we need to convert it to the single prop + if not isinstance(multi_val, list) or len(multi_val) > 1: + warnings.warn( + f"'{multi_prop}' should be a list with at most one key when 'selection_mode' is 'single'. Got: {multi_val}", + stacklevel=2, + ) + # Use the single prop for the ComboBox + props[single_prop] = multi_val[0] if multi_val else None + # In single-select mode but using the multi prop, so the callbacks receive a Selection + return False + + # No value provided for either prop, so keep the callback as is, expecting a Selection + # This is technically an ambiguous case and may conflict with deprecated usage + # but we have no way to know if the user intends to use the Key or Selection callbacks + # without a hacky check of some sort. + props[single_prop] = single_val + return False + + def _process_selection_props( props: dict[str, Any], is_multiple: bool, - *, - stacklevel: int = 3, ) -> None: - """Process selection-related props: emit deprecation warnings, convert or + """ + Process selection-related props: emit deprecation warnings, convert or strip inapplicable props, and wrap callbacks when needed. In single-select mode with the new selected_keys / default_selected_keys props, converts them to selected_key / default_selected_key (which the - JS ComboBox understands) and wraps callbacks so they receive a Selection. + ComboBox understands) and wraps callbacks so they receive a Selection. When the deprecated selected_key / default_selected_key props are used, callbacks continue to receive a single Key. Args: - props: Mutable props dict (modified in place). + props: Mutable props dict is_multiple: Whether multi-select mode is active. - stacklevel: Stack level passed to warnings.warn to point to the caller's code. """ - uses_keys = ( - props.get("selected_keys") is not Undefined - or props.get("default_selected_keys") is not None + selected_takes_key = _convert_selection_prop( + props, "selected_keys", "selected_key", is_multiple, Undefined + ) + default_takes_key = _convert_selection_prop( + props, "default_selected_keys", "default_selected_key", is_multiple, None ) - # warn about deprecated single-select props if they are set - if props.get("selected_key") is not Undefined: - warnings.warn( - "'selected_key' is deprecated. Use 'selected_keys' instead.", - DeprecationWarning, - stacklevel=stacklevel, - ) - if props.get("default_selected_key") is not None: - warnings.warn( - "'default_selected_key' is deprecated. Use 'default_selected_keys' instead.", - DeprecationWarning, - stacklevel=stacklevel, - ) - - if is_multiple: - # Multi-select: strip deprecated single-select props - for prop in _SINGLE_ONLY_PROPS: - props.pop(prop, None) - elif not uses_keys: - # Doesn't use multi props: strip them - for prop in _MULTI_ONLY_PROPS: - props.pop(prop, None) - else: - # Single-select but using new multi props - # Convert to single for ComboBox and wrap callbacks - sel_keys = props.pop("selected_keys", None) - def_sel_keys = props.pop("default_selected_keys", None) - props.pop("selected_key", None) - props.pop("default_selected_key", None) - - if sel_keys is not None: - if not isinstance(sel_keys, list) or len(sel_keys) > 1: - warnings.warn( - f"'selected_keys' should be a list with at most one key when 'selection_mode' is 'single'. Got: {sel_keys}", - stacklevel=3, - ) - props["selected_key"] = sel_keys[0] if sel_keys else None - if def_sel_keys is not None: - if not isinstance(def_sel_keys, list) or len(def_sel_keys) > 1: - warnings.warn( - f"'default_selected_keys' should be a list with at most one key when 'selection_mode' is 'single'. Got: {def_sel_keys}", - stacklevel=3, - ) - props["default_selected_key"] = def_sel_keys[0] if def_sel_keys else None - + if not (selected_takes_key or default_takes_key): + # We aren't in the deprecated single prop case, so we need to convert callbacks to always receive a Selection for cb_name in _SELECTION_CALLBACKS: cb = props.get(cb_name) if cb is not None: diff --git a/plugins/ui/test/deephaven/ui/test_combo_box.py b/plugins/ui/test/deephaven/ui/test_combo_box.py index 5bf40ccc0..9336350e2 100644 --- a/plugins/ui/test/deephaven/ui/test_combo_box.py +++ b/plugins/ui/test/deephaven/ui/test_combo_box.py @@ -51,8 +51,9 @@ def test_single_mode_no_conversion_when_keys_none(self): "default_selected_keys": None, } self._process(props, is_multiple=False) - self.assertNotIn("selected_key", props) - self.assertNotIn("default_selected_key", props) + # When selected_keys is None, falls through and sets single prop to its default + self.assertEqual(props["selected_key"], self.Undefined) + self.assertEqual(props["default_selected_key"], None) self.assertNotIn("selected_keys", props) self.assertNotIn("default_selected_keys", props) @@ -72,6 +73,8 @@ def test_multiple_mode_strips_single_props(self): props = { "selected_key": self.Undefined, "default_selected_key": None, + "selected_keys": None, + "default_selected_keys": None, "other": "value", } self._process(props, is_multiple=True) @@ -79,14 +82,15 @@ def test_multiple_mode_strips_single_props(self): self.assertNotIn("default_selected_key", props) self.assertEqual(props["other"], "value") - def test_multiple_mode_strips_set_single_props(self): + def test_multiple_mode_raises_when_single_props_set(self): props = { "selected_key": "some_key", "default_selected_key": "other", + "selected_keys": None, + "default_selected_keys": None, } - self._process(props, is_multiple=True) - self.assertNotIn("selected_key", props) - self.assertNotIn("default_selected_key", props) + with self.assertRaises(ValueError): + self._process(props, is_multiple=True) class ComboBoxWrapCallbackTest(BaseTestCase): @@ -142,7 +146,7 @@ def test_selected_key_warns(self): with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") combo_box(selected_key="a") - dep_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)] + dep_warnings = [x for x in w if issubclass(x.category, FutureWarning)] messages = [str(x.message) for x in dep_warnings] self.assertTrue( any("selected_key" in m and "selected_keys" in m for m in messages), @@ -155,7 +159,7 @@ def test_default_selected_key_warns(self): with warnings.catch_warnings(record=True) as w: warnings.simplefilter("always") combo_box(default_selected_key="a") - dep_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)] + dep_warnings = [x for x in w if issubclass(x.category, FutureWarning)] messages = [str(x.message) for x in dep_warnings] self.assertTrue( any( @@ -174,7 +178,7 @@ def test_no_warning_when_defaults(self): dep_warnings = [ x for x in w - if issubclass(x.category, DeprecationWarning) + if issubclass(x.category, FutureWarning) and ("selected_key" in str(x.message)) ] self.assertEqual( @@ -203,6 +207,7 @@ def test_single_wraps_when_no_deprecated_props(self): "selected_key": Undefined, "default_selected_key": None, "selected_keys": ["a"], + "default_selected_keys": None, "on_change": handler, } self._process(props, is_multiple=False) @@ -218,6 +223,8 @@ def test_single_no_wrap_when_selected_key_used(self): props = { "selected_key": "a", "default_selected_key": None, + "selected_keys": None, + "default_selected_keys": None, "on_change": handler, } self._process(props, is_multiple=False) @@ -232,22 +239,31 @@ def test_single_no_wrap_when_default_selected_key_used(self): props = { "selected_key": Undefined, "default_selected_key": "b", + "selected_keys": None, + "default_selected_keys": None, "on_change": handler, } self._process(props, is_multiple=False) props["on_change"]("my_key") self.assertEqual(received, ["my_key"]) - def test_multiple_no_wrap_regardless(self): + def test_multiple_wraps_callbacks(self): + from deephaven.ui.types import Undefined + received = [] handler = lambda v: received.append(v) props = { - "selected_key": "x", + "selected_key": Undefined, "default_selected_key": None, + "selected_keys": ["x"], + "default_selected_keys": None, "on_change": handler, } self._process(props, is_multiple=True) - # single props are stripped, callback untouched + # single props are stripped + self.assertNotIn("selected_key", props) + self.assertNotIn("default_selected_key", props) + # callback wrapped but lists pass through unchanged props["on_change"](["a", "b"]) self.assertEqual(received, [["a", "b"]]) From db4871079ccdaa0adcb50f32c50e683ff63ba042 Mon Sep 17 00:00:00 2001 From: Joe Numainville Date: Mon, 18 May 2026 13:15:56 -0500 Subject: [PATCH 15/15] better warns --- .../ui/src/deephaven/ui/components/combo_box.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/plugins/ui/src/deephaven/ui/components/combo_box.py b/plugins/ui/src/deephaven/ui/components/combo_box.py index 40a6c409f..c4b46a079 100644 --- a/plugins/ui/src/deephaven/ui/components/combo_box.py +++ b/plugins/ui/src/deephaven/ui/components/combo_box.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging import warnings from typing import Callable, Any, Literal, Sequence @@ -33,6 +34,9 @@ from ..types import Key, Selection, Undefined, UndefinedType from .basic import component_element + +logger = logging.getLogger(__name__) + ComboBoxElement = BaseElement SUPPORTED_SOURCE_ARGS = { @@ -136,9 +140,13 @@ def _convert_selection_prop( if multi_val is not None: # multi_prop is provided in single-select mode, so we need to convert it to the single prop - if not isinstance(multi_val, list) or len(multi_val) > 1: - warnings.warn( - f"'{multi_prop}' should be a list with at most one key when 'selection_mode' is 'single'. Got: {multi_val}", + if not isinstance(multi_val, Sequence) or isinstance(multi_val, str): + raise ValueError( + f"'{multi_prop}' should be a Sequence when 'selection_mode' is 'single'. Got type: {type(multi_val)}" + ) + if len(multi_val) > 1: + logger.warning( + f"'{multi_prop}' should be a Sequence with at most one key when 'selection_mode' is 'single'. Got: {multi_val}. Only the first value will be used.", stacklevel=2, ) # Use the single prop for the ComboBox