Page MenuHomePhabricator

Lookup: Option to open menu when field is focused
Closed, ResolvedPublic5 Estimated Story Points

Description

Background goal

While working on T350037: ZObjectSelector: Pre-populate the results with most common objects when the selection type is Z60/Language or Z4/Type we learned that is currently not possible to open a menu when people focus the Lookup.

During research we learned that there's an educational opportunity to proactively display suggested or recommended results when people focus a lookup field (based on the current context).

In the specific case of Wikifunctions, when people focus a lookup field that is used for the input type of a function, we could open the menu with the most used types for function inputs.

CleanShot 2023-11-10 at 15.18.20@2x.png (1×1 px, 123 KB)

User stories

add at least one user story

Design spec

Open questions

  • Do we want to implement this as a base behavior for Lookup? - No, this will affect the Lookup just if an initial list of menu items is passed into the Lookup. In this case, it will open the menu on focus, and keep the menu open when the input is cleared.

Acceptance criteria (or Done)

Design

Code

  • Implement this component's use case in Codex

Future tasks

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@AAlhazwani-WMF @Jdforrester-WMF for both this task and T351768, I believe @bmartinezcalvo recommended using the ChipInput with menu variant instead (T345291). If you're in agreement with that decision then I will decline these two Lookup tasks since this isn't something we've seen a need for elsewhere as of yet.

@AAlhazwani-WMF @Jdforrester-WMF for both this task and T351768, I believe @bmartinezcalvo recommended using the ChipInput with menu variant instead (T345291).

I missed that advice/meeting, sorry – where was that advice documented? I might be missing part of the suggestion, but this seems wrong from a UX perspective to me. This isn't an appropriate context for a ChipInput, and I don't understand how it would be? (Single-value selection not multi-value.)

If you're in agreement with that decision then I will decline these two Lookup tasks since this isn't something we've seen a need for elsewhere as of yet.

I'm happy to point out other use cases across the estate from other teams for such things, but that wasn't the advice we were given from DST, and I don't know how they might feel about me freelancing in other product areas.

@AAlhazwani-WMF @Jdforrester-WMF for both this task and T351768, I believe @bmartinezcalvo recommended using the ChipInput with menu variant instead (T345291).

I missed that advice/meeting, sorry – where was that advice documented? I might be missing part of the suggestion, but this seems wrong from a UX perspective to me. This isn't an appropriate context for a ChipInput, and I don't understand how it would be? (Single-value selection not multi-value.)

Oh I thought this was a multi-value selection but now I see that although there can be multiple inputs, there is only one type per input. I think this was misunderstood when the recommendation to use ChipInput was made.

If you're in agreement with that decision then I will decline these two Lookup tasks since this isn't something we've seen a need for elsewhere as of yet.

I'm happy to point out other use cases across the estate from other teams for such things, but that wasn't the advice we were given from DST, and I don't know how they might feel about me freelancing in other product areas.

Yeah fair enough. I think it would be best to have at least one use case if the task remains open. It should probably stay open either way based on the above misunderstanding, pending clarification from @bmartinezcalvo.

@AAlhazwani-WMF @Jdforrester-WMF for both this task and T351768, I believe @bmartinezcalvo recommended using the ChipInput with menu variant instead (T345291).

I missed that advice/meeting, sorry – where was that advice documented? I might be missing part of the suggestion, but this seems wrong from a UX perspective to me. This isn't an appropriate context for a ChipInput, and I don't understand how it would be? (Single-value selection not multi-value.)

@CCiufo-WMF @Jdforrester-WMF during our last AW/DS design sync with @AAlhazwani-WMF, @DTorsani-WMF @Volker_E and me discussed the possibility of using a ChipInput with a menu for this use case, so the user can easily add and remove more than one suggested type. @AAlhazwani-WMF please confirm if this was the intention.

