Update components to support both href and onClick (#554)

Update link, button, card, etc to support both href and onClick. Fixes #553
elastic · Mar 26, 2018 · b01218a · b01218a
1 parent 92a77ee
commit b01218a
Show file tree

Hide file tree

Showing 18 changed files with 170 additions and 86 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,6 @@
 # [`master`](https://github.com/elastic/eui/tree/master)
 
-No public interface changes since `0.0.34`.
+- Modified `link` and all buttons to support both href and onClick ([554](https://github.com/elastic/eui/pull/554))
 
 # [`0.0.34`](https://github.com/elastic/eui/tree/v0.0.34)
 

diff --git a/src-docs/src/views/card/card_image.js b/src-docs/src/views/card/card_image.js
@@ -39,11 +39,11 @@ export default () => (
     <EuiFlexItem>
       <EuiCard
         textAlign="left"
+        href="https://elastic.github.io/eui/"
         image="https://source.unsplash.com/400x200/?City"
         icon={<EuiIcon size="xxl" type="logoBeats" />}
         title={`Beats in the City`}
-        description="Example of a card's description. Stick to one or two sentences."
-        footer={cardFooterContent}
+        description="This card has an href and should be a link."
       />
     </EuiFlexItem>
   </EuiFlexGroup>

diff --git a/src-docs/src/views/link/link.js b/src-docs/src/views/link/link.js
@@ -25,7 +25,20 @@ export default () => (
           button
         </EuiLink>
       )} with an onClick handler.
-
+    </p>
+    <p>
+      Here is an example of a {(
+        <EuiLink
+          href="https://github.com/elastic/eui"
+          onClick={(e) => {
+            if (!confirm('Are you sure you want to see the eui repo?')) {
+              e.preventDefault();
+            }
+          }}
+        >
+          link
+        </EuiLink>
+      )} with both an href and an onClick handler.
     </p>
     <p>Links can be colored as well.</p>
     <ul>

diff --git a/src/components/button/button.js b/src/components/button/button.js
@@ -6,10 +6,7 @@ import {
   EuiLoadingSpinner
 } from '../loading';
 
-import {
-  checkHrefAndOnClick,
-  getSecureRelForTarget,
-} from '../../services';
+import { getSecureRelForTarget } from '../../services';
 
 import {
   ICON_TYPES,
@@ -53,7 +50,6 @@ export const EuiButton = ({
   href,
   target,
   rel,
-  onClick,
   type,
   buttonRef,
   ...rest
@@ -117,7 +113,6 @@ export const EuiButton = ({
       <button
         disabled={isDisabled}
         className={classes}
-        onClick={onClick}
         type={type}
         ref={buttonRef}
         {...rest}
@@ -152,7 +147,7 @@ EuiButton.propTypes = {
   color: PropTypes.oneOf(COLORS),
   size: PropTypes.oneOf(SIZES),
   isDisabled: PropTypes.bool,
-  href: checkHrefAndOnClick,
+  href: PropTypes.string,
   target: PropTypes.string,
   rel: PropTypes.string,
   onClick: PropTypes.func,

diff --git a/src/components/button/button.test.js b/src/components/button/button.test.js
@@ -1,5 +1,5 @@
 import React from 'react';
-import { render } from 'enzyme';
+import { render, mount } from 'enzyme';
 import { requiredProps } from '../../test/required_props';
 
 import {
@@ -117,5 +117,25 @@ describe('EuiButton', () => {
           .toMatchSnapshot();
       });
     });
+
+    describe('onClick', () => {
+      it('supports onClick and href', () => {
+        const handler = jest.fn();
+        const component = mount(
+          <EuiButton href="#" onClick={handler} />
+        );
+        component.find('a').simulate('click');
+        expect(handler.mock.calls.length).toEqual(1);
+      });
+
+      it('supports onClick as a button', () => {
+        const handler = jest.fn();
+        const component = mount(
+          <EuiButton onClick={handler} />
+        );
+        component.find('button').simulate('click');
+        expect(handler.mock.calls.length).toEqual(1);
+      });
+    });
   });
 });
diff --git a/src/components/button/button_empty/button_empty.js b/src/components/button/button_empty/button_empty.js
@@ -2,10 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
-import {
-  checkHrefAndOnClick,
-  getSecureRelForTarget,
-} from '../../../services';
+import { getSecureRelForTarget } from '../../../services';
 
 import {
   ICON_TYPES,
@@ -56,7 +53,6 @@ export const EuiButtonEmpty = ({
   href,
   target,
   rel,
-  onClick,
   type,
   buttonRef,
   ...rest
@@ -108,7 +104,6 @@ export const EuiButtonEmpty = ({
       <button
         disabled={isDisabled}
         className={classes}
-        onClick={onClick}
         type={type}
         ref={buttonRef}
         {...rest}
@@ -131,7 +126,7 @@ EuiButtonEmpty.propTypes = {
   size: PropTypes.oneOf(SIZES),
   flush: PropTypes.oneOf(FLUSH_TYPES),
   isDisabled: PropTypes.bool,
-  href: checkHrefAndOnClick,
+  href: PropTypes.string,
   target: PropTypes.string,
   rel: PropTypes.string,
   onClick: PropTypes.func,

diff --git a/src/components/button/button_empty/button_empty.test.js b/src/components/button/button_empty/button_empty.test.js
@@ -1,5 +1,5 @@
 import React from 'react';
-import { render } from 'enzyme';
+import { render, mount } from 'enzyme';
 import { requiredProps } from '../../../test/required_props';
 
 import {
@@ -109,5 +109,25 @@ describe('EuiButtonEmpty', () => {
           .toMatchSnapshot();
       });
     });
+
+    describe('onClick', () => {
+      it('supports onClick and href', () => {
+        const handler = jest.fn();
+        const component = mount(
+          <EuiButtonEmpty href="#" onClick={handler} />
+        );
+        component.find('a').simulate('click');
+        expect(handler.mock.calls.length).toEqual(1);
+      });
+
+      it('supports onClick as a button', () => {
+        const handler = jest.fn();
+        const component = mount(
+          <EuiButtonEmpty onClick={handler} />
+        );
+        component.find('button').simulate('click');
+        expect(handler.mock.calls.length).toEqual(1);
+      });
+    });
   });
 });
diff --git a/src/components/button/button_icon/button_icon.js b/src/components/button/button_icon/button_icon.js
@@ -2,10 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
-import {
-  checkHrefAndOnClick,
-  getSecureRelForTarget,
-} from '../../../services';
+import { getSecureRelForTarget } from '../../../services';
 
 import {
   ICON_TYPES,
@@ -43,7 +40,6 @@ export const EuiButtonIcon = ({
   color,
   isDisabled,
   href,
-  onClick,
   type,
   target,
   rel,
@@ -91,7 +87,6 @@ export const EuiButtonIcon = ({
       <button
         disabled={isDisabled}
         className={classes}
-        onClick={onClick}
         type={type}
         ref={buttonRef}
         {...rest}
@@ -109,7 +104,7 @@ EuiButtonIcon.propTypes = {
   color: PropTypes.oneOf(COLORS),
   isDisabled: PropTypes.bool,
   'aria-label': accessibleButtonIcon,
-  href: checkHrefAndOnClick,
+  href: PropTypes.string,
   target: PropTypes.string,
   rel: PropTypes.string,
   onClick: PropTypes.func,

diff --git a/src/components/button/button_icon/button_icon.test.js b/src/components/button/button_icon/button_icon.test.js
@@ -1,5 +1,5 @@
 import React from 'react';
-import { render } from 'enzyme';
+import { render, mount } from 'enzyme';
 import { requiredProps } from '../../../test/required_props';
 
 import { EuiButtonIcon, COLORS } from './button_icon';
@@ -60,5 +60,25 @@ describe('EuiButtonIcon', () => {
           .toMatchSnapshot();
       });
     });
+
+    describe('onClick', () => {
+      it('supports onClick and href', () => {
+        const handler = jest.fn();
+        const component = mount(
+          <EuiButtonIcon aria-label="hoi" href="#" onClick={handler} />
+        );
+        component.find('a').simulate('click');
+        expect(handler.mock.calls.length).toEqual(1);
+      });
+
+      it('supports onClick as a button', () => {
+        const handler = jest.fn();
+        const component = mount(
+          <EuiButtonIcon aria-label="hoi" onClick={handler} />
+        );
+        component.find('button').simulate('click');
+        expect(handler.mock.calls.length).toEqual(1);
+      });
+    });
   });
 });
diff --git a/src/components/card/card.js b/src/components/card/card.js
@@ -5,10 +5,6 @@ import classNames from 'classnames';
 import { EuiText } from '../text';
 import { EuiTitle } from '../title';
 
-import {
-  checkHrefAndOnClick,
-} from '../../services';
-
 const textAlignToClassNameMap = {
   left: 'euiCard--leftAligned',
   center: 'euiCard--centerAligned',
@@ -55,10 +51,10 @@ export const EuiCard = ({
   }
 
   let OuterElement = 'div';
-  if (onClick) {
-    OuterElement = 'button';
-  } else if (href) {
+  if (href) {
     OuterElement = 'a';
+  } else if (onClick) {
+    OuterElement = 'button';
   }
 
   return (
@@ -114,7 +110,7 @@ EuiCard.propTypes = {
    * Use only if you want to forego a button in the footer and make the whole card clickable
    */
   onClick: PropTypes.func,
-  href: checkHrefAndOnClick,
+  href: PropTypes.string,
   textAlign: PropTypes.oneOf(ALIGNMENTS),
 };
 

diff --git a/src/components/card/card.test.js b/src/components/card/card.test.js
@@ -1,5 +1,5 @@
 import React from 'react';
-import { render } from 'enzyme';
+import { render, mount } from 'enzyme';
 import { requiredProps } from '../../test';
 
 import { EuiCard } from './card';
@@ -13,4 +13,33 @@ describe('EuiCard', () => {
     expect(component)
       .toMatchSnapshot();
   });
+
+  describe('onClick', () => {
+    it('supports onClick as a link', () => {
+      const handler = jest.fn();
+      const component = mount(
+        <EuiCard
+          title="Hoi"
+          description="There"
+          href="#"
+          onClick={handler}
+        />
+      );
+      component.find('a').simulate('click');
+      expect(handler.mock.calls.length).toEqual(1);
+    });
+
+    it('supports onClick as a button', () => {
+      const handler = jest.fn();
+      const component = mount(
+        <EuiCard
+          title="Hoi"
+          description="There"
+          onClick={handler}
+        />
+      );
+      component.find('button').simulate('click');
+      expect(handler.mock.calls.length).toEqual(1);
+    });
+  });
 });
diff --git a/src/components/filter_group/filter_button.js b/src/components/filter_group/filter_button.js
@@ -2,10 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
-import {
-  checkHrefAndOnClick,
-  getSecureRelForTarget,
-} from '../../services';
+import { getSecureRelForTarget } from '../../services';
 
 import {
   ICON_TYPES,
@@ -41,7 +38,6 @@ export const EuiFilterButton = ({
   href,
   target,
   rel,
-  onClick,
   type,
   ...rest
 }) => {
@@ -93,7 +89,6 @@ export const EuiFilterButton = ({
       <button
         disabled={isDisabled}
         className={classes}
-        onClick={onClick}
         type={type}
         {...rest}
       >
@@ -109,6 +104,7 @@ export const EuiFilterButton = ({
 EuiFilterButton.propTypes = {
   children: PropTypes.node,
   className: PropTypes.string,
+  onClick: PropTypes.func,
   /**
    * Use any one of our icons
    */
@@ -127,7 +123,7 @@ EuiFilterButton.propTypes = {
   /**
    * If passed, changes the button to an anchor tag
    */
-  href: checkHrefAndOnClick,
+  href: PropTypes.string,
   /**
    * Used along with href
    */
@@ -136,10 +132,6 @@ EuiFilterButton.propTypes = {
    * Used along with href
    */
   rel: PropTypes.string,
-  /**
-   * Onclick should not be comibned with href
-   */
-  onClick: PropTypes.func,
   /**
    * Defines html button input type
    */