Tooltip bugs (adobe#1087)

Author: snowystinger

packages/@react-aria/tooltip/src/useTooltipTrigger.ts

@@ -11,7 +11,7 @@
  */
 
 import {FocusEvents} from '@react-types/shared';
-import {HoverProps, isFocusVisible, PressProps, usePress} from '@react-aria/interactions';
+import {getInteractionModality, HoverProps, isFocusVisible, PressProps, usePress} from '@react-aria/interactions';
 import {HTMLAttributes, RefObject, useEffect, useRef} from 'react';
 import {mergeProps, useId} from '@react-aria/utils';
 import {TooltipTriggerProps} from '@react-types/tooltip';
@@ -35,7 +35,9 @@ export function useTooltipTrigger(props: TooltipTriggerProps, state: TooltipTrig
   let isFocused = useRef(false);
 
   let handleShow = () => {
-    state.open(isFocused.current);
+    if (isHovered.current || isFocused.current) {
+      state.open(isFocused.current);
+    }
   };
 
   let handleHide = () => {
@@ -63,28 +65,43 @@ export function useTooltipTrigger(props: TooltipTriggerProps, state: TooltipTrig
   }, [ref, state]);
 
   let onHoverStart = () => {
-    isHovered.current = true;
+    // In chrome, if you hover a trigger, then another element obscures it, due to keyboard
+    // interactions for example, hover will end. When hover is restored after that element disappears,
+    // focus moves on for example, then the tooltip will reopen. We check the modality to know if the hover
+    // is the result of moving the mouse.
+    if (getInteractionModality() === 'pointer') {
+      isHovered.current = true;
+    } else {
+      isHovered.current = false;
+    }
     handleShow();
   };
+
   let onHoverEnd = () => {
+    // no matter how the trigger is left, we should close the tooltip
+    isFocused.current = false;
     isHovered.current = false;
     handleHide();
   };
+
   let onPressStart = () => {
-    if (isFocused.current) {
-      isFocused.current = false;
-    }
+    // no matter how the trigger is pressed, we should close the tooltip
+    isFocused.current = false;
+    isHovered.current = false;
     handleHide();
   };
+
   let onFocus = () => {
     let isVisible = isFocusVisible();
     if (isVisible) {
       isFocused.current = true;
       handleShow();
     }
   };
+
   let onBlur = () => {
     isFocused.current = false;
+    isHovered.current = false;
     handleHide();
   };
 

packages/@react-spectrum/tooltip/stories/TooltipTrigger.stories.tsx

@@ -11,7 +11,7 @@
  */
 
 import {action} from '@storybook/addon-actions';
-import {ActionButton} from '@react-spectrum/button';
+import {ActionButton, Button} from '@react-spectrum/button';
 import {Flex} from '@react-spectrum/layout';
 import React, {useState} from 'react';
 import {storiesOf} from '@storybook/react';
@@ -86,6 +86,20 @@ storiesOf('TooltipTrigger', module)
         </TooltipTrigger>
       </div>
     )
+  )
+  .add(
+    'tooltip with other hoverables',
+    () => (
+      <Flex gap="size-100">
+        <TooltipTrigger onOpenChange={action('openChange')}>
+          <ActionButton>Trigger Tooltip</ActionButton>
+          <Tooltip>
+            Long tooltip message that just goes on and on.
+          </Tooltip>
+        </TooltipTrigger>
+        <Button variant="secondary">No Tooltip</Button>
+      </Flex>
+    )
   );
 
 function render(props = {}) {

packages/@react-spectrum/tooltip/test/TooltipTrigger.test.js

@@ -16,6 +16,7 @@ import {Provider} from '@react-spectrum/provider';
 import React from 'react';
 import {theme} from '@react-spectrum/theme-default';
 import {Tooltip, TooltipTrigger} from '../';
+import {triggerPress} from '@react-spectrum/test-utils';
 
 // Sync with useTooltipTriggerState.ts
 const TOOLTIP_DELAY = 1500;
@@ -87,6 +88,8 @@ describe('TooltipTrigger', function () {
           </TooltipTrigger>
         </Provider>
       );
+      fireEvent.mouseDown(document.body);
+      fireEvent.mouseUp(document.body);
 
       let button = getByLabelText('trigger');
       fireEvent.mouseEnter(button);
@@ -104,7 +107,7 @@ describe('TooltipTrigger', function () {
       expect(tooltip).not.toBeInTheDocument();
     });
 
-    it('if hovered and focused, will not hide if hover leaves',  () => {
+    it('if hovered and focused, will hide if hover leaves',  () => {
       let {getByRole, getByLabelText} = render(
         <Provider theme={theme}>
           <TooltipTrigger onOpenChange={onOpenChange} delay={0}>
@@ -133,11 +136,11 @@ describe('TooltipTrigger', function () {
 
       // remove hover
       fireEvent.mouseLeave(button);
-      expect(onOpenChange).toHaveBeenCalledTimes(1);
+      expect(onOpenChange).toHaveBeenCalledTimes(2);
       act(() => {
         jest.runAllTimers();
       });
-      expect(tooltip).toBeVisible();
+      expect(tooltip).not.toBeInTheDocument();
 
       // remove focus
       act(() => {
@@ -151,7 +154,7 @@ describe('TooltipTrigger', function () {
       expect(tooltip).not.toBeInTheDocument();
     });
 
-    it('if hovered and focused, will not hide if focus leaves',  () => {
+    it('if hovered and focused, will hide if focus leaves',  () => {
       let {getByRole, getByLabelText} = render(
         <Provider theme={theme}>
           <TooltipTrigger onOpenChange={onOpenChange} delay={0}>
@@ -182,11 +185,11 @@ describe('TooltipTrigger', function () {
       act(() => {
         button.blur();
       });
-      expect(onOpenChange).toHaveBeenCalledTimes(1);
+      expect(onOpenChange).toHaveBeenCalledTimes(2);
       act(() => {
         jest.runAllTimers();
       });
-      expect(tooltip).toBeVisible();
+      expect(tooltip).not.toBeInTheDocument();
 
       // remove hover
       fireEvent.mouseLeave(button);
@@ -241,6 +244,8 @@ describe('TooltipTrigger', function () {
           <input type="text" />
         </Provider>
       );
+      fireEvent.mouseDown(document.body);
+      fireEvent.mouseUp(document.body);
 
       let button = getByLabelText('trigger');
       let input = getByRole('textbox');
@@ -278,16 +283,15 @@ describe('TooltipTrigger', function () {
           </TooltipTrigger>
         </Provider>
       );
+      fireEvent.mouseDown(document.body);
+      fireEvent.mouseUp(document.body);
 
       let button = getByLabelText('trigger');
-      act(() => {
-        button.focus();
-      });
+      fireEvent.mouseEnter(button);
       expect(onOpenChange).toHaveBeenCalledWith(true);
       let tooltip = getByRole('tooltip');
       expect(tooltip).toBeVisible();
-      fireEvent.mouseDown(document.activeElement, {button: 0});
-      fireEvent.mouseUp(document.activeElement, {button: 0});
+      triggerPress(button);
       expect(onOpenChange).toHaveBeenCalledWith(false);
       act(() => {
         jest.advanceTimersByTime(CLOSE_TIME);
@@ -303,6 +307,39 @@ describe('TooltipTrigger', function () {
     });
   });
 
+  it('is closed if the trigger is clicked with the keyboard',  () => {
+    let {getByRole, getByLabelText} = render(
+      <Provider theme={theme}>
+        <TooltipTrigger onOpenChange={onOpenChange} delay={0}>
+          <ActionButton aria-label="trigger" />
+          <Tooltip>Helpful information.</Tooltip>
+        </TooltipTrigger>
+      </Provider>
+    );
+
+    let button = getByLabelText('trigger');
+    act(() => {
+      button.focus();
+    });
+    expect(onOpenChange).toHaveBeenCalledWith(true);
+    let tooltip = getByRole('tooltip');
+    expect(tooltip).toBeVisible();
+    fireEvent.keyDown(button, {key: 'Enter'});
+    fireEvent.keyUp(button, {key: 'Enter'});
+    expect(onOpenChange).toHaveBeenCalledWith(false);
+    act(() => {
+      jest.advanceTimersByTime(CLOSE_TIME);
+    });
+    expect(tooltip).not.toBeInTheDocument();
+    act(() => {
+      button.blur();
+    });
+    act(() => {
+      jest.runAllTimers();
+    });
+    expect(() => getByRole('tooltip')).toThrow();
+  });
+
   describe('delay', () => {
     it('opens immediately for focus',  () => {
       let {getByRole, getByLabelText} = render(
@@ -340,6 +377,8 @@ describe('TooltipTrigger', function () {
           </TooltipTrigger>
         </Provider>
       );
+      fireEvent.mouseDown(document.body);
+      fireEvent.mouseUp(document.body);
 
       let button = getByLabelText('trigger');
       fireEvent.mouseEnter(button);
@@ -439,6 +478,8 @@ describe('TooltipTrigger', function () {
           </TooltipTrigger>
         </Provider>
       );
+      fireEvent.mouseDown(document.body);
+      fireEvent.mouseUp(document.body);
 
       let button = getByLabelText('trigger');
       fireEvent.mouseEnter(button);
@@ -701,6 +742,7 @@ describe('TooltipTrigger', function () {
       });
       expect(getByText(helpfulText)).toBeVisible();
 
+      fireEvent.mouseMove(document.body);
       // we've used focus to open one, now we can use hover to try and open a second one
       fireEvent.mouseEnter(badButton);
       fireEvent.mouseMove(badButton);
@@ -728,6 +770,7 @@ describe('TooltipTrigger', function () {
       let badButton = getByLabelText('bad-trigger');
       expect(getByText(helpfulText)).toBeVisible();
 
+      fireEvent.mouseMove(document.body);
       // open a second one through interaction
       fireEvent.mouseEnter(badButton);
       fireEvent.mouseMove(badButton);
@@ -786,6 +829,7 @@ describe('TooltipTrigger', function () {
           </TooltipTrigger>
         </Provider>
       );
+      fireEvent.mouseMove(document.body);
       let button = getByLabelText('trigger');
       fireEvent.mouseEnter(button);
       fireEvent.mouseMove(button);