fix(react-menu): Improving user experience by fixing a hoverDelay + click race condition on MenuTrigger

[stevencomsft]

Fixing issue where menu item trigger click could have race condition with hover open and cause the menu to close immediately right as it is opening.

Previous Behavior

With a hover delay set on Menu (default is 500ms), a user can encounter (frequently) a race condition where they start to hover a menu trigger, which sets a timeout of 500ms that starts to open the popover after the delay. In a lot of scenarios, our users wait between 500-700ms to click because they think the hover might not happen, but this causes a race condition where the popover gets set to open right as they click, and then the click ends up closing the popover instantly, which is not what the user expects. Logically this makes sense, but feels like a bug to our users.

CurrMenuHoverClickRace.mp4

New Behavior

Prevent click-closing the menu while the hover open animation is occurring (after the initial delay). This allows users to click the menu trigger while it has just started opening, before human reaction speed is able to tell the popover hover open has occurred. This feels more natural to the end user.

NewMenuHoverClickRace.mp4

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[stevencomsft]

@ling1726 I hear you have a lot of experience with Menu - was hoping to get your review/opinion on this change :)

Wondering your thoughts on having this ~200ms delay padding for the popover animation hardcoded and possibly out of sync with the actual animation.

basically the idea here is that the default 500ms delay for hover is long enough that we see a lot of users click just after the 500ms, between that delay and when the popover is mostly done animating, due to slow reaction times. So we have a lot of examples of users clicking between 500ms and 900ms after hovering and that causes the popover to immediately close, so ideally we would want to make this a better experience by preventing the click from overriding the hover open before the human brain is able to see and process the popover is actually opening

[ling1726]

Can you please provide some repro steps to this in the PR description?

I don't quite understand the root cause of the problem

which sets the internal open state of the menu to open without showing the menu as open visually for 500ms

This statement is incorrect, when the user hovers over a trigger, the open state is not changed immediately, the update happens in a timeout

fluentui/packages/react-components/react-menu/src/components/Menu/useMenu.tsx

Lines 213 to 224 in 84ccf12

	if (e.type === 'mouseleave' || e.type === 'mouseenter' || e.type === 'mousemove' || e.type === MENU_ENTER_EVENT) {
	if (state.triggerRef.current?.contains(e.target as HTMLElement)) {
	enteringTriggerRef.current = e.type === 'mouseenter' || e.type === 'mousemove';
	}
	// FIXME leaking Node timeout type
	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
	// @ts-ignore
	setOpenTimeout.current = setTimeout(() => trySetOpen(e, data), state.hoverDelay);
	} else {
	trySetOpen(e, data);
	}

[stevencomsft]

@ling1726 I made some changes after going back through the logic you quoted and missed that originally. updated the comments, description, and added some screen recordings :)