feat: DH-19683: multi-select combo box component in deephaven.ui by jnumainville · Pull Request #1349 · deephaven/deephaven-plugins

jnumainville · 2026-05-19T21:35:09Z

Add new multi_select component. Depends on deephaven/web-client-ui#2685, so won't pass or merge until that is merged.

github-actions · 2026-05-19T21:39:00Z

ui docs preview (Available for 14 days)

margaretkennedy · 2026-05-20T12:25:21Z

-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.
+
+<!-- prettier-ignore -->


not sure why we need the prettier ignore here? is formatting messing with the note?

Yes. It keeps collapsing any > [!NOTE] into one line on save.

mofojed · 2026-05-20T13:38:35Z

@jnumainville As mentioned in our 1:1, I don't like how this is currently a breaking change. Looping @dsmmcken for some input.
Basically - we are trying to mimick ListView, which has selectionMode single or multiple, but for the onSelectionChange callback it always takes a set of keys for the parameter, regardless of if you have single or multiple selection. selectedKeys and defaultSelectedKeys also are always a set that's passed in, not a single value.
In combo_box currently, we have the selected_key, default_selected_key, on_selection_change and then on_change (which is just an alias of on_selection_change) which all take a single value (not an array or set). Now with the addition of a selection_mode, we're trying to add selected_keys and default_selected_keys, deprecating selected_key and default_selected_key... but then the on_change needs to break to return a set instead of a single value. I think we have a few options:

Have callbacks return a single value or a set based on the selection_mode parameter. I don't like this because it makes the typing more complicated, and I reckon users and agents will screw this up.
Use a different callback for multiple values, e.g. on_keys_changed. I think again this is confusing and people/agents will screw this up.
Make this a breaking change. I don't like this because it would break existing usages of combo_box. If we go this route, I'd also just remove selected_key and default_selected_key entirely so the breakage is more apparent. I don't think we should do this, as it annoy users.
Just use a different component (multi_select, or tag_field, or something). This is my favourite option, as it doesn't break the existing API, and then the API for the new component is well defined and easy to get right (for both users and agents).

Thoughts?

dsmmcken · 2026-05-20T13:41:07Z

My preference is 1, but If you've given it thought then I back your conclusion of 4.

mofojed · 2026-05-20T14:03:34Z

@jnumainville I think you concluded 4 would be more elegant as well. Unless you make a case for 1, I'd like to go with 4. multi_select or tag_field both seem like appropriate names.
We can add a note in combo_box that if you want multiple selection, use multi_select.

Also, how big of tables have you tested this with?

jnumainville · 2026-05-20T14:46:03Z

I prefer 4 as well. The implementation here is unpleasant, specifically the deprecation logic, and I think it's a case where if the deprecation logic is tricky to implement it's tricky to follow (when do I have aKey? when do I have a Selection?). Even if we did accept the pains of full deprecation, it also feels like going from a Key to a Selection is a downgrade for the single select case, even if logically consistent with list_view. Even in comparison to list_view this new component is quite a bit more overhead than just allowing multiple selections for checkboxes that are already there.
I'll rework with multi_select as the name unless there is strong disgreement, I just think that is a more intuitive name.
I've only tested with smaller tables (like distinct stocks Sym) but will try bigger ones after the rework.

mofojed

A few updates to the docs, a few comments. Also we need to add an e2e test (probably should have one for combo box as well).

mofojed · 2026-05-22T14:03:48Z

+from deephaven import ui
+
+
+@ui.component
+def ui_multi_select_form_examples():
+    return [
+        ui.form(
+            ui.multi_select(
+                ui.item("Chocolate"),
+                ui.item("Mint"),
+                ui.item("Vanilla"),
+                ui.item("Strawberry"),
+                ui.item("Cookies and Cream"),
+                ui.item("Coffee"),
+                ui.item("Mango"),
+                label="Ice cream flavors",
+                name="flavors",
+            ),
+            ui.button("Submit", type="submit"),
+            on_submit=lambda event: print(event),
+        )
+    ]
+
+
+my_multi_select_form_examples = ui_multi_select_form_examples()


This is just one example, don't need the array or the pluralization

Suggested change

from deephaven import ui

@ui.component

def ui_multi_select_form_examples():

return [

ui.form(

ui.multi_select(

ui.item("Chocolate"),

ui.item("Mint"),

ui.item("Vanilla"),

ui.item("Strawberry"),

ui.item("Cookies and Cream"),

ui.item("Coffee"),

ui.item("Mango"),

label="Ice cream flavors",

name="flavors",

),

ui.button("Submit", type="submit"),

on_submit=lambda event: print(event),

)

]

my_multi_select_form_examples = ui_multi_select_form_examples()

from deephaven import ui

@ui.component

def ui_multi_select_form_example():

