fix(tooltip): ensure esc closes tooltip, avoid bubbling beyond the bo…

…dy (carbon-design-system#10220) Co-authored-by: Abbey Hart <[email protected]>
tay1orjones · Dec 3, 2021 · 48cec74 · 48cec74
1 parent 505af19
commit 48cec74
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 29 deletions.
diff --git a/packages/react/src/components/Modal/Modal-story.js b/packages/react/src/components/Modal/Modal-story.js
@@ -16,6 +16,7 @@ import {
   text,
   withKnobs,
 } from '@storybook/addon-knobs';
+import { settings } from 'carbon-components';
 import Modal from '../Modal';
 import Button from '../Button';
 import Select from '../Select';
@@ -24,6 +25,9 @@ import Dropdown from '../Dropdown';
 import SelectItem from '../SelectItem';
 import TextInput from '../TextInput';
 import mdx from './Modal.mdx';
+import Tooltip from '../Tooltip';
+
+const { prefix } = settings;
 
 const sizes = {
   'Extra small (xs)': 'xs',
@@ -319,6 +323,19 @@ export const WithStateManager = () => {
             Custom domains direct requests for your apps in this Cloud Foundry
             organization to a URL that you own. A custom domain can be a shared
             domain, a shared subdomain, or a shared domain and host.
+            <Tooltip tooltipBodyId="tooltip-body">
+              <p id="tooltip-body">
+                This is some tooltip text. This box shows the maximum amount of
+                text that should appear inside. If more room is needed please
+                use a modal instead.
+              </p>
+              <div className={`${prefix}--tooltip__footer`}>
+                <a href="/" className={`${prefix}--link`}>
+                  Learn More
+                </a>
+                <Button size="small">Create</Button>
+              </div>
+            </Tooltip>
           </p>
           <TextInput
             data-modal-primary-focus

diff --git a/packages/react/src/components/Tooltip/Tooltip-story.js b/packages/react/src/components/Tooltip/Tooltip-story.js
@@ -167,12 +167,6 @@ export const DefaultBottom = () => (
         that should appear inside. If more room is needed please use a modal
         instead.
       </p>
-      <div className={`${prefix}--tooltip__footer`}>
-        <a href="/" className={`${prefix}--link`}>
-          Learn More
-        </a>
-        <Button size="small">Create</Button>
-      </div>
     </Tooltip>
   </div>
 );

diff --git a/packages/react/src/components/Tooltip/Tooltip-test.js b/packages/react/src/components/Tooltip/Tooltip-test.js
@@ -194,22 +194,44 @@ describe('Tooltip', () => {
       // Enzyme doesn't seem to allow state() in a forwardRef-wrapped class component
       expect(rootWrapper.find('Tooltip').instance().state.open).toEqual(false);
     });
-  });
 
-  it('escape key keyDown should not bubble outside the tooltip', () => {
-    const onKeyDown = jest.fn();
-    render(
-      <>
-        {/* eslint-disable-next-line jsx-a11y/no-static-element-interactions */}
-        <div onKeyDown={onKeyDown}>
-          <Tooltip triggerText="Tooltip" />
-        </div>
-      </>
-    );
+    it('escape key keyDown should not bubble outside the tooltip', () => {
+      const onKeyDown = jest.fn();
+      render(
+        <>
+          {/* eslint-disable-next-line jsx-a11y/no-static-element-interactions */}
+          <div onKeyDown={onKeyDown}>
+            <Tooltip triggerText="Tooltip" />
+          </div>
+        </>
+      );
+
+      userEvent.click(screen.getAllByRole('button')[0]);
+      userEvent.keyboard('{esc}');
+
+      expect(onKeyDown).not.toHaveBeenCalled();
+    });
 
-    userEvent.click(screen.getAllByRole('button')[0]);
-    userEvent.keyboard('{esc}');
+    it('should close the tooltip when escape key is pressed', () => {
+      render(
+        <Tooltip triggerText="trigger text" tooltipBodyId="tooltip-body">
+          <p id="tooltip-body">tooltip body</p>
+        </Tooltip>
+      );
+
+      expect(screen.queryByText('trigger text')).toBeInTheDocument();
+      expect(screen.queryByText('tooltip body')).not.toBeInTheDocument();
+
+      const triggerButton = screen.getByRole('button');
+      userEvent.click(triggerButton);
+      // I am unsure why, but the trigger must be clicked a second time for the tooltip body to appear
+      userEvent.click(triggerButton);
 
-    expect(onKeyDown).not.toHaveBeenCalled();
+      expect(screen.queryByText('tooltip body')).toBeInTheDocument();
+
+      userEvent.keyboard('{esc}');
+
+      expect(screen.queryByText('tooltip body')).not.toBeInTheDocument();
+    });
   });
 });
diff --git a/packages/react/src/components/Tooltip/Tooltip.js b/packages/react/src/components/Tooltip/Tooltip.js
@@ -268,8 +268,6 @@ class Tooltip extends Component {
     if (!this._debouncedHandleFocus) {
       this._debouncedHandleFocus = debounce(this._handleFocus, 200);
     }
-
-    document.addEventListener('keydown', this.handleEscKeyPress, false);
   }
 
   componentDidUpdate(prevProps, prevState) {
@@ -443,8 +441,6 @@ class Tooltip extends Component {
       this._debouncedHandleFocus.cancel();
       this._debouncedHandleFocus = null;
     }
-
-    document.removeEventListener('keydown', this.handleEscKeyPress, false);
   }
 
   static getDerivedStateFromProps({ open }, state) {
@@ -707,11 +703,7 @@ class Tooltip extends Component {
             {/* eslint-disable-next-line jsx-a11y/no-static-element-interactions */}
             <div
               className={tooltipClasses}
-              onKeyDown={(event) => {
-                if (keyDownMatch(event, [keys.Escape])) {
-                  event.stopPropagation();
-                }
-              }}
+              onKeyDown={this.handleEscKeyPress}
               {...other}
               id={this._tooltipId}
               data-floating-menu-direction={storedDirection}