-
Notifications
You must be signed in to change notification settings - Fork 12
Introduce picker component #3908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Preview deployments |
Host Test Results 1 files ± 0 1 suites ±0 3h 25m 39s ⏱️ + 1h 44m 0s For more details on these errors, see this check. Results for commit c1ad1b1. ± Comparison against base commit 5a329a3. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a635ba8779
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @dropdownClass={{if | ||
| @dropdownClass | ||
| @dropdownClass | ||
| 'boxel-multi-select__dropdown' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve base multi-select dropdown class when customizing
When a custom @dropdownClass is provided, the default boxel-multi-select__dropdown class is replaced rather than combined. The base dropdown styles (box shadow, list reset/padding, option layout, z-index) are scoped to .boxel-multi-select__dropdown later in the same file, so any custom dropdown (e.g. the new Picker’s boxel-picker__dropdown) will lose those base styles. This will change dropdown rendering (bulleted lists, missing padding/shadow, z-index) in any consumer that sets a custom class. Consider concatenating the default class with the custom class (like the Select component does) so custom dropdowns keep the base styling.
Useful? React with 👍 / 👎.
burieberry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are a couple of suggestions for improvement:
Functionality:
- When a selection is made, the default selection ("All" Realms or Types) should no longer be selected as per the design:
Styling:
- the default state needs to be updated to mimic the design
- The X button on pills looks really small and not aligned in the middle. You can use a similar X button as the linksToMany component pills.
- We need to adjust the colors for hover, selected, selected hover states. Currently they seem to have uneven padding and different coloring:
packages/boxel-ui/addon/src/components/picker/selected-item.gts
Outdated
Show resolved
Hide resolved
packages/boxel-ui/addon/src/components/picker/selected-item.gts
Outdated
Show resolved
Hide resolved
|
@burieberry Thanks for the feedback! I made a few updates based on your suggestions:
Screen.Recording.2026-01-29.at.13.53.44.mov |
|
The only left is the icon for |
This new component is a modified version of the multi-select component with a few behavioral differences:
This component will be used for the realm picker and type picker in the card chooser.
Screen.Recording.2026-01-27.at.18.01.07.mov