Change 413287 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[oojs/ui@master] MenuTagMultiselectWidget: Highlight first item when filtering

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

@matmarex this was a little harder than I thought, and exposed another potential bug in MenuSelectWidget with the config variable "highlightOnFilter". Supposedly, "highlightOnFilter" should do exactly this behavior, but it doesn't work right now. I tried to fix it in MenuSelectWidget, but the cross-events that trigger 'updateItemVisibility' (on key strokes, on changing of input, etc) interfere with this behavior, and make the selection jump up and be unsynchronized with the arrow movement.

The 'highlightOnFilter' behavior does work on mixin.LookupElement, so I tried to follow the same process, but LookupElement calls the highlighting functionality from outside MenuSelectWidget not inside it, which means that it bypasses all the potential cross-events that are triggered (or rather, it waits until they're all over). This is why in this case I also put the behavior in MenuTagMultiselectWidget rather than inside MenuSelectWidget itself.

While this works, there are a few questions to follow up or decide on with this patch:

• Should this behavior be cancellable with a config variable? It seems at the very least it should be default to MenuTagMultiselectWidget; in RCFilters we override the entire item-visibility-related events completely, since we're using a model/controller to set it up, so RCFilter is really an edge case here, and would have overriden these methods regardless.
• Whatever happens in this fix, 'highlightOnFilter' on MenuSelectWidget is broken right now. I couldn't manage to directly fix it without completely changing the way the events work, and almost rewriting the widget. My approach to a fix would have been separating, at the very least, the event response to editing the input with the events related to keystroke, but that change is much more nuanced, since some of those are not really distinct as we want them to be. I didn't want to get too much into this without consulting first, though. So -- do we cancel the 'highlightOnFilter' config as non working, or should we rewrite the functionality to try and decouple events and make it work?

In T187398#3991636, @Mooeypoo wrote:

• Whatever happens in this fix, 'highlightOnFilter' on MenuSelectWidget is broken right now. I couldn't manage to directly fix it without completely changing the way the events work, and almost rewriting the widget. My approach to a fix would have been separating, at the very least, the event response to editing the input with the events related to keystroke, but that change is much more nuanced, since some of those are not really distinct as we want them to be. I didn't want to get too much into this without consulting first, though. So -- do we cancel the 'highlightOnFilter' config as non working, or should we rewrite the functionality to try and decouple events and make it work?

To be more specific, calling updateItemVisibility on keystroke "up" / "down" / "left" or "right" seems redundant and unnecessary. Item visibility only changes if the filtering changed, so it should really be only called on actual change of the input -- either 'change' event and/or detecting copy/paste. But currently, the updateItemVisibility seems to conflate those two actions (keyboard action that changes input and hence changes the visible items, and keyboard action that doesn't) and so attaching any sort of highlighting action is also conflated; it means that the code tries to "highlight first item" both when the items changed ( = good) and when the user hits the arrow buttons ( = bad)

This conflation of events, in my opinion, should be untangled regardless of having this feature. It will also allow any widget that extends MenuSelectWidget to choose whether they extend the editing of the input functionality, or the arrowkey / escape/etc functionality separately. I almost insisted on doing that because RCFilters needed that difference, but then RCFilters started using its entire filtering (and display and highlight) logic through the data model and controller, so it made the internal widget operation change moot -- but I think it's still very confusing and should be untangled regardless.

I think it would be reasonable to remove the broken 'highlightOnFilter' option.

Change 413287 merged by jenkins-bot:
[oojs/ui@master] MenuTagMultiselectWidget: Highlight first item when filtering

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