Captura de pantalla 2023-12-11 a las 10.54.00.png (706×1 px, 72 KB)
Captura de pantalla 2023-12-11 a las 11.05.51.png (912×2 px, 148 KB)
Using LookupUsing ChipInput

If we do this, then we will need to include the ChipInput with displayed menu T345291: ChipInput: Add menu functionality (since the current ChipInput doesn't display any menu for now) and we discussed that @AAlhazwani-WMF could contribute the design of this new ChipInput with menu functionality.

@AAlhazwani-WMF @Jdforrester-WMF for both this task and T351768, I believe @bmartinezcalvo recommended using the ChipInput with menu variant instead (T345291).

I missed that advice/meeting, sorry – where was that advice documented? I might be missing part of the suggestion, but this seems wrong from a UX perspective to me. This isn't an appropriate context for a ChipInput, and I don't understand how it would be? (Single-value selection not multi-value.)

@CCiufo-WMF @Jdforrester-WMF during our last AW/DS design sync with @AAlhazwani-WMF, @DTorsani-WMF @Volker_E and me discussed the possibility of using a ChipInput with a menu for this use case, so the user can easily add and remove more than one suggested type.
@AAlhazwani-WMF please confirm if this was the intention.

Captura de pantalla 2023-12-11 a las 10.54.00.png (706×1 px, 72 KB)
Captura de pantalla 2023-12-11 a las 11.05.51.png (912×2 px, 148 KB)
Using LookupUsing ChipInput

maybe it got lost in conversation :) but function creators can only pick one type per input, not multiple.

If we do this, then we will need to include the ChipInput with displayed menu T345291: ChipInput: Add menu functionality (since the current ChipInput doesn't display any menu for now) and we discussed that @AAlhazwani-WMF could contribute the design of this new ChipInput with menu functionality.

yeah, we chatted about it, but hypothetically. we wondered if there were other codex components that might be of use instead of the lookup, mostly because we were talking about the lack of a stronger feedback after editors pick a result from the menu of a lookup field.

we left the convo with the decision to keep this task open until i would bring this potential alternative to the rest of the abstract team, especially because "our lookup" is being used on several pages on wikifunctions, and it's a core (and critical too) component. moreover there might be potential conflict with this approach as we're currently using the the ChipInput for the Aliases field, see https://www.wikifunctions.org/wiki/Z10000?action=edit&uselang=en (you need to be logged in to view).

CleanShot 2023-12-11 at 11.47.42@2x.png (1×3 px, 696 KB)

that said, i believe this task is still valid, even if we decide to use a different component for the function input type in the future. for instance, given that languages are wikifunctions objects too, we need this lookup event to display and suggest UN languages when editors focus a lookup field for picking a language, see T343209: Display UN6 languages as suggested languages in the Global Language Selector.

@AAlhazwani-WMF @Jdforrester-WMF for both this task and T351768, I believe @bmartinezcalvo recommended using the ChipInput with menu variant instead (T345291).

I missed that advice/meeting, sorry – where was that advice documented? I might be missing part of the suggestion, but this seems wrong from a UX perspective to me. This isn't an appropriate context for a ChipInput, and I don't understand how it would be? (Single-value selection not multi-value.)

@CCiufo-WMF @Jdforrester-WMF during our last AW/DS design sync with @AAlhazwani-WMF, @DTorsani-WMF @Volker_E and me discussed the possibility of using a ChipInput with a menu for this use case, so the user can easily add and remove more than one suggested type.
@AAlhazwani-WMF please confirm if this was the intention.

Captura de pantalla 2023-12-11 a las 10.54.00.png (706×1 px, 72 KB)
Captura de pantalla 2023-12-11 a las 11.05.51.png (912×2 px, 148 KB)
Using LookupUsing ChipInput

maybe it got lost in conversation :) but function creators can only pick one type per input, not multiple.

If the user can pick just one type per input, then it would make more sense to use a single selection component such as Lookup or Select, since ChipInput is more suitable for cases where the intention is to create/select different chips.

