Change 209167 had a related patch set uploaded (by Catrope):
Add user name suggestions from the API in the mention inspector

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

Change 209168 had a related patch set uploaded (by Catrope):
[WIP] MentionTargetInputWidget: Segment the suggestions list

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

I went ahead and implemented this, but I wasn't quite sure what to do with the suggestions based on the users that have already commented on the topic vs suggestions from the API. The topic suggestions are helpful because they are more contextual (you're likely to mention someone already active in the discussion) and they are more searchable (we can and do easily search in any part of the user name, whereas the API only allows prefix searches). So I kept both kinds, and separated the list of users into two sections. Because this separation (and its labeling) requires more product and design input than the rest of the change, I've separated it into its own patch: https://gerrit.wikimedia.org/r/209168

It looks like this:

The idea is to show a list with the participants in the conversation as an initial suggestion when the user has not typed anything. That anticipates the user intent and makes the selection immediate if those are the ones the user is looking for.

Once the user types "R", the intent is to locate a specific user (probably not one of the initial list) so the list should contain only those users matching the search criteria (starting with R).

I see the following valid options:

1. Keep the two sections separated but showing only usernames starting with "R".
   • Pros. For lists with many participants, it may be helpful to locate that participant using filtering but with less chances to pick an unrelated user with a similar name.
   • Cons. Adds some complexity and takes additional space. We need to take care of edge cases (e.g., not show any section headers when the "participants" list is empty).
2. Show a single list of users starting with "R" surfacing those in the conversation at the top of the list.
   • Pros. Helps with the above case (lots of participants in the conversation where filtering can be useful) by making the most likely names easier to find.
   • Cons. Reordering breaks the alphabetical order you could expect without any clear reason of why.
3. Show a single list of users starting with "R"
   • Pros. Simplest approach, you typed for "R" and you get the users starting with "R".
   • Cons. The initial listing becomes the only help to quickly select participants of the conversation.

From your image, it seems that the participants in the conversation are kept despite not matching the search criteria. In this context that seems to get in the way of the results the user is looking for. So I would not recommend that option.

Considering that we have not experienced how well the initial list solves the case of conversations with many participants, I'm totally ok to start simply with a flat list (option 3).
If the effort to extend the current patchset to support option 1 is really minimal, it will be an improvement worth doing. Here are some of the aspects to check in that case:

• The initial list of participants, does not have a heading separation.
• Once the user starts typing, only results that match the search query appear.
  • If there are results both for participants and other users, both headers appear. The same user should appear only in one of those lists, no duplicates.
  • If there are only results from the "participants", then the "participants" heading should be visible but not the "other users" one.
  • If there are results from the "other users" but not the "participants" one, no heading will be shown.

I think participants in the current topic already get a notification (unless they explicitly unwatched the topic) when you post.

So arguably, the other one (all users on wiki) is a lot more useful (bringing someone into the conversation), and maybe the current-topic one could be removed. However, it could still be useful if you just want to visually flag who (of the existing participants) you're talking to.

In T93421#1264080, @Pginer-WMF wrote:

From your image, it seems that the participants in the conversation are kept despite not matching the search criteria. In this context that seems to get in the way of the results the user is looking for. So I would not recommend that option.

That's not entirely true. In my image, "Catrope" was shown because I typed "R", and "Catrope" contains an "R". As I said, it does substring matches on topic users and prefix matches on other users. I think you're right that that's probably too confusing and we should just stick with prefix matches.

I'll amend the patch to show users from the topic when the input is empty, and only show users matching the prefix when the input is nonempty (option 3). Then we can get rid of the segmentation too.

Change 209168 abandoned by Catrope:
MentionTargetInputWidget: Segment the suggestions list

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

In T93421#1267030, @Catrope wrote:

I'll amend the patch to show users from the topic when the input is empty, and only show users matching the prefix when the input is nonempty (option 3). Then we can get rid of the segmentation too.

Done. It's now a simple prefix search except when the search field is empty, in which case the list of users active on the topic is displayed.

That's not entirely true. In my image, "Catrope" was shown because I typed "R", and "Catrope" contains an "R". As I said, it does substring matches on topic users and prefix matches on other users.

Sorry, I missed that part.

I tried the patchset and it works great. I have some comments that probably deserve separate tickets (I created some):

• The input was missing a placeholder (I created T98452 for this).
• The input field turns into red too early. I just open the inspector and it is already telling me I did something wrong by keeping the field empty. I would expect that to happen when the focus moves away from the input field but not when I'm focused on the input field about to type.
• I can add mentions to non-existing users. If the user I'm typing does not exist, I'd would expect the field to turn red and the "add" button to be deactivated (I may be missing some specific case where that could be useful). This and the above point are covered by T98455.
• For wikis where in addition to the username, they use the "Real Name" are we also matching against those values? I think it is totally fine to just focus on usernames (since those are the ones shown in Flow) but we may want to think how to support them in this inspector if we plan to provide some kind of support for them in Flow in the future.

