search

day8 / re-com

A ClojureScript library of reusable components for Reagent
https://re-com.day8.com.au
MIT License
797 stars 146 forks source link

Add :max-choices parameter to single-dropdown. #143

Closed rars closed 3 years ago

rars commented 7 years ago

Here's a PR for this issue: https://github.com/Day8/re-com/issues/102

It's a bit hard to unit test the single-dropdown component because of the reagent calls in the dropdown-top function.

I notice that the filtering has some surprising behaviour. E.g. filtering on 'Un' brings up 'Afghanistan' (presumably because 'Un' is in the group heading name "'A' Countries".

Gregg8 commented 7 years ago

I've shown a working copy of this to a few co-workers and they all feel a dropdown with hidden extra choices has poor usability, as there is no way to easily discover the extra options.

So, I'm finding it difficult to include this. Is there something I've misunderstood?

Also, re your filtering query, yes it is intentional that group headings are included in the search and showing all items under that group name would appear to be the most sensible thing to do. e.g. if you had a list of cities grouped by country and you typed the country name, you'd get a list of all cities in that country.

rars commented 7 years ago

Yes, I tend to agree with you on the usability aspect of this. I don't need it myself. My personal motivation was just to get a bit of experience working with reagent, and issue #102 seemed like an easy start. Perhaps you could close that issue now?

Let me know if you find anything from this PR useful to include and I could split that off into a separate PR; otherwise, feel free to decline the PR. Thanks for taking a look.

Regarding the filtering behaviour, I can see a use case for both behaviours. In the demo example, I don't think I'd find myself wanting the query to match on group names because the group names only differ by their leading letter ('A' Countries, 'B' Countries, etc). However, I can imagine that there would be situations where querying for a group name would be useful.

superstructor commented 3 years ago

As per previous discussion closing without merging as hidden choices is probably poor usability.

Thanks for the effort all the same @rars