feat: ComboBox - @deephaven/components

[bmingles]

• New ComboBox component in @deephaven/components
• Split out some shared logic from Picker since ComboBox is basically a subclass
• Deleted old ComboBox component and replaced usage (condition column + row formatting. Also tested to make sure it still works as before)
• Updated Styleguide

resolves #2065

BREAKING CHANGE: ComboBox component has been replaced.
To migrate to new version:

• Passing children is used instead of options prop to define dropdown items. For cases where option value and display are the same, passing an array of values as children will work. For cases where value and display differ, Item elements must be passed as children. e.g. <Item key={value}>{display}</Item>
  e.g.

// values will be used for display + value
const items = useMemo(
  () => ['Aaa', 'Bbb', 'Ccc'], 
  []
)
<ComboBox>{items}</ComboBox>

<ComboBox>
  <Item key="aaa">Aaa</Item>
  <Item key="bbb">Bbb</Item>
  <Item key="ccc">Ccc</Item>
</ComboBox>

• The spellcheck=false prop is no longer supported or needed
• searchPlaceholder and inputPlaceholder props are no longer supported and should be omitted. There is an optional description prop for cases where a descriptive label is desired. There is also a label prop for the primary component label.

[codecov]

Codecov Report

Attention: Patch coverage is 68.33333% with 19 lines in your changes missing coverage. Please review.

Project coverage is 46.61%. Comparing base (c804b15) to head (ea02fa9).
Report is 2 commits behind head on main.

❗ Current head ea02fa9 differs from pull request most recent head a19c4a7

Please upload reports for the commit a19c4a7 to get more accurate results.

Files	Patch %	Lines
...s/components/src/spectrum/picker/usePickerProps.ts	0.00%	13 Missing ⚠️
...ages/components/src/spectrum/comboBox/ComboBox.tsx	0.00%	3 Missing ⚠️
packages/components/src/spectrum/picker/Picker.tsx	0.00%	1 Missing ⚠️
...ebar/conditional-formatting/ColumnFormatEditor.tsx	0.00%	1 Missing ⚠️
...sidebar/conditional-formatting/RowFormatEditor.tsx	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2067      +/-   ##
==========================================
+ Coverage   46.45%   46.61%   +0.16%     
==========================================
  Files         673      677       +4     
  Lines       38734    38556     -178     
  Branches     9815     9772      -43     
==========================================
- Hits        17994    17974      -20     
+ Misses      20688    20530     -158     
  Partials       52       52              

Flag	Coverage Δ
unit	46.61% <68.33%> (+0.16%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mofojed]

Looks good, just needs to be labelled a breaking change and then migration support should be a little more detailed (particularly around placeholder)

[mofojed]

This change should be marked BREAKING, with migration instructions to transition to the new ComboBox.
Pretty much just mapping the options to children and wrapping them in Item? Would be good if you had an example.
I notice we don't have a spellCheck prop anymore (seems to be false by default, which is probably what we want anyway), and no more placeholder props (Spectrum doesn't seem to like them) - what should we do when migrating those props?

[mofojed]

I think we still want to template the props here so items get passed through correctly no?
Something like:

Suggested change

	export type ComboBoxProps = PickerPropsT<SpectrumComboBoxProps<NormalizedItem>>;
	export type ComboBoxProps<T extends NormalizedItem = NormalizedItem> = PickerPropsT<SpectrumComboBoxProps<T>>;

[bmingles]

What's the use case? SpectrumComboBoxProps<NormalizedItem> should already pass things through. We aren't extending NormalizedItem so I think it should be ok?

[mofojed]

I guess you're right - I was thinking if you pass in items that extended items and wanted the original item back, but that's not how this works anyway.

[mofojed]

See in these demos we have no placeholder anymore. We should have instructions how to deal with that (default to the desired value instead? Display a validation error if blank with an appropriate warning?) and show those cases in the styleguide.