CCiufo-WMF renamed this task from Lookup: event to open menu when field is focused to Lookup: Option to open menu when field is focused.Dec 11 2023, 9:51 PM
CCiufo-WMF updated the task description. (Show Details)

It sounds like there was a misunderstanding and that we should keep this task open.

I've updated the task title to focus more on the use case (opening a menu on focus) and less on the specific solution (triggering an event). The use case might be generalizable but I think we have enough here to start exploring the proposed component improvement. I'm wondering if we still need to keep T351768 open.

CCiufo-WMF moved this task from Triaging to Needs Refinement on the Design-System-Team board.
bmartinezcalvo set the point value for this task to 5.

We are moving this task to the sprint to evaluate this behavior, but this will need discussion.

I see a few possible solutions here:

  1. Allow users of the Lookup component to force the menu to open. Lookup already emits an event when it's focused, so this would allow developers to listen for the focus event, and force-open the Lookup menu in response.
  2. Add a prop to Lookup that, if set to true, causes the Lookup to open its menu when it's focused, even when no text is entered. This is kind of like #1, but moves the logic into the Lookup component rather than making the parent component responsible for it.
  3. Change the Lookup component to always open its menu when it's focused, even when no text is entered. This is like #2, but implemented for everyone rather than on an opt-in basis. In most traditional Lookup use cases this would behave the same as the current behavior, because traditionally there are no menu items when the user hasn't entered any content yet. (However, the parent component would have to be careful to empty out the menuItems array when the user clears the text in the Lookup input.)

