From 48cec747b876cf8353abfe3f2f9ba4790c06e454 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Fri, 3 Dec 2021 15:09:26 -0600 Subject: [PATCH] fix(tooltip): ensure esc closes tooltip, avoid bubbling beyond the body (#10220) Co-authored-by: Abbey Hart --- .../react/src/components/Modal/Modal-story.js | 17 +++++++ .../src/components/Tooltip/Tooltip-story.js | 6 --- .../src/components/Tooltip/Tooltip-test.js | 50 +++++++++++++------ .../react/src/components/Tooltip/Tooltip.js | 10 +--- 4 files changed, 54 insertions(+), 29 deletions(-) diff --git a/packages/react/src/components/Modal/Modal-story.js b/packages/react/src/components/Modal/Modal-story.js index 86556df7f226..ddf291cc4d24 100644 --- 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. + +

+ 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. +

+
+ + Learn More + + +
+

( that should appear inside. If more room is needed please use a modal instead.

-
- - Learn More - - -
); diff --git a/packages/react/src/components/Tooltip/Tooltip-test.js b/packages/react/src/components/Tooltip/Tooltip-test.js index 72ddbc00e9a9..99a861ae0d0b 100644 --- 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 */} -
- -
- - ); + 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 */} +
+ +
+ + ); + + 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 body

+
+ ); + + 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 index eaa0d4c093c9..f87d5a21d6ed 100644 --- 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 */}
{ - if (keyDownMatch(event, [keys.Escape])) { - event.stopPropagation(); - } - }} + onKeyDown={this.handleEscKeyPress} {...other} id={this._tooltipId} data-floating-menu-direction={storedDirection}