Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the Select component with multi-select functionality and changes the default search behavior to filter by label instead of description, addressing issues #25 and #26.
Changes:
- Added multi-select mode with checkbox UI that allows users to select multiple options
- Changed default search filtering from description field to label field
- Updated type signatures to support both single values and arrays of values
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| packages/baukasten/src/components/Select/Select.tsx | Added multi-select support with checkboxes, updated search to filter by label instead of description, added state management for array values |
| packages/baukasten/src/components/Select/Select.stories.tsx | Added multi-select stories and examples, updated existing stories with type guards for backward compatibility |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [internalValue, setInternalValue] = useState<T | T[] | undefined>( | ||
| multiple ? (Array.isArray(defaultValue) ? defaultValue : []) : defaultValue | ||
| ); |
There was a problem hiding this comment.
When multiple is true and defaultValue is not an array (e.g., undefined or a single value), the internal value is initialized to an empty array. However, if a user passes a non-array defaultValue like a string value when multiple={true}, it will be silently ignored and defaulted to an empty array.
Consider adding a warning in development mode or handling the case where defaultValue is a single value by wrapping it in an array, depending on the intended API behavior. This would make the component more forgiving and easier to use correctly.
There was a problem hiding this comment.
Couldn't we type-check this??
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…props Split SelectProps into SelectBaseProps, SingleSelectProps, and MultiSelectProps using a discriminated union on the `multiple` prop. This ensures `value`, `defaultValue`, and `onChange` are correctly typed for each mode at every call site, replacing the previous loose `T | T[]` union approach. Internally the component widens the discriminated union back to `T | T[]` because TypeScript cannot narrow generic discriminated unions inside function bodies, but external consumers now get strict type safety. BREAKING CHANGE: SelectProps is now a type alias (discriminated union) instead of a plain interface. Consumers importing or extending SelectProps directly may need to update their code. New types SelectBaseProps, SingleSelectProps, and MultiSelectProps are now exported.
Add usage hint to MultiSelectProps JSDoc for the `multiple` prop, reword the SelectProps union description to include concrete type examples (`T` / `T[]`), condense the internal type-widening rationale into fewer lines, and drop the explanatory block above the multi-select onChange call that restated what the discriminated union already covers.
Closes:
Risk level: