Add buttonRef props to all buttons, and fix bug with autoFocused butt…

…ons on EuiConfirmModal. (#529)
elastic · Mar 16, 2018 · 3dee390 · 3dee390
1 parent 11edf69
commit 3dee390
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 74 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,10 +1,12 @@
 # [`master`](https://github.com/elastic/eui/tree/master)
 
 - Added `EuiIconTip` to make it easier to display icons with tooltips ([#528](https://github.com/elastic/eui/pull/528))
+- Added `buttonRef` prop to `EuiButton`, `EuiButtonEmpty`, and `EuiButtonIcon` ([#529](https://github.com/elastic/eui/pull/529))
 
 **Bug fixes**
 
 - Fix `EuiPageContent` centering within `EuiPage` issue ([#527](https://github.com/elastic/eui/pull/527))
+- `EuiConfirmModal` will now correctly auto-focus on its confirm and cancel buttons (#529[](https://github.com/elastic/eui/pull/529))
 
 # [`0.0.28`](https://github.com/elastic/eui/tree/v0.0.28)
 

diff --git a/src/components/button/button.js b/src/components/button/button.js
@@ -55,6 +55,7 @@ export const EuiButton = ({
   rel,
   onClick,
   type,
+  buttonRef,
   ...rest
 }) => {
 
@@ -102,6 +103,7 @@ export const EuiButton = ({
         href={href}
         target={target}
         rel={secureRel}
+        ref={buttonRef}
         {...rest}
       >
         <span className="euiButton__content">
@@ -117,6 +119,7 @@ export const EuiButton = ({
         className={classes}
         onClick={onClick}
         type={type}
+        ref={buttonRef}
         {...rest}
       >
         <span className="euiButton__content">
@@ -163,6 +166,7 @@ EuiButton.propTypes = {
    * Standard HTML attribute
    */
   type: PropTypes.string,
+  buttonRef: PropTypes.func,
 };
 
 EuiButton.defaultProps = {

diff --git a/src/components/button/button_empty/button_empty.js b/src/components/button/button_empty/button_empty.js
@@ -58,6 +58,7 @@ export const EuiButtonEmpty = ({
   rel,
   onClick,
   type,
+  buttonRef,
   ...rest
 }) => {
 
@@ -93,6 +94,7 @@ export const EuiButtonEmpty = ({
         href={href}
         target={target}
         rel={secureRel}
+        ref={buttonRef}
         {...rest}
       >
         <span className="euiButtonEmpty__content">
@@ -108,6 +110,7 @@ export const EuiButtonEmpty = ({
         className={classes}
         onClick={onClick}
         type={type}
+        ref={buttonRef}
         {...rest}
       >
         <span className="euiButtonEmpty__content">
@@ -133,6 +136,7 @@ EuiButtonEmpty.propTypes = {
   rel: PropTypes.string,
   onClick: PropTypes.func,
   type: PropTypes.string,
+  buttonRef: PropTypes.func,
 };
 
 EuiButtonEmpty.defaultProps = {

diff --git a/src/components/button/button_icon/button_icon.js b/src/components/button/button_icon/button_icon.js
@@ -47,6 +47,7 @@ export const EuiButtonIcon = ({
   type,
   target,
   rel,
+  buttonRef,
   ...rest
 }) => {
 
@@ -79,6 +80,7 @@ export const EuiButtonIcon = ({
         href={href}
         target={target}
         rel={secureRel}
+        ref={buttonRef}
         {...rest}
       >
         {buttonIcon}
@@ -91,6 +93,7 @@ export const EuiButtonIcon = ({
         className={classes}
         onClick={onClick}
         type={type}
+        ref={buttonRef}
         {...rest}
       >
         {buttonIcon}
@@ -111,6 +114,7 @@ EuiButtonIcon.propTypes = {
   rel: PropTypes.string,
   onClick: PropTypes.func,
   type: PropTypes.string,
+  buttonRef: PropTypes.func,
 };
 
 EuiButtonIcon.defaultProps = {

diff --git a/src/components/modal/confirm_modal.js b/src/components/modal/confirm_modal.js
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { Component } from 'react';
 import PropTypes from 'prop-types';
 import classnames from 'classnames';
 
@@ -25,75 +25,97 @@ const CONFIRM_MODAL_BUTTONS = [
   CANCEL_BUTTON,
 ];
 
-export function EuiConfirmModal({
-  children,
-  title,
-  onCancel,
-  onConfirm,
-  cancelButtonText,
-  confirmButtonText,
-  className,
-  defaultFocusedButton,
-  ...rest
-}) {
-  const classes = classnames('euiModal--confirmation', className);
-
-  let modalTitle;
-
-  if (title) {
-    modalTitle = (
-      <EuiModalHeader>
-        <EuiModalHeaderTitle data-test-subj="confirmModalTitleText">
-          {title}
-        </EuiModalHeaderTitle>
-      </EuiModalHeader>
-    );
+export class EuiConfirmModal extends Component {
+  componentDidMount() {
+    // We have to do this instead of using `autoFocus` because React's polyfill for auto-focusing
+    // elements conflicts with the focus-trap logic we have on EuiModal.
+    const { defaultFocusedButton } = this.props;
+
+    // Wait a beat for the focus-trap to complete, and then set focus to the right button.
+    requestAnimationFrame(() => {
+      if (defaultFocusedButton === CANCEL_BUTTON) {
+        this.cancelButton.focus();
+      } else if (defaultFocusedButton === CONFIRM_BUTTON) {
+        this.confirmButton.focus();
+      }
+    });
   }
 
-  let message;
+  confirmRef = node => this.confirmButton = node;
+  cancelRef = node => this.cancelButton = node;
 
-  if (typeof children === 'string') {
-    message = <p>{children}</p>;
-  } else {
-    message = children;
-  }
+  render() {
+    const {
+      children,
+      title,
+      onCancel,
+      onConfirm,
+      cancelButtonText,
+      confirmButtonText,
+      className,
+      defaultFocusedButton, // eslint-disable-line no-unused-vars
+      ...rest
+    } = this.props;
+
+    const classes = classnames('euiModal--confirmation', className);
+
+    let modalTitle;
+
+    if (title) {
+      modalTitle = (
+        <EuiModalHeader>
+          <EuiModalHeaderTitle data-test-subj="confirmModalTitleText">
+            {title}
+          </EuiModalHeaderTitle>
+        </EuiModalHeader>
+      );
+    }
+
+    let message;
 
-  return (
-    <EuiModal
-      className={classes}
-      onClose={onCancel}
-      {...rest}
-    >
-      {modalTitle}
-
-      <EuiModalBody>
-        <EuiText data-test-subj="confirmModalBodyText">
-          {message}
-        </EuiText>
-      </EuiModalBody>
-
-      <EuiModalFooter>
-        <EuiButtonEmpty
-          autoFocus={defaultFocusedButton === CANCEL_BUTTON}
-          data-test-subj="confirmModalCancelButton"
-          onClick={onCancel}
-          size="s"
-        >
-          {cancelButtonText}
-        </EuiButtonEmpty>
-
-        <EuiButton
-          autoFocus={defaultFocusedButton === CONFIRM_BUTTON}
-          data-test-subj="confirmModalConfirmButton"
-          onClick={onConfirm}
-          size="s"
-          fill
-        >
-          {confirmButtonText}
-        </EuiButton>
-      </EuiModalFooter>
-    </EuiModal>
-  );
+    if (typeof children === 'string') {
+      message = <p>{children}</p>;
+    } else {
+      message = children;
+    }
+
+    return (
+      <EuiModal
+        className={classes}
+        onClose={onCancel}
+        {...rest}
+      >
+        {modalTitle}
+
+        <EuiModalBody>
+          <EuiText data-test-subj="confirmModalBodyText">
+            {message}
+          </EuiText>
+        </EuiModalBody>
+
+        <EuiModalFooter>
+          <EuiButtonEmpty
+            data-test-subj="confirmModalCancelButton"
+            onClick={onCancel}
+            size="s"
+            buttonRef={this.cancelRef}
+          >
+            {cancelButtonText}
+          </EuiButtonEmpty>
+
+          <EuiButton
+            data-test-subj="confirmModalConfirmButton"
+            onClick={onConfirm}
+            size="s"
+            fill
+            buttonRef={this.confirmRef}
+          >
+            {confirmButtonText}
+          </EuiButton>
+        </EuiModalFooter>
+      </EuiModal>
+    );
+  }
 }
 
 EuiConfirmModal.propTypes = {

diff --git a/src/components/modal/confirm_modal.test.js b/src/components/modal/confirm_modal.test.js
@@ -82,7 +82,7 @@ describe('onCancel', () => {
 });
 
 describe('defaultFocusedButton', () => {
-  test('is cancel', () => {
+  test('is cancel', done => {
     const component = mount(
       <EuiConfirmModal
         onCancel={onCancel}
@@ -93,11 +93,15 @@ describe('defaultFocusedButton', () => {
       />
     );
 
-    const button = findTestSubject(component, 'confirmModalCancelButton').getDOMNode();
-    expect(document.activeElement).toEqual(button);
+    // The auto-focus implementation waits a frame before focusing.
+    requestAnimationFrame(() => {
+      const button = findTestSubject(component, 'confirmModalCancelButton').getDOMNode();
+      expect(document.activeElement).toEqual(button);
+      done();
+    });
   });
 
-  test('is confirm', () => {
+  test('is confirm', done => {
     const component = mount(
       <EuiConfirmModal
         onCancel={onCancel}
@@ -108,11 +112,15 @@ describe('defaultFocusedButton', () => {
       />
     );
 
-    const button = findTestSubject(component, 'confirmModalConfirmButton').getDOMNode();
-    expect(document.activeElement).toEqual(button);
+    // The auto-focus implementation waits a frame before focusing.
+    requestAnimationFrame(() => {
+      const button = findTestSubject(component, 'confirmModalConfirmButton').getDOMNode();
+      expect(document.activeElement).toEqual(button);
+      done();
+    });
   });
 
-  test('when not given gives focus to the modal', () => {
+  test('when not given gives focus to the modal', done => {
     const component = mount(
       <EuiConfirmModal
         onCancel={onCancel}
@@ -121,6 +129,11 @@ describe('defaultFocusedButton', () => {
         confirmButtonText="Confirm Button Text"
       />
     );
-    expect(document.activeElement).toEqual(component.getDOMNode().firstChild);
+
+    // The auto-focus implementation waits a frame before focusing.
+    requestAnimationFrame(() => {
+      expect(document.activeElement).toEqual(component.getDOMNode().firstChild);
+      done();
+    });
   });
 });