return ui.form(

ui.multi_select(

ui.item("Chocolate"),

ui.item("Mint"),

ui.item("Vanilla"),

ui.item("Strawberry"),

ui.item("Cookies and Cream"),

ui.item("Coffee"),

ui.item("Mango"),

label="Ice cream flavors",

name="flavors",

),

ui.button("Submit", type="submit"),

on_submit=lambda event: print(event),

)

my_multi_select_form_example = ui_multi_select_form_example()

mofojed · 2026-05-22T14:05:37Z

+    return [
+        ui.multi_select(
+            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"),
+            input_value=input_value,
+            on_input_change=handle_input_change,
+            selected_keys=selection_state,
+            on_change=handle_selection_change,
+            label="Pick options",
+        )
+    ]


Suggested change

return [

ui.multi_select(

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"),

input_value=input_value,

on_input_change=handle_input_change,

selected_keys=selection_state,

on_change=handle_selection_change,

label="Pick options",

)

]

return ui.multi_select(

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"),

input_value=input_value,

on_input_change=handle_input_change,

selected_keys=selection_state,

on_change=handle_selection_change,

label="Pick options",

)

mofojed · 2026-05-22T14:07:37Z

+      return (
+        // eslint-disable-next-line react/jsx-props-no-spreading
+        <DHMultiSelect loadingState="loading" {...multiSelectProps}>
+          {[]}


What's going on with these children here? At least use EMPTY_ARRAY, but is it required to specify children if we're in a loading/error state?

This appears to be adapted directly from Combobox (and ListView and Picker)

Here's some more context (AI traced but I did double check):
Yes, the empty arrays are necessary. The DHComboBox component from @deephaven/components has children as a required prop (typed as ItemOrSection | ItemOrSection[]). Since TypeScript would error if you omitted it, the empty array {[]} satisfies the type requirement while rendering no items — appropriate for the loading and error states where there's no real data to display.

Then I asked for more details:
The children prop flows into wrapItemChildren() which calls ensureArray(children). That utility treats non-array values by wrapping them in an array — so undefined becomes [undefined]. Then .map() processes each element, and since undefined isn't an Item or Section element, it falls through to the primitive branch which does String(undefined) → "undefined" and creates an <Item key="undefined" textValue="undefined">.
So passing undefined wouldn't crash, but it would silently render a phantom item labeled "undefined" in the dropdown. The empty array [] produces an empty .map() result and correctly renders nothing.

Here's the link to that function:
https://github.com/deephaven/web-client-ui/blob/211519a91f7bbdf4e8dee812a1061a989447d0a2/packages/components/src/spectrum/utils/itemWrapperUtils.tsx#L56

I did switch to EMPTY_ARRAY though

We could fix this in itemWrapperUtils.tsx, it's in our code. May as well add it to deephaven/web-client-ui#2685 ?

jnumainville · 2026-05-22T17:07:54Z

Waiting on e2e tests until I have deephaven/web-client-ui#2685 merged and released

Add multi-select combobox component. Tested with (and required for) deephaven/deephaven-plugins#1349 --------- Co-authored-by: gzh2003 <germainzhanghoule@gmail.com> Co-authored-by: Mike Bender <mikebender@deephaven.io>

mofojed

Looks good but we'll need a web-client-ui release first

gzh2003 and others added 15 commits May 6, 2026 09:31

multiselect plugin wrapper

9c9c886

fix docs error

db5a635

Issue with raise entries in autodoc

086748b

should be using hand rolled props

83107fe

fix round trip issue

23417ed

deprecation

343d651

fixed

665b178

refactor and tests

29ac4d2

fixing issues

ce954fa

fixed screenshots

0498ace

fixed comments

ac827c2

a few more comments

d4a7edc

snapshots

50db888

comments

933b760

better warns

db48710

jnumainville requested a review from mofojed May 19, 2026 21:35

github-actions Bot requested a review from margaretkennedy May 19, 2026 21:35

This was referenced May 19, 2026

feat: DH-19683: Multi-select combobox component deephaven/web-client-ui#2685

Merged

feat: DH-19683: multi-select combo box component in deephaven.ui #1347

Closed

margaretkennedy reviewed May 20, 2026

View reviewed changes

split out

574fbb5

mofojed requested changes May 22, 2026

View reviewed changes

comments

b662249

jnumainville requested a review from mofojed May 22, 2026 17:16

work children

90dc770

mofojed approved these changes May 22, 2026

View reviewed changes

Conversation

jnumainville commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnumainville May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mofojed commented May 20, 2026

Uh oh!

dsmmcken commented May 20, 2026

Uh oh!

mofojed commented May 20, 2026

Uh oh!

jnumainville commented May 20, 2026

Uh oh!

mofojed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnumainville May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnumainville commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mofojed left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jnumainville commented May 19, 2026 •

edited

Loading

jnumainville May 20, 2026 •

edited

Loading

jnumainville May 22, 2026 •

edited

Loading

jnumainville commented May 22, 2026 •

edited

Loading