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

Add the ability to have multiple options marked as selected #152

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshavg
Copy link

@joshavg joshavg commented Oct 2, 2018

This way, multi selectes can be realized where the select tag hast the attribute multiple="multiple"
together with size="x" and users can ctrl-click multiple options

This way, multi selectes can be realized where the select tag hast the attribute multiple="multiple"
together with size="x" and users can ctrl-click multiple options
@weavejester
Copy link
Owner

Thanks for the PR. Can you ensure the commit follows these contributing guidelines, and can you also remove the docstring from the private function?

I also think that this might be more flexible if we check for ifn? instead, so if we want to have multiple options set we can supply a set or a function.

Lists and vectors are also considered IFn's, so the are explicitly
checked and treated as standard data structures using (some ...).
@joshavg
Copy link
Author

joshavg commented Oct 2, 2018

Hi, I hope, I followed the guidelines better this time. Simply switching sequential for ifn? didn't work out that well. Maybe you can come up with a more elegant solution?

@@ -74,6 +74,12 @@
:value value
:checked checked?}]))

(defn- selected? [x selected]
(cond (or (list? selected) (vector? selected))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

(or (and (sequential? selected) ((set selected) x))
    (and (ifn? selected) (selected x))
    (= x selected))

@weavejester weavejester added the waiting on dev Waiting for the PR opener to make changes label Jan 20, 2022
@xuanwu
Copy link

xuanwu commented Feb 4, 2024

Looking for this feature as well. What needs to happen for this to move forward?

@weavejester
Copy link
Owner

Looking for this feature as well. What needs to happen for this to move forward?

The code needs to be updated as per my previous comment. That's why this issue is marked as "waiting on dev".

@xuanwu
Copy link

xuanwu commented Feb 4, 2024

Thanks for the quick reply. Sorry I didn't mean to sound pushy. I understand that's ideally what needs to happen (I did see the tag), but since it's been more than 5 years since you requested the change and more than 2 years from the addition of the tag and the dev hasn't responded, I am wondering what happens to the PR if the dev never responds. Does it stay in limbo forever? I imagine it'd be awkward if someone else agreed with your changes and issued the same PR with those changes. I'm new to Clojure so I'm genuinely curious.

@weavejester
Copy link
Owner

Someone else can make the same change and submit a PR. In which case I'll merge that PR and close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting on dev Waiting for the PR opener to make changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants