Merge pull request #4 from cjcenizal/tooltip-accessibility

Improve EuiToolTip accessibility, remove ability to display it on click

src-docs/src/views/tool_tip/tool_tip.js

@@ -1,6 +1,7 @@
 import React from 'react';
 
 import {
+  EuiIcon,
   EuiToolTip,
   EuiLink,
   EuiText,
@@ -12,35 +13,37 @@ import {
 export default () => (
   <div>
     <EuiText>
-      <p style={{ overflow: 'hidden' }}>
+      <p>
+        This tooltip appears on the{' '}
+        <EuiToolTip position="top" content="Here is some tooltip text">
+          <EuiLink href="#">top</EuiLink>
+        </EuiToolTip>
+      </p>
+
+      <p>
         This tooltip appears on the{' '}
         <EuiToolTip
           position="left"
           title="Tooltip titles are optional"
           content="Here is some tooltip text. Lets add some more content to see how it wraps."
         >
-          <EuiLink>left</EuiLink>
+          <EuiLink href="#">left</EuiLink>
         </EuiToolTip>
         {' '} and includes the optional title.
       </p>
-      <p style={{ overflow: 'hidden' }}>
+
+      <p>
         This tooltip appears on the{' '}
         <EuiToolTip position="right" content="Here is some tooltip text">
-          <EuiLink>right</EuiLink>
+          <EuiLink href="#">right</EuiLink>
         </EuiToolTip>
       </p>
-      <p style={{ overflow: 'hidden' }}>
-        This tooltip appears on the{' '}
-        <EuiToolTip position="top" content="Here is some tooltip text">
-          <EuiLink>top</EuiLink>
-        </EuiToolTip>
-      </p>
-      <p style={{ overflow: 'hidden' }}>
-        This tooltip appears on the{' '}
-        <EuiToolTip position="bottom" clickOnly content="You need to click or leave focus to dismiss this one.">
-          <EuiLink>bottom</EuiLink>
+
+      <p>
+        This tooltip appears on the bottom of this icon:{' '}
+        <EuiToolTip position="bottom" content="Here is some tooltip text">
+          <EuiIcon tabIndex="0" type="alert" title="Icon with tooltip" />
         </EuiToolTip>
-        {' '} and requires a <strong>click to activate</strong>.
       </p>
     </EuiText>
 

src/components/portal/__snapshots__/portal.test.js.snap

@@ -0,0 +1,7 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`EuiPortal is rendered 1`] = `
+<div>
+  Content
+</div>
+`;

src/components/portal/portal.test.js

@@ -0,0 +1,24 @@
+import React from 'react';
+import { render } from 'enzyme';
+import ReactDOM from 'react-dom';
+import { requiredProps } from '../../test';
+import { EuiPortal } from './portal';
+
+// TODO: Temporary hack which we can remove once react-test-renderer supports portals.
+// More info at https://github.com/facebook/react/issues/11565.
+ReactDOM.createPortal = node => node;
+
+describe('EuiPortal', () => {
+  test('is rendered', () => {
+    const component = render(
+      <div>
+        <EuiPortal>
+          Content
+        </EuiPortal>
+      </div>
+    );
+
+    expect(component)
+      .toMatchSnapshot();
+  });
+});

src/components/tool_tip/__snapshots__/tool_tip.test.js.snap

@@ -1,10 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`EuiToolTip is rendered 1`] = `
-<span
-  aria-describedby="id"
-  tabindex="0"
->
-  trigger
+<span>
+  <button
+    aria-describedby="id"
+  >
+    Trigger
+  </button>
 </span>
 `;

src/components/tool_tip/tool_tip.js

@@ -1,13 +1,14 @@
 import React, {
   Component,
+  cloneElement,
+  Fragment,
 } from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
 import { EuiPortal } from '../portal';
 import { EuiToolTipPopover } from './tool_tip_popover';
 import { calculatePopoverPosition, calculatePopoverStyles } from '../../services';
-import { EuiOutsideClickDetector } from '../outside_click_detector';
 
 import makeId from '../form/form_row/make_id';
 
@@ -21,28 +22,23 @@ const positionsToClassNameMap = {
 export const POSITIONS = Object.keys(positionsToClassNameMap);
 
 export class EuiToolTip extends Component {
-
   constructor(props) {
     super(props);
 
     this.state = {
       visible: false,
+      hasFocus: false,
       calculatedPosition: this.props.position,
       toolTipStyles: {},
       id: this.props.id || makeId(),
     };
-
-    this.showToolTip = this.showToolTip.bind(this);
-    this.positionToolTip = this.positionToolTip.bind(this);
-    this.hideToolTip = this.hideToolTip.bind(this);
-    this.toggleToolTipVisibility = this.toggleToolTipVisibility.bind(this);
   }
 
-  showToolTip() {
+  showToolTip = () => {
     this.setState({ visible: true });
-  }
+  };
 
-  positionToolTip(toolTipRect) {
+  positionToolTip = (toolTipRect) => {
     const wrapperRect = this.wrapper.getBoundingClientRect();
     const userPosition = this.props.position;
 
@@ -54,27 +50,38 @@ export class EuiToolTip extends Component {
       calculatedPosition,
       toolTipStyles,
     });
-  }
+  };
 
-  hideToolTip() {
+  hideToolTip = () => {
     this.setState({ visible: false });
-  }
+  };
 
-  toggleToolTipVisibility(event) {
-    event.preventDefault();
-    this.setState(prevState => ({
-      visible: !prevState.visible
-    }));
-  }
+  onFocus = () => {
+    this.setState({
+      hasFocus: true,
+    });
+    this.showToolTip();
+  };
 
-  render() {
+  onBlur = () => {
+    this.setState({
+      hasFocus: false,
+    });
+    this.hideToolTip();
+  };
+
+  onMouseOut = () => {
+    if (!this.state.hasFocus) {
+      this.hideToolTip();
+    }
+  };
 
+  render() {
     const {
       children,
       className,
       content,
       title,
-      clickOnly,
       ...rest
     } = this.props;
 
@@ -103,35 +110,23 @@ export class EuiToolTip extends Component {
       );
     }
 
-    let toolTipProps;
-    if (clickOnly) {
-      // react fires onFocus before onClick, but onMouseDown gets called before onFocus
-      // using onMouseDown so handler gets called before onFocus
-      // https://stackoverflow.com/a/28963938/890809
-      toolTipProps = {
-        onMouseDown: this.toggleToolTipVisibility
-      };
-    } else {
-      toolTipProps = {
-        onMouseOver: this.showToolTip,
-        onMouseOut: this.hideToolTip
-      };
-    }
+    const trigger = (
+      <span ref={wrapper => this.wrapper = wrapper}>
+        {cloneElement(children, {
+          onFocus: this.showToolTip,
+          onBlur: this.hideToolTip,
+          'aria-describedby': this.state.id,
+          onMouseOver: this.showToolTip,
+          onMouseOut: this.onMouseOut
+        })}
+      </span>
+    );
 
     return (
-      <EuiOutsideClickDetector onOutsideClick={this.hideToolTip}>
-        <span
-          onFocus={this.showToolTip}
-          onBlur={this.hideToolTip}
-          ref={wrapper => this.wrapper = wrapper}
-          aria-describedby={this.state.id}
-          tabIndex={0}
-          {...toolTipProps}
-        >
-          {children}
-          {tooltip}
-        </span>
-      </EuiOutsideClickDetector>
+      <Fragment>
+        {trigger}
+        {tooltip}
+      </Fragment>
     );
   }
 }
@@ -140,7 +135,7 @@ EuiToolTip.propTypes = {
   /**
    * The in-view trigger for your tooltip.
    */
-  children: PropTypes.node.isRequired,
+  children: PropTypes.element.isRequired,
   /**
    * The main content of your tooltip.
    */

src/components/tool_tip/tool_tip.test.js

@@ -7,7 +7,9 @@ import { EuiToolTip } from './tool_tip';
 describe('EuiToolTip', () => {
   test('is rendered', () => {
     const component = render(
-      <EuiToolTip children="trigger" title="title" id="id" content="content" {...requiredProps} />
+      <EuiToolTip title="title" id="id" content="content" {...requiredProps}>
+        <button>Trigger</button>
+      </EuiToolTip>
     );
 
     expect(component)

src/components/tool_tip/tool_tip_popover.js

@@ -27,7 +27,10 @@ export class EuiToolTipPopover extends Component {
 
   updateDimensions() {
     requestAnimationFrame(() => {
-      this.props.positionToolTip(this.popover.getBoundingClientRect());
+      // Because of this delay, sometimes `positionToolTip` becomes unavailable.
+      if (this.popover) {
+        this.props.positionToolTip(this.popover.getBoundingClientRect());
+      }
     });
   }
 
@@ -41,9 +44,7 @@ export class EuiToolTipPopover extends Component {
       children,
       title,
       className,
-      /* eslint-disable */
-      positionToolTip,
-      /* eslint-enable */
+      positionToolTip, // eslint-disable-line no-unused-vars
       ...rest
     } = this.props;