feat(secondary-icon-button): add as prop (#1168)

* feat(secondary-icon-button): add as

* chore: add tests

Author: montezume

src/components/buttons/accessible-button/accessible-button.js

@@ -1,5 +1,6 @@
-import PropTypes from 'prop-types';
 import React from 'react';
+import PropTypes from 'prop-types';
+import { oneLine } from 'common-tags';
 import styled from '@emotion/styled';
 import vars from '../../../../materials/custom-properties';
 
@@ -67,7 +68,23 @@ AccessibleButton.displayName = 'AccessibleButton';
 AccessibleButton.propTypes = {
   as: PropTypes.oneOfType([PropTypes.string, PropTypes.elementType]),
   id: PropTypes.string,
-  type: PropTypes.oneOf(['submit', 'reset', 'button']),
+  type: (props, propName, componentName, ...rest) => {
+    // the type defaults to `button`, so we don't need to handle undefined
+    if (props.as && props.type !== 'button') {
+      throw new Error(
+        oneLine`
+          ${componentName}: "${propName}" does not have any effect when
+          "as" is set.
+        `
+      );
+    }
+    return PropTypes.oneOf(['submit', 'reset', 'button'])(
+      props,
+      propName,
+      componentName,
+      ...rest
+    );
+  },
   label: PropTypes.string.isRequired,
   children: PropTypes.node.isRequired,
   // set to true or false to indicate a toggle button

src/components/buttons/secondary-icon-button/README.md

@@ -24,14 +24,17 @@ also pass a label for accessibility reasons.
 
 #### Properties
 
-| Props        | Type     | Required | Values                      | Default  | Description                                                                            |
-| ------------ | -------- | :------: | --------------------------- | -------- | -------------------------------------------------------------------------------------- |
-| `type`       | `string` |    -     | `submit`, `reset`, `button` | `button` | Used as the HTML `type` attribute.                                                     |
-| `label`      | `string` |    ✅    | -                           | -        | Should describe what the button does, for accessibility purposes (screen-reader users) |
-| `icon`       | `node`   |    ✅    | -                           | -        | An `Icon` component                                                                    |
-| `isDisabled` | `bool`   |    -     | -                           | `false`  | Tells when the button should present a disabled state                                  |
-| `onClick`    | `func`   |    ✅    | -                           | -        | What the button will trigger when clicked                                              |
-| `color`      | `oneOf`  |    -     | `solid`, `primary`          | `solid`  | Sets the color of the icon                                                             |
+| Props        | Type                  | Required | Values                      | Default  | Description                                                                                                                                  |
+| ------------ | --------------------- | :------: | --------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------- |
+| `type`       | `string`              |    -     | `submit`, `reset`, `button` | `button` | Used as the HTML `type` attribute.                                                                                                           |
+| `label`      | `string`              |    ✅    | -                           | -        | Should describe what the button does, for accessibility purposes (screen-reader users)                                                       |
+| `icon`       | `node`                |    ✅    | -                           | -        | An `Icon` component                                                                                                                          |
+| `isDisabled` | `bool`                |    -     | -                           | `false`  | Tells when the button should present a disabled state                                                                                        |
+| `onClick`    | `func`                |    ✅    | -                           | -        | What the button will trigger when clicked                                                                                                    |
+| `color`      | `oneOf`               |    -     | `solid`, `primary`          | `solid`  | Sets the color of the icon                                                                                                                   |
+| `as`         | `string` or `element` |    -     | -                           | -        | You may pass in a string like "a" to have the button render as an anchor tag instead. Or you could pass in a React Component, like a `Link`. |
+
+The component further forwards all valid HTML attributes to the underlying `button` component.
 
 #### Note
 

src/components/buttons/secondary-icon-button/secondary-icon-button.js

@@ -1,18 +1,23 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import filterAriaAttributes from '../../../utils/filter-aria-attributes';
-import filterDataAttributes from '../../../utils/filter-data-attributes';
+import omit from 'lodash/omit';
+import filterInvalidAttributes from '../../../utils/filter-invalid-attributes';
 import AccessibleButton from '../accessible-button';
 import { getBaseStyles } from './secondary-icon-button.styles';
 
+const propsToOmit = ['type'];
+
 export const SecondaryIconButton = props => {
   const buttonAttributes = {
+    ...filterInvalidAttributes(omit(props, propsToOmit)),
     'data-track-component': 'SecondaryIconButton',
-    ...filterAriaAttributes(props),
-    ...filterDataAttributes(props),
+    // if there is a divergence between `isDisabled` and `disabled`,
+    // we fall back to `isDisabled`
+    disabled: props.isDisabled,
   };
   return (
     <AccessibleButton
+      as={props.as}
       type={props.type}
       buttonAttributes={buttonAttributes}
       label={props.label}
@@ -26,7 +31,9 @@ export const SecondaryIconButton = props => {
 };
 
 SecondaryIconButton.displayName = 'SecondaryIconButton';
+
 SecondaryIconButton.propTypes = {
+  as: PropTypes.oneOfType([PropTypes.string, PropTypes.elementType]),
   type: PropTypes.oneOf(['submit', 'reset', 'button']),
   icon: PropTypes.element.isRequired,
   color: PropTypes.oneOf(['solid', 'primary']),

src/components/buttons/secondary-icon-button/secondary-icon-button.spec.js

@@ -1,4 +1,5 @@
 import React from 'react';
+import { Link } from 'react-router-dom';
 import { render } from '../../../test-utils';
 import { PlusBoldIcon } from '../../icons';
 import SecondaryIconButton from './secondary-icon-button';
@@ -43,6 +44,32 @@ describe('rendering', () => {
       'true'
     );
   });
+  describe('when there is a divergence between `disabled` and `isDisabled`', () => {
+    describe('when `isDisabled` and not `disabled`', () => {
+      it('should favour `isDisabled`', () => {
+        const { getByLabelText } = render(
+          <SecondaryIconButton {...props} isDisabled={true} disabled={false} />
+        );
+        expect(getByLabelText('test-button')).toHaveAttribute('disabled');
+        expect(getByLabelText('test-button')).toHaveAttribute(
+          'aria-disabled',
+          'true'
+        );
+      });
+    });
+    describe('when not `isDisabled` and `disabled`', () => {
+      it('should favour `isDisabled`', () => {
+        const { getByLabelText } = render(
+          <SecondaryIconButton {...props} isDisabled={false} disabled={true} />
+        );
+        expect(getByLabelText('test-button')).not.toHaveAttribute('disabled');
+        expect(getByLabelText('test-button')).not.toHaveAttribute(
+          'aria-disabled',
+          'true'
+        );
+      });
+    });
+  });
   describe('type variations', () => {
     it('should render a button of type "button"', () => {
       const { getByLabelText } = render(<SecondaryIconButton {...props} />);
@@ -61,4 +88,42 @@ describe('rendering', () => {
       expect(getByLabelText('test-button')).toHaveAttribute('type', 'reset');
     });
   });
+  describe('when used with `as`', () => {
+    describe('when as is a valid HTML element', () => {
+      it('should render as that HTML element', () => {
+        const { container } = render(
+          <SecondaryIconButton
+            {...props}
+            as="a"
+            href="https://www.kanyetothe.com"
+            target="_BLANK"
+          />
+        );
+        const linkButton = container.querySelector('a');
+        expect(linkButton).toHaveAttribute(
+          'href',
+          'https://www.kanyetothe.com'
+        );
+        expect(linkButton).not.toHaveAttribute('type', 'button');
+        expect(linkButton).toHaveAttribute('target', '_BLANK');
+      });
+    });
+    describe('when as is a React component', () => {
+      it('should render as that component', () => {
+        const { getByLabelText } = render(
+          <SecondaryIconButton
+            {...props}
+            as={Link}
+            to="foo/bar"
+            target="_BLANK"
+          />
+        );
+
+        const linkButton = getByLabelText('test-button');
+        expect(linkButton).toHaveAttribute('href', '/foo/bar');
+        expect(linkButton).toHaveAttribute('target', '_BLANK');
+        expect(linkButton).not.toHaveAttribute('type', 'button');
+      });
+    });
+  });
 });