-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: Add support for multiple options in Select #166
Conversation
Looks very cool. Maybe we can split it into a separate component (Multiselect). |
I was considering that, it would also mean it's no longer a breaking change. Though I'm not sure how this could be done without duplicating so much of the code? |
I wonder if it's worth having a prop to override the entire view rendering of the label? For example, you might want to have custom behaviour where the label is just text with a number of items selected: eg |
Hmm not really limiting, I mean more like custom render function for the label itself. For users who want to render anything they want. In my personal use case, I want a filter called "Bookmakers", and I don't want to render all the bookmakers selected, instead I only want to show the number of bookmakers selected.
This could be done with something like: view! {
<MultiSelect
label=|values| view! { {format!("Bookmakers ({})", values.len()) }
values
options
/>
} |
We can do this by adding a label slot. view! {
<Multiselect
values
options
>
<MultiselectLabel slot>
{format!("Bookmakers ({})", values.len())
<MultiselectLabel>
</Multiselect>
} |
@luoxiaozero I've separated them into separate components. I also renamed it from I also implemented the We should also probably include a dropdown arrow in the select component. But maybe that can be a separate PR? |
I think the name MultiSelect is better, we can put the MultiSelect documentation and code into Select to solve this problem.
Agree. |
Great I've pushed another commit with these changes |
I've just implemented a clear button, when you hover over a multi select, it shows a little Additionally, clicking on the select input box itself will toggle the menu, where previously clicking the input wouldn't hide it. |
thaw_components/src/binder/mod.rs
Outdated
@@ -182,7 +184,8 @@ fn FollowerContainer<El: ElementDescriptor + Clone + 'static>( | |||
let mut style = String::new(); | |||
if let Some(width) = width { | |||
let width = match width { | |||
FollowerWidth::Target => format!("width: {}px;", target_rect.width()), | |||
FollowerWidth::Target => format!("min-width: {}px;", target_rect.width()), | |||
FollowerWidth::MinTarget => format!("width: {}px;", target_rect.width()), |
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.
What is the purpose of these two lines of code?
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.
Oops, MinTarget
should've been min-width
instead of Target
.
I added MinTarget
and used it in the Select
now to make sure the select dropdow menu doesn't get cut off if the select input is too short. I think this would be fine to have the dropdown menu be longer in case the options are longer, but its up to you to keep this or not. Personally as a user of Thaw, I'd prefer the text in the dropdown not to be cut off
thaw/src/select/select.css
Outdated
padding: 0 10px; | ||
height: 30px; | ||
line-height: 30px; | ||
display: inline-block; |
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's a breaking change.
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.
Okay I've remove this in 68928ad in order to keep the PR non-breaking, but are you happy to include this in the next version? If so, I can open an issue to track this.
I often refer to Ant Design and other UI libraries for these decisions and see they use inline-block, and it also makes sense for the UI's I'm building with Thaw.
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 need to think about it. I opened #176 .
No more problems, now merge. |
Adds support for selecting multiple options in the
Select
component using closable tags.This should close #101
Screen.Recording.2024-04-15.at.5.53.47.PM.mov
Issues:
Since this changes the type ofvalue
in theSelect
component, it means this PR is a breaking change.The popup menu doesn't update it's position when the multiselect gets too long and goes onto two lines.(Fixed with d0e96eb)