Page MenuHomePhabricator

Menu: enable multiple selections
Closed, ResolvedPublic5 Estimated Story Points

Assigned To
Authored By
AnneT
Jun 10 2024, 5:33 PM
Referenced Files
F55437939: Captura de pantalla 2024-06-18 a las 16.44.43.png
Tue, Jun 18, 2:44 PM
F55437896: image.png
Tue, Jun 18, 2:43 PM
F55311669: Captura de pantalla 2024-06-14 a las 12.47.14.png
Jun 14 2024, 10:49 AM
F55311273: Screenshot 2024-06-14 at 06.30.23.png
Jun 14 2024, 10:30 AM
F55309960: Captura de pantalla 2024-06-14 a las 11.41.27.png
Jun 14 2024, 9:41 AM
F55286657: image.png
Jun 13 2024, 1:03 PM
F55284804: Captura de pantalla 2024-06-13 a las 13.23.25.png
Jun 13 2024, 11:27 AM
F55005928: Captura de pantalla 2024-06-07 a las 11.51.24.png
Jun 13 2024, 11:27 AM
Tokens
"Love" token, awarded by Michael.

Description

Background

Currently, a Menu can only have a single selected item at a time. We need to enable it to have multiple selected items as well. This is required for the ChipInput with Menu (T345291).

Implementation

The Checkbox component already supports multiselect - you can pass in a modelValue that's a single boolean value (true for checked, false for unchecked), or an array of selected values (which correspond to the inputValues of the checked checkboxes:

name: 'CdxCheckbox',
props: {
    /**
     * Value of the checkbox or checkbox group.
     *
     * Provided by `v-model` binding in the parent component.
     */
    modelValue: {
        type: [ Boolean, Array ] as PropType<boolean | ( string|number )[]>,
            default: false
        }
    }

We can do something similar in the Menu component, where the selected prop can either be one of the current types (string, number, or null) or an array of strings and/or numbers. Ideally, like with Checkbox, the user can "turn on" multiselect by simply providing an array for the selected prop rather than a single value or null. If this is not possible, or if we prefer a more explicit API, we can add a prop to allow multiselect.

The Menu component will need to be updated to handle this change to the selected prop, and the UI should show selected styles for each selected item.


Acceptance criteria

  • The Menu component optionally supports multiple selections
  • When multiple items are selected, they all display selected styles
  • Keyboard navigation and accessibility features in Menu still function with multiselect
  • Demo is added to the Menu page on the docs site to demonstrate this new functionality

Future tasks

Event Timeline

AnneT triaged this task as Medium priority.Jun 10 2024, 5:33 PM
AnneT set the point value for this task to 5.
AnneT renamed this task from [placeholder] ChipInput with Menu pre-work to Menu: enable multiple selections.Jun 12 2024, 6:05 PM
AnneT updated the task description. (Show Details)

@AnneT I propose not marking all selected items within a menu as 'selected' (2). Instead, once an item is selected (and included as chip), it would remove it from the menu (1):

Captura de pantalla 2024-06-07 a las 11.51.24.png (918×2 px, 119 KB)

If we mark all menu items as selected, the Menu could end up with all its items highlighted in blue. Check this Figma prototype with both options.

Captura de pantalla 2024-06-13 a las 13.23.25.png (266×656 px, 29 KB)

hey @bmartinezcalvo, there are a few reasons I think we should keep the selected items in the menu:

  1. Removing a selected item from the menu and showing it as a chip would be a ChipInput-specific behavior. We may have other menu multiselect components in the future where it would be preferable to keep the selected items in the menu (for example, the native <select> element has a multiple mode where the menu is shown and you can select multiple options, which become highlighted)
  2. Removing a selected item from the menu seems like a somewhat jarring experience for the user. I worry that it might cause confusion or disorientation.
  3. The OOUI MenuTagMultiselectWidget keeps the selected items in the menu, even if all of them end up being selected:

image.png (330×1 px, 43 KB)

@Volker_E @Catrope do you have any context on why the MenuTagMultiselectWidget behaves this way? That may help us make this decision.

AnneT changed the task status from Open to In Progress.Jun 13 2024, 8:35 PM
AnneT claimed this task.

Change #1043227 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] Menu: Add multiselect mode

https://gerrit.wikimedia.org/r/1043227

@Volker_E @Catrope do you have any context on why the MenuTagMultiselectWidget behaves this way? That may help us make this decision.

Unfortunately I don't :/ . But I have some guesses based on reconsidering this, see below.

  1. Removing a selected item from the menu and showing it as a chip would be a ChipInput-specific behavior. We may have other menu multiselect components in the future where it would be preferable to keep the selected items in the menu (for example, the native <select> element has a multiple mode where the menu is shown and you can select multiple options, which become highlighted)

I think this is a reason why it makes sense to implement multiselect behavior in Menu, but not necessarily a reason why ChipInput should use that multiselect behavior. I think it would be possible and not necessarily inconsistent to have an equivalent of <select multiple> that involves a multiselect Menu, but also have ChipInput remove selected items from its Menu.

  1. Removing a selected item from the menu seems like a somewhat jarring experience for the user. I worry that it might cause confusion or disorientation.
  2. The OOUI MenuTagMultiselectWidget keeps the selected items in the menu, even if all of them end up being selected:

