-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
DataViews: implement multiple selection in filters #56468
Conversation
Size Change: +81 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
This PR puts more pressure on fixing how filters grow (Edit: #56467 may be a good enough starting point.) Gravacao.do.ecra.2023-11-23.as.09.37.32.mov |
@jameskoster @SaxonF I was thinking about filters that allow multi-selection vs the ones that don't (only a single item can be selected). I think I saw somewhere a mockup where these would work differently: checkboxes (selected items with checkbox) vs radios (selected item with circle), respectively. Is this correct? If so, I'd welcome some clarification. In testing the new components and talking to @ciampo about their behavior (see conversation), he brought up that radios are meant to always have an item selected. So. I wonder how would you like the empty state to work for single selection filters if we use radios (e.g.: how a user clears a selection after having selected some item.). Do we bring back the "None/All" item for single-selection filters? Do we use checkboxes for both single and multi selection filters? etc. |
I'm also going to add that, at least in terms of semantics,
Looking at this use case, potentially an alternative to using checkboxes and/or radios inside a Related read: https://ariakit.org/components/menu#should-i-use-menu-or-select Cc'ing @andrewhayward for further advice on this (and to make sure that my advice makes sense!) |
To confirm @ciampo's suspicions, these should be comboboxes rather than menus. |
In practical terms, currently using the
|
More generally for this PR, could we try the following format on the filter button:
The "or" part will be important for when we add |
What would think of codifying that piece of data right after the filter name:
Would this be clear enough? I'm thinking that it provides the same info in a more compressed fashion, and it doesn't require users to scan both the beginning and end of the sentence to understand how the filter works. We could use different wording: |
Normally, if semantically we're using radio items (ie. a list of options where only one option can be selected at the same time), we can't unselect the currently selected item. That's why a "None"/"All"/"Any" option may be needed to "disable" the selection. In this specific case, since it's possible to delete the filter by clicking the "Remove filter" menu item (which would cause the dropdown to disappear) I guess there's no need for such an option.
Radios are great when we want the user to be able to select only one option at a time from all options available. Also, selected radios can't be unselected. This means that there will be instances where the radio semantics won't work for a filter. In those cases, if we were to start using radio-like design also for items that are not actually radio items, that would have the potential to really confuse the end user.
Adding a "remove filter" action (ie. not an option that can be selected from a list) can potentially mean that this widget needs to be a What needs to be understood, then, is what is ideally the best way (in terms of semantics and UX) to implement this widget:
@diegohaz and @andrewhayward :
|
Warning I'm brain-dumping here, so take this with a pinch of salt!
As a baseline, the trigger control should be a
So I wonder if the most appropriate route here is to have a <button role="combobox" aria-haspopup="dialog">Comments</button>
...
<div role="dialog">
<div role="listbox">
<button role="option" aria-selected="true">Enabled</button>
<button role="option" aria-selected="false">Disabled</button>
</div>
<div role="menu">
<button role="menuitem">Reset filter</button>
</div>
</div> When the We'd need to experiment with this a little to see whether it actually works, but in principle it might suit our needs here. For multi-selection, it would only be slightly different... <button role="combobox" aria-haspopup="dialog">Author</button>
...
<div role="dialog">
<div role="listbox" aria-multiselectable>
<button role="option" aria-checked="true">Author McName</button>
<button role="option" aria-checked="false">Writer O'Surname</button>
<button role="option" aria-checked="true">Scribe Lastname</button>
</div>
<div role="menu">
<button role="menuitem">Reset filter</button>
</div>
</div> |
Making sure that a Again, this is untested, but something like this could work: <div role="combobox" aria-label="Author">
<span aria-hidden="true">
Author is Scribe Lastname
</span>
<span class="visually-hidden">
Scribe Lastname
</span>
</div> It has its issues – the label isn't strictly visible, what's rendered doesn't match the semantics, etc – but might be as close as we're going to get. |
In theory, a A select widget is essentially a select-only So, I'd say it's fine for the filters to use this pattern. There's a similar example on Ariakit (unlisted), but the "Remove filter" button is rendered outside the filter select: https://ariakit.org/examples/select-menu-default-open |
Alternatively, it could use <div role="combobox" aria-labelledby="label">
<span id="label" aria-hidden="true">
Author is
</span>
Scribe Lastname, Other
</div> This will result in "Author is " as the label, and "Scribe Lastname, Other" as the value. |
That was my first thought, but we'd need to be mindful of translation, which becomes that much harder when sentences are broken up across DOM nodes. |
@oandregal the wording is tricky. I don't know that 'contains' translates to 'or' very well 🤔 It also makes the button quite long. Maybe we don't need to communicate the operator in the label. It can be made quite clear in the popover like: There we have more space to get the wording right (above is just a placeholder). What do you think? |
The problem with that is if an As a general rule, we should try to keep options for choices and menu items for actions. |
Of course, if the final output is going to be more akin to |
It's been a while since I last tested it. I'm not sure if browsers already implement this spec, but as far as I understand, they should only include an implicit
MDN also makes a distinction between selectable, non-selectable, and disabled options:
If browsers don't follow this and add an implicit |
I don't have anything else to hand right now, but with the following very minimal test case... <div role="listbox" aria-label="Selected?">
<button role="option" aria-selected="true">True</button>
<button role="option" aria-selected="false">False</button>
<button role="option">Undefined</button>
</div> ...VoiceOver reads the following in Safari:
Chrome/Edge:
Firefox:
So it's clearly at least variable! |
As with all these things, user experience is priority zero. So agreed, ultimately we go with whatever actually makes the most sense. If that's a collection of buttons following a |
Thanks for testing. I opened an issue on Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1504835 |
I've been working on refactoring a bit the existing components to be able to add this feature all at once in all of them. I'd like to land #56514 before this PR. |
What is not clear to me, is what we identified as the best way to implement these widgets — if I followed the conversation correctly, are we thinking of using a dialog with a listbox inside (basically what @andrewhayward suggested in this comment) ? If so, shouldn't we first prototype a proof of concept before moving forward? And where should such a component live? Probably better to start by having it as a private component for data views, and later decided whether it makes sense to abstract it and move it to the |
#58569 redesigns the filter UX, multi-selection would need to be added to that work. |
Part of #55083
Related #55100
What?
Implements multiselection in filters.
Gravacao.do.ecra.2023-11-23.as.09.25.29.mov
Why?
Many fields support being filtered by multiple values at once.
How?
Updates the filter components & the view to query adaptor to take arrays instead of a string for the values.
Testing Instructions
TODO / Questions