In T300625#7770529, @AnneT wrote:

Re: the selection behavior, the reason we're selecting the menu item on keyboard highlight is so that, if the user hits enter, we have a way of telling, in the onSubmit handler, which item was selected for instrumentation purposes. We can't rely on the highlighted item because it's cleared when the menu is closed.

If the user hits enter, doesn't that cause the highlighted menu to be selected before the menu is closed? I don't know what sequence these events happen in exactly, but it seems to me that either the submit event happens before the menu closes, in which case the relevant menu item should still be highlighted, or it happens after the menu closes, in which case the highlight will have been cleared but the item should have been selected before the menu closed. Am I missing something?

Sorry, submitted too soon:

What I was trying to get at with the comment above is whether we can disable the "select the highlighted item on keyboard navigation" behavior (which would fix this issue) without causing other issues.

Ahh, you're totally right. We don't need to select the item when it's highlighted, because it becomes selected when the user hits enter. I can confirm that this happens before the submit handler.

The one thing we'll need to retain is updating the inputValue on keyboard highlight. This happens as a side effect of selection with the current selectHighlighted setup, so we'll just need to change the behavior to specifically update the inputValue.

We're kind of going back into a circle we were in when initially developing TypeaheadSearch, where we were updating the inputValue within the useMenu composable, meaning you had to pass it to useMenu, which is less ideal than an explicit boolean prop like selectHighlight. We'll have to consider how best to update the code to adjust this behavior.

Is merging of https://gerrit.wikimedia.org/r/c/design/codex/+/770014 blocked until this issue is resolved, or should this be handled in a follow-up patch? I assume the latter but just want to make sure.

I think the latter is fine; this behavior already exists on the main branch and we can easily remove selectHighlighted from the menu config type/prop in a future patch.

I think I was unclear on the desired UX when we were working on selectHighlighted – we actually never want something to be selected and highlighted at the same time due to keyboard navigation, correct?

Right now the Menu component contains a handleHighlight method that looks like this :

/**
 * Handle changes related to highlighting a new item.
 *
 * @param newHighlightedMenuItem
 */
function handleHighlight( newHighlightedMenuItem?: MenuItemDataWithId ) {
	if ( !newHighlightedMenuItem ) {
		return;
	}

	// Change menu state.
	handleMenuItemChange( 'highlighted', newHighlightedMenuItem );

	if ( props.selectHighlighted ) {
		emit( 'update:selected', newHighlightedMenuItem.value );
	}
}

This method only gets called during keyboard navigation. If the selectHighlight prop has been set to true, an "update:selected" event gets fired and we see the incorrect behavior described above.

Here's an idea. This behavior is specific to Typeahead, correct? Since there are already a lot of props that Menu has to handle, maybe a better solution would be to emit a "menuKeyboardNavigation" event instead of "update:selected". We could remove the conditional (and the selectHighlighted prop itself) and just emit this event in all circumstances; Typeahead could listen for this event to update the input value, while other Menu-embedding components could just ignore it.

I can open a patch for this shortly, just want to make sure I understand the use-case correctly.

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

[design/codex@main] Menu, TypeaheadSearch: Remove selectHighlighted prop

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

Change 770074 merged by jenkins-bot:

[design/codex@main] Menu, TypeaheadSearch: Remove selectHighlighted prop

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

Signing off because menu items now display the right visual styles when interacted with via keyboard.