In T93421#1268268, @Pginer-WMF wrote:

That's not entirely true. In my image, "Catrope" was shown because I typed "R", and "Catrope" contains an "R". As I said, it does substring matches on topic users and prefix matches on other users.

Sorry, I missed that part.

I tried the patchset and it works great. I have some comments that probably deserve separate tickets (I created some):

• The input was missing a placeholder (I created T98452 for this).

Good catch, I'll fix that.

• The input field turns into red too early. I just open the inspector and it is already telling me I did something wrong by keeping the field empty. I would expect that to happen when the focus moves away from the input field but not when I'm focused on the input field about to type.

I believe that's T98202: [Regression pre-wmf5] Link inspector should not have red highlight when still empty (I know that sounds like a VE bug, but it's the same underlying OOUI component that's at fault).

• I can add mentions to non-existing users. If the user I'm typing does not exist, I'd would expect the field to turn red and the "add" button to be deactivated (I may be missing some specific case where that could be useful). This and the above point are covered by T98455.

Good point. Note though, that the flip side of this is that clicking "Insert" won't be immediate, because it takes time to determine whether the user name you typed exists. If you chose the name from a suggestion list then we'll already know and it'll be instantaneous, but if you rapidly type "Foobar[ENTER]", it'll take about half a second before the inspector has finished validating the username so it can either close and insert the mention or yell at you.

The current validity check only tests that the user name is one that could possibly be created (i.e. doesn't contain weird characters like | and doesn't start with things like Talk:), which is based on simple rules that we can check instantaneously on the client, so extending this will take a little bit of work. It being async isn't necessarily bad, but I wanted you to know what the implications are before I do it :)

• For wikis where in addition to the username, they use the "Real Name" are we also matching against those values? I think it is totally fine to just focus on usernames (since those are the ones shown in Flow) but we may want to think how to support them in this inspector if we plan to provide some kind of support for them in Flow in the future.

I don't think real names are available in any way yet. Bugs like T90055: [Spike] Changing the displayed username, in Flow and T95759: Auto-complete for alternate names in Flow mentions are still open.

In T93421#1269776, @Catrope wrote:

Good point. Note though, that the flip side of this is that clicking "Insert" won't be immediate, because it takes time to determine whether the user name you typed exists. If you chose the name from a suggestion list then we'll already know and it'll be instantaneous, but if you rapidly type "Foobar[ENTER]", it'll take about half a second before the inspector has finished validating the username so it can either close and insert the mention or yell at you.

Yes, but this half-second is well worth it, since the mention is mostly useless (doesn't trigger an Echo ping) if it's even slightly mis-spelled.

This was already implemented, but there's evidently a regression. Tracked as T98455: Mention inspector validation needs adjustment to trigger when is needed to avoid premature warnings and mentioning inexistent users

The current validity check only tests that the user name is one that could possibly be created (i.e. doesn't contain weird characters like | and doesn't start with things like Talk:), which is based on simple rules that we can check instantaneously on the client, so extending this will take a little bit of work. It being async isn't necessarily bad, but I wanted you to know what the implications are before I do it :)

What current check are you referring to? The one in the code still looks like the one I implemented, where this worked.

This was probably broken by an OOjs UI change to how validation works.

In T93421#1269800, @Mattflaschen wrote:

What current check are you referring to? The one in the code still looks like the one I implemented, where this worked.

This was probably broken by an OOjs UI change to how validation works.

Sorry, I meant "current" as in "currently existing in my patchset", not "current as in existing in master". I see how that was confusing because you'd normally expect the word "current" to mean the latter.

I'll restore this behavior and make it work better with whatever OOUI changes might have occurred.

In T93421#1269800, @Mattflaschen wrote:

What current check are you referring to? The one in the code still looks like the one I implemented, where this worked.

This was probably broken by an OOjs UI change to how validation works.

Sorry, I meant "current" as in "currently existing in my patchset", not "current as in existing in master". I see how that was confusing because you'd normally expect the word "current" to mean the latter.

I'll restore this behavior and make it work better with whatever OOUI changes might have occurred.

I have now updated my patch to restore the behavior that T98455 asks for. I haven't yet written a patch to fix the validation regression in master.

In T93421#1271247, @Catrope wrote:

I have now updated my patch to restore the behavior that T98455 asks for. I haven't yet written a patch to fix the validation regression in master.

It turns out this was never broken in master, it was just broken in an earlier version of my patch.

Change 209167 merged by jenkins-bot:
Add user name suggestions from the API in the mention inspector

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