diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d7ed3a013a..7f922e59efb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ - Added `zIndexAdjustment` to `EuiPopover` which allows tweaking the popover content's `z-index` ([#1097](https://github.com/elastic/eui/pull/1097)) - Added new `EuiSuperSelect` component and `hasArrow` prop to `EuiPopover` ([#921](https://github.com/elastic/eui/pull/921)) +- Added a new `EuiWindowEvent` component for declarative, safe management of `window` event listeners ([#1127](https://github.com/elastic/eui/pull/1127)) +- Changed `Flyout` component to close on ESC keypress even if the flyout does not have focus, using new Window Event component ([#1127](https://github.com/elastic/eui/pull/1127)) **Bug fixes** diff --git a/src-docs/src/routes.js b/src-docs/src/routes.js index b5286e43229..f8a8bd64a72 100644 --- a/src-docs/src/routes.js +++ b/src-docs/src/routes.js @@ -216,6 +216,9 @@ import { ToolTipExample } import { ToggleExample } from './views/toggle/toggle_example'; +import { WindowEventExample } + from './views/window_event/window_event_example'; + import { XYChartExample } from './views/series_chart/series_chart_example'; @@ -395,6 +398,7 @@ const navigation = [{ ToggleExample, UtilityClassesExample, MutationObserverExample, + WindowEventExample, ].map(example => createExample(example)), }, { name: 'Package', diff --git a/src-docs/src/views/window_event/basic_window_event.js b/src-docs/src/views/window_event/basic_window_event.js new file mode 100644 index 00000000000..27d5d0b7c79 --- /dev/null +++ b/src-docs/src/views/window_event/basic_window_event.js @@ -0,0 +1,31 @@ +import React from 'react'; + +import { + EuiModal, + EuiModalBody, + EuiModalHeader, + EuiModalHeaderTitle, + EuiOverlayMask +} from '../../../../src/components'; + +import { ModalExample } from './modal_example_container'; + +const BasicModal = ({ onClose }) => ( + + + + + Example modal + + + +

This modal closes when you press ESC, using a window event listener.

