- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 10
chore: Migrate Popover Dropdown component #180
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
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.
LGTM!
| Screenshots in the PR description would be a nice addition, especially because we don't get a preview env :) | 
| I've now refactored the component to have a more aligned interface like the Select component, alongside with allowing for a custom trigger element to be passed in. With that custom trigger, I could also now remove all the button props (color, shape, variant, size), should I? | 
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.
This looks pretty good!
| {#if !value} | ||
| {placeholder} | ||
| {:else} | ||
| {value?.label} | ||
| {/if} | 
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.
I would probably do this in JS land with {value?.label || placeholder}. That's definitely exclusively personal preference though
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.
oh yeah agreed, that's a remnant of what I had there before
| 
 I don't think so. That's much more convenient if you only want to change a single thing. | 
As mentioned over here: immich-app/immich#18570
This moves the "Popover Dropdown" component from the main project to the ui lib, and refactors it to use the Popover from
bits-ui. This fixes some clipping/overflow issues when using the dropdown inside a modal (specifically theSlideshowSettingsModal)Screenshots