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 “select all” & “select none” option to checkboxes #835

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mboynes
Copy link
Contributor

@mboynes mboynes commented Apr 4, 2022

Something to get the conversation started.

Resolves #834

@mboynes mboynes changed the title Add “select all” & “select none” to checkboxes Add “select all” & “select none” option to checkboxes Apr 4, 2022
@dlh01
Copy link
Member

dlh01 commented Apr 5, 2022

Nice! I tested the branch with https://github.com/alleyinteractive/fieldmanager-demos and with the checkboxes from the screenshot in #834, and it worked as expected.

Some high-level comments:

  • I'd personally vote for this being opt-out, rather than opt-in. There are a lot of checkboxes out there already that I would think would benefit from it, and it seems to me like the kind of thing you'd wish was everywhere once you saw it.
  • I found the text a little difficult to see. I know Fieldmanager tends to be inconsistent about using core CSS classes, but the .button.button-link or .button-group > .button.button-secondary classes make these look more natural to me (see screenshot below of the button group), plus they'd adapt to the user's color scheme.
  • Bikeshedding, but bulk "select" and "selectable" read a little funny to me in the context of checkboxes. What about "bulk toggle"? Or "bulk actions," in line with core?
  • I had expected the buttons to be at the bottom of the list, not the top, and while I came to appreciate their being at the top, I'm 50/50 on whether to advocate for making that configurable in the same way that the "Add more" button or field description can be.

@dlh01
Copy link
Member

dlh01 commented Apr 5, 2022

Forgot that screenshot:

Screen Shot 2022-04-04 at 8 47 21 PM

@mboynes
Copy link
Contributor Author

mboynes commented Apr 5, 2022

Thanks for trying this out and sharing thoughtful feedback @dlh01!

I'd personally vote for this being opt-out, rather than opt-in. There are a lot of checkboxes out there already that I would think would benefit from it, and it seems to me like the kind of thing you'd wish was everywhere once you saw it.

I hadn't thought about it, but sure, I can get on board with that.

I found the text a little difficult to see. I know Fieldmanager tends to be inconsistent about using core CSS classes, but the .button.button-link or .button-group > .button.button-secondary classes make these look more natural to me (see screenshot below of the button group), plus they'd adapt to the user's color scheme.

I'm not opposed to buttons like in your screenshot, and the argument that we can adopt core styles and benefit from core themes is reason enough for me. I considered something similar at first but moved to the UI I chose because I felt it was a pretty common pattern around the web (screenshot below for posterity).

Screen Shot 2022-04-05 at 11 05 05 AM

Here's a similar example from kayak.com:

Screen Shot 2022-04-05 at 10 40 09 AM

I looked around the WordPress Admin and the common UI pattern seems to be a single checkbox that represents all or none. See this screenshot from the menu manager, which is probably most similar:

Screen Shot 2022-04-05 at 11 02 25 AM

All that said, I don't have a strong preference here but agree with going with either your approach or one matching the menu manager. Do you have a preference between the two @dlh01?

Bikeshedding, but bulk "select" and "selectable" read a little funny to me in the context of checkboxes. What about "bulk toggle"? Or "bulk actions," in line with core?

You win, we can paint the bike shed blue! 😉

I like bulk actions, I'll rename things to that.

I had expected the buttons to be at the bottom of the list, not the top, and while I came to appreciate their being at the top, I'm 50/50 on whether to advocate for making that configurable in the same way that the "Add more" button or field description can be.

My two cents is that the top generally works best. In my opinion, adding it to the bottom requires that the list be limited in height and scrollable (like the menu manager screenshot) -- which is not out of the question to do alongside this change. But without that, if a user had a list of 100 items, they'd have to scroll down the page to get to the select all/none. I don't have a strong opinion this except that I don't particularly favor adding a top-or-bottom option here... decisions over options, after all.

@mboynes
Copy link
Contributor Author

mboynes commented Apr 5, 2022

Here's a sample of what the checkbox approach might look like:

Screen Shot 2022-04-05 at 11 26 45 AM

@mboynes
Copy link
Contributor Author

mboynes commented Apr 5, 2022

I pushed up the "Select all" toggle checkbox to kick around

@dlh01
Copy link
Member

dlh01 commented Apr 7, 2022

I don't have a strong preference here but agree with going with either your approach or one matching the menu manager. Do you have a preference between the two @dlh01?

I think, probably, my personal favorite is the dual-links approach like the Kayak screenshot, which I think could be more-or-less be achieved with the .button.button-link styles from core.

The single-checkbox approach that core takes always seemed so-so for me. I would think (although I'm assuming) that it's not intuitive to many users that you have to "select all" before you can "select none," not to mention that "select none" requires an extra click if only some boxes are checked.

But...I can see the value in providing an experience that looks and feels like core. So maybe that'd be slightly where my preference would be, if reluctantly.

One thing I noticed in my testing of the PR: If I manually check all the boxes, the "Select all" box doesn't become checked, unlike in core. (If I check "Select all," then uncheck a box, "Select all" becomes unchecked as expected.)

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.

Add 'Select all' and 'Select none' buttons to checkboxes
3 participants