-
Notifications
You must be signed in to change notification settings - Fork 32
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
New variant for tags-mode #78
base: main
Are you sure you want to change the base?
Conversation
The multiple select option for the new tags_mode makes it possible to select multiple options at once without needing to search or force the dropdown to appear. It is a kind of hybrid of the search variant and a normal multi select input input.
691c311
to
f4d4e24
Compare
Hi @ringvold thanks a lot for this, I just took a superficial look but it sounds like solid work 💪 using checkboxes in the dropdown is something I've been thinking of adding for a long time now. Unfortunately, I'm quite busy this coming week, but I will take a proper look as soon as I find some time. Cheers! |
Hi @ringvold I finally found the time to read carefully what you wrote (I've been really busy in the past few weeks, still busy but things are getting a bit better). I haven't looked at the code yet. Here's a couple of comments:
EDIT: thinking about it a bit more, a simple additional boolean attribute
<:option :let={%{label: label, value: value, selected: selected}}>
In this way, old code that ignores the selected field would still work
Let me know what you think |
Hi! Thought I'd get back to you sooner but stuff happened here as well.
Functionally it is a bit more than "don't close the dropdown" (as I made it here at least) as it also enables you to deselect from the drop down which I believe is not possible on "normal" tags mode. I think that might be why I think of it more as a variant of tag mode. You should have the final say on the API but I feel like So I lean towards
Other things of noteI have noticed that this PR breaks the current behavior for tags as you can deselect from the drop down. Which you can not to in the current released version. I have tried to not change the current functionality of tag mode so I'm not sure what happened there. I consider that a bug. One thing I have not figured out yet (although not look at it very much yet) is how to not loose focus of the element when selecting it when operating it with keyboard only. Currently it resets focus to the search box when selecting. I would like to keep it in the dropdown at/around the selected element to be able to continue the selection. Any thoughts on this? |
Do not move focus back to search box on first select. Also refactor to use more efficient way of checking for already selected.
Yeah apologies I hadn't realized you can deselect from the dropdown...
And this is exactly what I meant by "using checkboxes to deselect". Ok so I agree with you now, close_dropdown_on_select obviously doesn't cut it. A third selection mode is more appropriate. quick_tags sounds good to me!
You mean custom fields they're passing to the slots? To preempt this, we could simply taise an exception when someone uses the selected key for the options. I will take a look at the last 2 points you mentioned (breaking current behavior and not losing focus) soon. But it sounds like at least we agreed on how to expose this new mode in the API and how to pass the selected status in the slots :) |
Yes, it does sound like we are onto something her 😄
That is maybe also a breaking change but certainly much smaller than my suggestion. I will make a suggestion on the default styles for this mode. I cant remember the details but there are some changes that need to be made to make it look decent out of the box. |
I don't think this is a breaking change because we're not breaking any API. The docs say that the options can be specified as maps with the shape |
Is this one good to merge? |
@egeersoz I still have to implement what me and Max talked about here, add test and then probably a new check by @maxmarcon. I hope to find some time to look at it this weekend though. :) |
Hi! 😄
This is a suggestion for a new variant of the tag mode. The multiple_select option for the new tags_mode attribute makes it possible to select multiple options quickly without needing to search or force the dropdown to appear. The current tag mode hides the dropdown-part when a option is selected. This approach could probably be described as a kind hybrid of the normal live_select tag mode and a normal multi select input.
A new attribute
tags_mode
is added with:default
(how tags work today) and:multiple_select
as possible options.Example usage taken from demo-app:
There is some code in the demo app related to selected_option_class as this feature would need some new style defaults/style changes to look passable out of the box. This should probably be solved some other way. Maybe by adding some new default styles.
The name chosen, multiple_select, is very much up for debate. I just needed something to work with.
Potential performance issue
When testing in the demo app I do notice some slowness when selecting or deselecting many options in rapid succession. I did not experience this in our app. I am not sure if this is a flaw in the implementation or just that the dataset for the demo app is very large. Any suggestions on this point is especially welcome.
Breaking changes
The feature also changes the value exposed to the option-slot to also return an indication if the option is already selected. This is one of the main points of the change as the impetus for this feature is a design/UX that displays the selected with a checkbox. See attached picture for example. I suppose this could be done with just styles but the design seems harder to replicate that way and I also think it would be nice to be able to have that info available to the slot. But still open for debate. :)
Example of option-slot usage:
Notice the :let now exposing a tuple which contains the option and a selected boolean.
Rendered example:
This is a suggestion based on functionality that we where in need of and I hope it can be added to the project in some form.
This is not expected to be merged as-is (ex. tests is missing for now) but it is working code that we would use in our fork and a good point to discuss from I think.
I'm aware that this might not be something that this project wants and I'm happy to make changes or discuss the approach. Maybe there is a better way to achive the same or something very similar?
The changes necessary does not seem to invasive though from my perspective (except maybe the option-slot breaking change 😅).
Let me know what you think. 😄
EDIT: I forgot while writing this, but there is also a bug in this implementation I have not figured out. When making the first selection the blur event triggers and hides the dropdown. This does not happen on other selections but consistently happens from state of nothing selected to first option selected.