V9 Tooltip should stop event propagation on escape
Similar to #14465, tooltips on focus targets in surfaces (modal, menu etc) should stop the propagation of the escape key so that the parent is not also dismissed.
v9 example of current behavior
Dismissing a tooltip on a button in a menu, also closed the menu https://codesandbox.io/s/hopeful-fast-9qrbbj
v8 example of expected/past behavior
Dismissing the tooltip on the button in a modal does not close the modal https://codesandbox.io/s/romantic-cerf-cw85t9?file=/src/App.tsx
My assumption here is:
Tooltip component attaches the keydown listener to the document when MenuPopover attaches the event listener to the div element with role presentation.
So, it looks like the event from MenuPopover executes first and then Tooltip actions, as a result MenuPopover closes everything. The old version (v8) of Tooltip attaches event to the direct componet instead of attaching keydown listener to the document.
There are a few approaching of fixing it:
- Move
Tooltipkeydown listener fromdocumentto thechildrencomponent - Change
const onDocumentKeyDown = (ev: KeyboardEvent) => {
if (ev.key === 'Escape' || ev.key === 'Esc') {
thisTooltip.hide();
ev.stopPropagation();
}
};
targetDocument?.addEventListener('keydown', onDocumentKeyDown);
return () => {
if (context.visibleTooltip === thisTooltip) {
context.visibleTooltip = undefined;
}
targetDocument?.removeEventListener('keydown', onDocumentKeyDown);
};
to
const onDocumentKeyDown = (ev: KeyboardEvent) => {
if (ev.key === 'Escape' || ev.key === 'Esc') {
thisTooltip.hide();
ev.stopPropagation(); <--------
}
};
targetDocument?.addEventListener('keydown', onDocumentKeyDown, true); <--------
return () => {
if (context.visibleTooltip === thisTooltip) {
context.visibleTooltip = undefined;
}
targetDocument?.removeEventListener('keydown', onDocumentKeyDown, true); <--------
};
packages\react-components\react-tooltip\src\components\Tooltip\useTooltip.tsx The number two looks easier than the first one because it is hard to attach an event listener to the children + there are a few more approaches to fix it which maybe I do not see now.
I hope my research of the issue will help somebody :)
Probably related to this issue: when tabbing and you come to a component with a tooltip, if esc is pressed to close the tooltip, the component you were focused on also loses focus. Here's a video of this behavior on the v9 playground page. In the video, I hit tab to focus on the button and then esc to close the tooltip.
Untitled video - Screen Recording - 10_14_2022, 3_17_11 PM.webm
I actually soft-disagree with the proposal to stop propagation of escape for tooltips for a few reasons:
- A user may be unaware they have a tooltip open, either because they're using a screen reader, or they're just not paying attention to the tiny popup. In these cases, it would be weird to have escape not dismiss the primary popup the user is interacting with.
- Tooltips need to be dismissable regardless of where keyboard focus is (i.e. hover tooltips when focus is elsewhere). This needs to happen without moving the mouse cursor, so essentially the keyboard dismissal needs to work with hover tooltips. If we choose escape for this, accidentally hovering over a thing with a tooltip means that escape doesn't dismiss your dialog (or menu, callout, etc.).
- In a user study, I've found people are conservative about using escape to dismiss tooltips when they are within dialogs and other popups.
I'd recommend we let escape propagate from the tooltip, and use the control key and shift + F11 (or whatever that function key was) as the alternate means to dismiss tooltips without also dismissing the popup.
This already fixed with this commit https://github.com/microsoft/fluentui/commit/81de40f0df0dfa8bac4e4fb522bf830d9fe8e066