+
+
+
+); + +export const BasicWindowEvent = () => ; diff --git a/src-docs/src/views/window_event/modal_example_container.js b/src-docs/src/views/window_event/modal_example_container.js new file mode 100644 index 00000000000..565b6e4d1ae --- /dev/null +++ b/src-docs/src/views/window_event/modal_example_container.js @@ -0,0 +1,50 @@ +import React, { Component } from 'react'; +import { + EuiButton, +} from '../../../../src/components'; + +import { + EuiWindowEvent, +} from '../../../../src/services'; + +export class ModalExample extends Component { + constructor(props) { + super(props); + + this.state = { + open: false + }; + + this.open = this.open.bind(this); + this.close = this.close.bind(this); + this.closeOnEscape = this.closeOnEscape.bind(this); + } + + open() { + this.setState({ open: true }); + } + + close() { + if (this.state.open) { + this.setState({ open: false }); + } + } + + closeOnEscape({ key }) { + if (key === 'Escape') { + this.close(); + } + } + + render() { + const { modal: Modal, buttonText = 'Open Modal' } = this.props; + const button = {buttonText}; + + return ( +
+ + {this.state.open ? : button} +
+ ); + } +} \ No newline at end of file diff --git a/src-docs/src/views/window_event/mouse_position.js b/src-docs/src/views/window_event/mouse_position.js new file mode 100644 index 00000000000..8116203e2ee --- /dev/null +++ b/src-docs/src/views/window_event/mouse_position.js @@ -0,0 +1,43 @@ +import React, { Component } from 'react'; + +import { + EuiSwitch, + EuiDescriptionList, + EuiSpacer +} from '../../../../src/components'; + +import { + EuiWindowEvent, +} from '../../../../src/services'; + +export class MousePosition extends Component { + + state = { + tracking: false, + coordinates: {} + }; + + onSwitchChange = () => this.setState((state) => ({ tracking: !state.tracking })); + + onMouseMove = ({ clientX, clientY }) => this.setState({ coordinates: { clientX, clientY } }); + + render() { + const listItems = [ + { title: 'Position X', description: this.state.coordinates.clientX || '??' }, + { title: 'Position Y', description: this.state.coordinates.clientY || '??' } + ]; + return ( +
+ + {this.state.tracking ? : null} + + + +
+ ); + } +} \ No newline at end of file diff --git a/src-docs/src/views/window_event/window_event_conflict.js b/src-docs/src/views/window_event/window_event_conflict.js new file mode 100644 index 00000000000..5c683b5081d --- /dev/null +++ b/src-docs/src/views/window_event/window_event_conflict.js @@ -0,0 +1,65 @@ +import React from 'react'; + +import { + EuiModal, + EuiModalBody, + EuiModalHeader, + EuiModalHeaderTitle, + EuiOverlayMask, + EuiFieldText, + EuiSpacer +} from '../../../../src/components'; + +import { ModalExample } from './modal_example_container'; + +class ConflictModal extends React.Component { + + constructor(props) { + super(props); + + this.state = { + inputValue: '' + }; + } + + updateInputValue = e => this.setState({ inputValue: e.target.value }); + + clearInputValueOnEscape = e => { + if (e.key === 'Escape') { + this.setState({ inputValue: '' }); + e.stopPropagation(); + } + } + + render() { + return ( + + + + + Example modal + + + + + +

While typing in this field, ESC will clear the field.

+ +

Otherwise, the event bubbles up to the window and ESC closes the modal.

+
+
+
+ ); + } +} + +export const WindowEventConflict = () => ( + +); diff --git a/src-docs/src/views/window_event/window_event_example.js b/src-docs/src/views/window_event/window_event_example.js new file mode 100644 index 00000000000..159a1f88f74 --- /dev/null +++ b/src-docs/src/views/window_event/window_event_example.js @@ -0,0 +1,117 @@ +import React from 'react'; + +import { renderToHtml } from '../../services'; + +import { + GuideSectionTypes, +} from '../../components'; + +import { + EuiWindowEvent +} from '../../../../src/services'; + +import { + EuiCode, + EuiCallOut, + EuiSpacer +} from '../../../../src/components'; + +import { BasicWindowEvent } from './basic_window_event'; +const basicSource = require('!!raw-loader!./basic_window_event'); +const basicHtml = renderToHtml(BasicWindowEvent); + +import { WindowEventConflict } from './window_event_conflict'; +const conflictSource = require('!!raw-loader!./window_event_conflict'); +const conflictHtml = renderToHtml(WindowEventConflict); + +import { MousePosition } from './mouse_position'; +const mousePositionSource = require('!!raw-loader!./mouse_position'); +const mousePositionHtml = renderToHtml(MousePosition); + +export const WindowEventExample = { + title: 'Window Events', + sections: [ + { + title: 'Basic example: closing a modal on escape', + source: [{ + type: GuideSectionTypes.JS, + code: basicSource, + }, { + type: GuideSectionTypes.HTML, + code: basicHtml, + }], + text: ( +
+

+ Use an EuiWindowEvent to safely and declaratively manage adding and auto-removing event listeners + to the window. This is preferable to setting up your own window event listeners because it will remove + old listeners when your component unmounts, preventing you from accidentally leaving them around forever. +

+

+ This modal example registers a listener on the keydown event and listens for ESC key presses, + which closes the open modal. +

+
+ ), + components: { EuiWindowEvent }, + props: { EuiWindowEvent }, + demo: , + }, + { + title: 'Avoiding event conflicts', + source: [{ + type: GuideSectionTypes.JS, + code: conflictSource, + }, { + type: GuideSectionTypes.HTML, + code: conflictHtml, + }], + text: ( +
+ +

+ Since window event listeners are global, they can conflict with other event listeners if you aren't careful. +

+
+ +

+ The safest and best way to avoid these conflicts is to use event.stopPropagation() at the + lowest, most specific level where you are responding to a DOM event. This will prevent the event from bubbling + up to the window, and the WindowEvent listener will never be triggered, avoiding the conflict. +

+
+ ), + components: { EuiWindowEvent }, + demo: , + }, + { + title: 'Tracking mouse position', + source: [{ + type: GuideSectionTypes.JS, + code: mousePositionSource, + }, { + type: GuideSectionTypes.HTML, + code: mousePositionHtml, + }], + text: ( +
+

+ For some DOM events, you have to listen on the window. One example of this is tracking mouse position. Below, + when you click the toggle switch, your mouse position is tracked. When you toggle off, tracking stops. +

+

+ If you were manually attaching window listeners, you might forget to remove the listener and be silently + responding to mouse events in the background for the life of your app. The WindowEvent component + manages that unmount/unregister process for you. +

+
+ ), + components: { EuiWindowEvent }, + demo: , + } + ], +}; diff --git a/src/components/flyout/flyout.js b/src/components/flyout/flyout.js index 251b8961066..866a3715bba 100644 --- a/src/components/flyout/flyout.js +++ b/src/components/flyout/flyout.js @@ -3,7 +3,7 @@ import classnames from 'classnames'; import PropTypes from 'prop-types'; import FocusTrap from 'focus-trap-react'; -import { keyCodes } from '../../services'; +import { keyCodes, EuiWindowEvent } from '../../services'; import { EuiOverlayMask } from '../overlay_mask'; import { EuiButtonIcon } from '../button'; @@ -20,7 +20,6 @@ export class EuiFlyout extends Component { onKeyDown = event => { if (event.keyCode === keyCodes.ESCAPE) { event.preventDefault(); - event.stopPropagation(); this.props.onClose(); } }; @@ -72,7 +71,6 @@ export class EuiFlyout extends Component { }} className={classes} tabIndex={0} - onKeyDown={this.onKeyDown} style={newStyle || style} {...rest} > @@ -90,6 +88,7 @@ export class EuiFlyout extends Component { return ( + {optionalOverlay} {/* Trap focus even when ownFocus={false}, otherwise closing the flyout won't return focus to the originating button */} diff --git a/src/services/index.js b/src/services/index.js index 9bdbd73d641..de7a563cc2a 100644 --- a/src/services/index.js +++ b/src/services/index.js @@ -67,3 +67,7 @@ export { calculatePopoverPosition, findPopoverPosition, } from './popover'; + +export { + EuiWindowEvent +} from './window_event'; diff --git a/src/services/window_event/index.js b/src/services/window_event/index.js new file mode 100644 index 00000000000..a426a83af0b --- /dev/null +++ b/src/services/window_event/index.js @@ -0,0 +1 @@ +export { default as EuiWindowEvent } from './window_event'; \ No newline at end of file diff --git a/src/services/window_event/window_event.js b/src/services/window_event/window_event.js new file mode 100644 index 00000000000..8bf407863ce --- /dev/null +++ b/src/services/window_event/window_event.js @@ -0,0 +1,46 @@ +import { Component } from 'react'; +import PropTypes from 'prop-types'; + +export default class WindowEvent extends Component { + + componentDidMount() { + this.addEvent(this.props); + } + + componentDidUpdate(prevProps) { + if (prevProps.event !== this.props.event || prevProps.handler !== this.props.handler) { + this.removeEvent(prevProps); + this.addEvent(this.props); + } + } + + componentWillUnmount() { + this.removeEvent(this.props); + } + + addEvent({ event, handler }) { + window.addEventListener(event, handler); + } + + removeEvent({ event, handler }) { + window.removeEventListener(event, handler); + } + + render() { + return null; + } + +} + +WindowEvent.displayName = 'WindowEvent'; + +WindowEvent.propTypes = { + /** + * Type of valid DOM event + */ + event: PropTypes.string.isRequired, + /** + * Event callback function + */ + handler: PropTypes.func.isRequired +}; diff --git a/src/services/window_event/window_event.test.js b/src/services/window_event/window_event.test.js new file mode 100644 index 00000000000..aa21a725e5a --- /dev/null +++ b/src/services/window_event/window_event.test.js @@ -0,0 +1,53 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import { EuiWindowEvent } from '.'; + +describe('EuiWindowEvent', () => { + + beforeEach(() => { + window.addEventListener = jest.fn(); + window.removeEventListener = jest.fn(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('attaches handler to window event on mount', () => { + const handler = () => null; + shallow(); + expect(window.addEventListener).toHaveBeenCalledTimes(1); + expect(window.addEventListener).toHaveBeenCalledWith('click', handler); + }); + + test('removes handler on unmount', () => { + const handler = () => null; + const wrapper = shallow(); + wrapper.unmount(); + expect(window.removeEventListener).toHaveBeenLastCalledWith('click', handler); + }); + + test('removes and re-attaches handler to window event on update', () => { + const handler1 = () => null; + const handler2 = () => null; + const wrapper = shallow(); + + expect(window.addEventListener).toHaveBeenLastCalledWith('click', handler1); + + wrapper.setProps({ event: 'hover', handler: handler2 }); + + expect(window.removeEventListener).toHaveBeenLastCalledWith('click', handler1); + expect(window.addEventListener).toHaveBeenLastCalledWith('hover', handler2); + }); + + test('does not remove or re-attach handler if update is irrelevant', () => { + const handler = () => null; + const wrapper = shallow(); + expect(window.addEventListener).toHaveBeenCalledTimes(1); + + wrapper.setProps({ whatever: 'ugh' }); + expect(window.addEventListener).toHaveBeenCalledTimes(1); + expect(window.removeEventListener).not.toHaveBeenCalled(); + }); + +}); \ No newline at end of file