Skip to content
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

Merged
merged 23 commits into from
Apr 23, 2024

Conversation

tqwewe
Copy link
Contributor

@tqwewe tqwewe commented Apr 15, 2024

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 of value in the Select 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)

@luoxiaozero
Copy link
Collaborator

Looks very cool. Maybe we can split it into a separate component (Multiselect).

@luoxiaozero luoxiaozero mentioned this pull request Apr 15, 2024
11 tasks
@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 15, 2024

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?

@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 15, 2024

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 "Categories (2 selected)"

@luoxiaozero
Copy link
Collaborator

@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 15, 2024

Hmm not really limiting, I mean more like custom render function for the label itself.
https://ant.design/components/select#select-demo-custom-label-render

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.

[ Bookmakers (3) ]
- [x] Betfair
- [x] Sportsbet
- [x] Tab
- [ ] Pinnacle

This could be done with something like:

view! {
    <MultiSelect
        label=|values| view! { {format!("Bookmakers ({})", values.len()) }
        values
        options
    />
}

@luoxiaozero
Copy link
Collaborator

We can do this by adding a label slot.

view! {
    <Multiselect
        values
        options
    >
         <MultiselectLabel slot>
              {format!("Bookmakers ({})", values.len())
        <MultiselectLabel>
   </Multiselect>
}

@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 19, 2024

@luoxiaozero I've separated them into separate components. I also renamed it from MultiSelect into SelectMulti so it would be found in the docs beside Select (alphabetical).

I also implemented the SelectLabel slot for the selects.

We should also probably include a dropdown arrow in the select component. But maybe that can be a separate PR?

demo_markdown/docs/select_multi/mod.md Outdated Show resolved Hide resolved
thaw/src/select/mod.rs Outdated Show resolved Hide resolved
thaw/src/select_multi/mod.rs Outdated Show resolved Hide resolved
thaw/src/select_multi/mod.rs Outdated Show resolved Hide resolved
@luoxiaozero
Copy link
Collaborator

I've separated them into separate components. I also renamed it from MultiSelect into SelectMulti so it would be found in the docs beside Select (alphabetical).

I think the name MultiSelect is better, we can put the MultiSelect documentation and code into Select to solve this problem.

We should also probably include a dropdown arrow in the select component. But maybe that can be a separate PR?

Agree.

@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 20, 2024

Great I've pushed another commit with these changes

@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 20, 2024

Actually I've just gone ahead and added the dropdown arrow to the select component. Was quite a simple task so thought I'd add it here.
Screenshot 2024-04-20 at 2 05 35 PM

@tqwewe tqwewe marked this pull request as draft April 20, 2024 06:10
@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 20, 2024

I'm just trying to debug why it's looking a little different in my personal project when using this fork. The alignment isn't correct.
Screenshot 2024-04-20 at 2 22 53 PM

I've been digging through the styles but not sure what's different.

Edit

I've fixed this by setting the font-size of the select component to 14px.

@tqwewe tqwewe marked this pull request as ready for review April 20, 2024 06:58
@tqwewe tqwewe marked this pull request as draft April 20, 2024 07:03
@tqwewe tqwewe marked this pull request as ready for review April 20, 2024 08:11
@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 20, 2024

I've just implemented a clear button, when you hover over a multi select, it shows a little x button where the arrow is allowing you to clear all items.

Additionally, clicking on the select input box itself will toggle the menu, where previously clicking the input wouldn't hide it.

@tqwewe tqwewe changed the title feat!: Add support for multiple options in Select feat: Add support for multiple options in Select Apr 20, 2024
@tqwewe tqwewe marked this pull request as draft April 20, 2024 08:30
@tqwewe tqwewe marked this pull request as ready for review April 20, 2024 08:55
@tqwewe tqwewe marked this pull request as draft April 20, 2024 11:22
@tqwewe tqwewe marked this pull request as ready for review April 21, 2024 11:55
demo_markdown/docs/select/mod.md Outdated Show resolved Hide resolved
@@ -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()),
Copy link
Collaborator

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?

Copy link
Contributor Author

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

padding: 0 10px;
height: 30px;
line-height: 30px;
display: inline-block;
Copy link
Collaborator

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.

Copy link
Contributor Author

@tqwewe tqwewe Apr 22, 2024

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.

Copy link
Collaborator

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 .

@tqwewe
Copy link
Contributor Author

tqwewe commented Apr 22, 2024

I'd also like to share this screenshot of what I've implemented in my project where I'm using this fork and the <SelectLabel slot> slot to customize the label.

Screenshot 2024-04-22 at 10 53 35 PM

It's an alternative version of the multi select, where it has a label, and shows a <Tag> beside it with the number of items selected. Perhaps something like this might be of interest to people in the future?

@luoxiaozero
Copy link
Collaborator

No more problems, now merge.

@luoxiaozero luoxiaozero merged commit 324bc57 into thaw-ui:main Apr 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select with multiple items
2 participants