I'm not sure yet which option I prefer, but some additional considerations that come to mind:

  • How jarring it is for the selected item to be removed from the menu may depend on whether the menu closes when you make a selection, or whether it stays open. If it closes, I think it's fine; if it stays open, I agree it may be jarring. In general, I think it probably makes sense to keep the menu open.
  • How much sense it makes for selected items to continue to appear in the menu may depend on whether the menu presents a fixed set of options (e.g. the list of all namespaces, possibly filtered if the user types) or whether it presents Lookup-style search suggestions from a very large set that is never displayed all at once (e.g. pages or users). In the former case, it probably makes sense to keep selected items in the menu so that the full list of options continues to be displayed in order (and if the "all items are selected" case happens, that's because the user really did select every possible item). But in the latter case, it may not make sense to show already-selected results in response to the user typing a new search query.
  • Contradicting myself: in the Lookup-style search suggestions use case, it may not be easy/possible to get a list of suggestions from whatever API is being queried that excludes already-selected items. Those could be filtered out client-side, but then you'd present fewer results to the user (e.g. if you requested 5 results from the API and 3 of them are already selected, you'd only show 2 suggestions), or maybe even none (if you requested 5 results from the API and all 5 of them are already selected, like in Bárbara's avocado example, what do you do? Ask the API for more results I guess?)

So overall I agree with Anne, mostly from a practical standpoint. From a design standpoint, my ideal would be a compromise between Bárbara and Anne's approaches, where selected items stay in the menu when they become selected, and stay in the menu forever in a "limited set of options use case", but are not re-suggested when the user types something else in a "Lookup search suggestions" use case. But I think that latter part is probably not practical to implement.

Thanks Roan. This makes sense to me - I agree that the menu should likely stay open when an item is selected, and that the default behavior should be to leave the Menu items. If it makes sense for the use case, the parent component could be responsible for removing the selected item from the menu.

@bmartinezcalvo The engineers agree that we should consider allowing users to deselect a selected menu item by clicking it again. This is how it works with OOUI's MenuTagMultiselectWidget. What do you think? You can test my work-in-progress multiselect menu here (it currently does not deselect items on click).

El T367100#9890769, @Catrope escribió:

How jarring it is for the selected item to be removed from the menu may depend on whether the menu closes when you make a selection, or whether it stays open. If it closes, I think it's fine; if it stays open, I agree it may be jarring. In general, I think it probably makes sense to keep the menu open.

@AnneT @Catrope I agree that if the menu keeps open while the input is focused, it's logical to keep the menu items visible and selected. However, if the menu closes upon selection, it makes more sense to remove the selected item from the menu.

Since we aim to keep the selection in the menu as done in OOUI, I suggest the following:

  1. Keep the menu open while selecting items (at the moment, it closes in this patch)
  2. Indicate the selected items in the menu (we could consider adding a check indicator to reinforce this selection). cc: @DTorsani-WMF
  3. As @AnneT mentioned, selected menu items converted to chips can be deselected from the menu, allowing chips to be removed either by deleting the chip or unselecting the item in the menu.

I agree that the addition of a check indicator would help reinforce selection.

Something like this:

Screenshot 2024-06-14 at 06.30.23.png (810×1 px, 48 KB)

I agree that the addition of a check indicator would help reinforce selection.

Something like this:

Screenshot 2024-06-14 at 06.30.23.png (810×1 px, 48 KB)

I agree on including the check indicator as end icon. Since the MenuItem selected uses @color-base, I recommend using the same color in case we include the check indicator.

Captura de pantalla 2024-06-14 a las 12.47.14.png (686×1 px, 80 KB)

Great, thanks @bmartinezcalvo and @DTorsani-WMF! I'll update my patch to:

  • Keep the menu open on select
  • Add the check icons
  • Add deselection by re-clicking an option

Thanks @AnneT. This change of re-clicking to deselect an option might warrant an addition to progressive subtle background tokens to introduce interactive states for things like this. @bmartinezcalvo were actually talking about this earlier. And the broader color expansion and update outlined in T360494 supports this improvement.

El T367100#9898764, @AnneT escribió:

Great, thanks @bmartinezcalvo and @DTorsani-WMF! I'll update my patch to:

  • Keep the menu open on select
  • Add the check icons
  • Add deselection by re-clicking an option

@AnneT thank you. Since the check icon will affect the design of the MenuItem select state, I've revised its Figma spec to incorporate these updates. This includes specifying how the 16px check icon will behave with lengthy menu-item label text.

Captura de pantalla 2024-06-18 a las 16.44.43.png (557×1 px, 68 KB)

@AnneT during yesterday's DST design sync we discussed about the addition of the check icon in the selected menu items. Since we need to unblock this task and there is no full agreement on this check icon addition, we decided to remove the check icon from the selected menu items for now and leave the selected menu items as they used to be, using just the light blue to indicate the selection.

We will work on the improvement of selected menu items in this separated task T368022.

Change #1043227 merged by jenkins-bot:

[design/codex@main] Menu, MenuItem: Add multiselect mode

https://gerrit.wikimedia.org/r/1043227

Change #1049640 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from 1.7.0 to 1.8.0

https://gerrit.wikimedia.org/r/1049640

Test wiki created on Patch demo by EGardner (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/f74be9bc8e/w

Change #1049640 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from 1.7.0 to 1.8.0

https://gerrit.wikimedia.org/r/1049640