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

[jnumainville]

Add "multiple" selection_mode option for combo_box. Depends on deephaven/web-client-ui#2685, so won't pass or merge until that is merged.

BREAKING CHANGE: The selected_key and default_selected_key props are deprecated. Due to this, the default value of the argument passed into the on_change and on_selection_change callbacks is a Selection instead of a Key. If selected_key or default_selected_key are passed in to combo_box the callbacks will still pass in a Key until the deprecated props are removed, but in all other cases the callbacks will pass in a Selection.

[github-actions]

ui docs preview (Available for 14 days)

[margaretkennedy]

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

[jnumainville]

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

[mofojed]

@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:

1. 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.
2. 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.
3. 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.
4. 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]

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

[mofojed]

@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]

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.