I don't think we should do #1, because I think #2 is preferable over #1. But I think we should discuss the pros and cons of #2 vs #3, because it's not entirely clear to me which one is better (and maybe there are other options I haven't considered).

I see a few possible solutions here:

  1. Allow users of the Lookup component to force the menu to open. Lookup already emits an event when it's focused, so this would allow developers to listen for the focus event, and force-open the Lookup menu in response.
  2. Add a prop to Lookup that, if set to true, causes the Lookup to open its menu when it's focused, even when no text is entered. This is kind of like #1, but moves the logic into the Lookup component rather than making the parent component responsible for it.
  3. Change the Lookup component to always open its menu when it's focused, even when no text is entered. This is like #2, but implemented for everyone rather than on an opt-in basis. In most traditional Lookup use cases this would behave the same as the current behavior, because traditionally there are no menu items when the user hasn't entered any content yet. (However, the parent component would have to be careful to empty out the menuItems array when the user clears the text in the Lookup input.)

I don't think we should do #1, because I think #2 is preferable over #3. But I think we should discuss the pros and cons of #2 vs #3, because it's not entirely clear to me which one is better (and maybe there are other options I haven't considered).

I agree on #2 to automatically display the menu when the Lookup is focused just if the menu displays some suggested options as in the AW example shared in this task.

CleanShot 2023-11-10 at 15.18.20@2x.png (1×1 px, 123 KB)

When the menu contains an extensive list of options, typing in the Lookup to search for them makes more sense.

I just realized this is a duplicate of an existing task, and I wonder if we should consider @AnneT's suggestion there of this possibly being better served by the Combobox:

@Celenduin is this perhaps a use case for a Combobox, where you show an initial list of items and then filter the items once there is text in the input?

If we stick with using the Lookup component though, I agree with #2 as well. Here's a new question though: What controls which items, and how many, are populated in the menu on focus by default? In the Wikifunctions example, it seems like that list of "Suggested Types" is a set of values pre-determined by the designers (i.e. it should always show Boolean, List, String on focus). Or maybe it changes depending on the context? It seems like either way, it might not just be the default sort order of the menu items. Do we need to account for adding that functionality as well, or is it already possible with the current Menu? cc: @AAlhazwani-WMF
to provide more context.

If we stick with using the Lookup component though, I agree with #2 as well. Here's a new question though: What controls which items, and how many, are populated in the menu on focus by default? In the Wikifunctions example, it seems like that list of "Suggested Types" is a set of values pre-determined by the designers (i.e. it should always show Boolean, List, String on focus). Or maybe it changes depending on the context? It seems like either way, it might not just be the default sort order of the menu items. Do we need to account for adding that functionality as well, or is it already possible with the current Menu? cc: @AAlhazwani-WMF
to provide more context.

The Lookup component completely outsources the contents of the menu to the parent/user component (in this case, the Wikifunctions component that uses Lookup). When the user types something, Lookup emits and event, and expects the parent component to respond by providing it with menu items. So Wikifunctions (or whatever else uses Lookup) has complete control over which items appear initially (when no text is entered), which items appear in response to what user input, how many of them there are, and what order they appear in.

This is also why I think #3 might make sense (I just corrected a typo in my earlier comment, I meant to say I prefer #2 over #1, not #2 over #3). I think we may be able to always show the menu on focus, because the code using the Lookup component could effectively disable that behavior by setting the list of menu items to empty when there is no user input (empty menus are most displayed), and most probably already do.

The Lookup component completely outsources the contents of the menu to the parent/user component (in this case, the Wikifunctions component that uses Lookup). When the user types something, Lookup emits and event, and expects the parent component to respond by providing it with menu items. So Wikifunctions (or whatever else uses Lookup) has complete control over which items appear initially (when no text is entered), which items appear in response to what user input, how many of them there are, and what order they appear in.

👍

This is also why I think #3 might make sense (I just corrected a typo in my earlier comment, I meant to say I prefer #2 over #1, not #2 over #3). I think we may be able to always show the menu on focus, because the code using the Lookup component could effectively disable that behavior by setting the list of menu items to empty when there is no user input (empty menus are most displayed), and most probably already do.

Got it, so #3 offers the configurability of #2 but without a prop. That's a bit less obvious and might be a bit more challenging to document, but it seems "cleaner". Would this be a breaking change though?

This is also why I think #3 might make sense (I just corrected a typo in my earlier comment, I meant to say I prefer #2 over #1, not #2 over #3). I think we may be able to always show the menu on focus, because the code using the Lookup component could effectively disable that behavior by setting the list of menu items to empty when there is no user input (empty menus are most displayed), and most probably already do.

Got it, so #3 offers the configurability of #2 but without a prop. That's a bit less obvious and might be a bit more challenging to document, but it seems "cleaner". Would this be a breaking change though?

That's the thing, I think it technically might be. If you're a parent component using Lookup, I think you probably don't have to worry about explicitly setting the menu items list back to empty when the user clears the input, because the menu would be hidden anyway; but with this change, if you don't explicitly clear the menu, it'll stay open (with the previous set of menu items) when the user clears the input. I think we'd have to look at whether our documentation examples do that, how many real-world uses of Lookup do that, and whether not doing it causes other bugs currently (e.g. if code written in that style results in the old items appearing as soon as the user types, that would already be a bug).

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

[design/codex@main] Lookup: Enable an initial list of suggestions

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

In the patch linked above, I've proposed another option: if an initial list of menu items is passed into the Lookup, it will open the menu on focus, and keep the menu open when the input is cleared. This enables the parent component to show a list of options initially, and to show them again when the input is cleared. Here's a demo.

This covers the use case described in this task and doesn't affect existing implementations of Lookup, but I'm concerned that it makes too many assumptions about the desired behavior when an initial list of menu items is provided. Are there cases where this behavior would not be ideal?

IMO, the alternative to my proposal is to do option 3 that @Catrope described above. I'd like to avoid this since it breaks some existing implementations of Lookup, but I do think it's a cleaner approach that forces the value of the menuItems prop always to match the desired menu behavior (i.e. open the menu when there are items, close it otherwise)

In the patch linked above, I've proposed another option: if an initial list of menu items is passed into the Lookup, it will open the menu on focus, and keep the menu open when the input is cleared. This enables the parent component to show a list of options initially, and to show them again when the input is cleared. Here's a demo.

This covers the use case described in this task and doesn't affect existing implementations of Lookup, but I'm concerned that it makes too many assumptions about the desired behavior when an initial list of menu items is provided. Are there cases where this behavior would not be ideal?

IMO, the alternative to my proposal is to do option 3 that @Catrope described above. I'd like to avoid this since it breaks some existing implementations of Lookup, but I do think it's a cleaner approach that forces the value of the menuItems prop always to match the desired menu behavior (i.e. open the menu when there are items, close it otherwise)

I like this solution and I think the assumptions make sense. Is there a way to do a code search for examples of where an initial list of menu items is provided?

Also pinging @AAlhazwani-WMF @Jdforrester-WMF for feedback.

I like this solution and I think the assumptions make sense. Is there a way to do a code search for examples of where an initial list of menu items is provided?

@CCiufo-WMF yes, I looked at the existing implementations of Lookup found in code search - all of them initially set menuItems to an empty array. There are a couple of implementations that do not reset menuItems to an empty array when the input is cleared, so if we went with option 3, we'd need to update these.

This is brilliant, thank you for this excellent idea @AnneT! It's the best of both worlds, because:

  • It doesn't add an ugly boolean prop that toggles the behavior; instead it uses the presence/absence of initial menu items as a proxy for this
  • Old code keeps working the way it did, even if it doesn't clear the menuItems when the input is cleared (except if it sets menuItems to a non-empty array initially, but nobody does that currently)
  • New code intending to use this feature is written in an intuitive way, and looks exactly the same as it would if we had implemented my suggestions
  • New code that doesn't want to use this feature still has an intuitive path to doing so, because it set menuItems to empty initially and then populate it when something is typed, and/or clear menuItems when it wants to hide the menu

Change 1003444 merged by jenkins-bot:

[design/codex@main] Lookup: Enable an initial list of suggestions

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

Thank you @AnneT for solving this! Reviewing this new demo, it seems that there is an error to fix when the menu is displayed and there are not results to show. In this case, the menu should display the No results content but now just a short empty menu is displayed.

Captura de pantalla 2024-02-16 a las 12.23.30.png (198×1 px, 20 KB)


Apart from this, since this is a new use case we are including in the Lookup component, we will need to:

  • Update the Figma component to include it and represent its use case in the Figma spec.
  • Provide design Guidelines about this use case and the suggested items (e.g. how many items is it recommended to display?)

So I'm going to work on this and once I provide the text and image for the guidelines, we will need to include it in the Codex site @CCiufo-WMF.

Also keep in mind that the Menu Header (to use in the "Suggested items") is not implemented a Codex menu variant yet T305036: Menu: Support headers for grouping options.

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

[design/codex@main] docs: Add "no results" message to Lookup demo

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

Thank you @AnneT for solving this! Reviewing this new demo, it seems that there is an error to fix when the menu is displayed and there are not results to show. In this case, the menu should display the No results content but now just a short empty menu is displayed.

Oh good catch @bmartinezcalvo! I went ahead and pushed a patch to fix the issue with this demo by adding a "no results" message. The Wikifunctions use case already has a "no results" message so it'll work for them. That said, we should create a more flexible/robust fix for this for the general initial suggestions use case. I'll open a task for that next week but I wanted to fix the demo before the release next week (since I'm out today and Monday)

Also keep in mind that the Menu Header (to use in the "Suggested items") is not implemented a Codex menu variant yet T305036: Menu: Support headers for grouping options.

The current demo just fakes this, the "Suggested items" header is actually just a disabled menu item.

Change 1004194 merged by jenkins-bot:

[design/codex@main] docs: Add "no results" message to Lookup demo

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

Change 1005171 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] Update Codex from v1.3.2 to v1.3.3

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

Change 1005171 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v1.3.2 to v1.3.3

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