Separate Icons from Button component

Author: catarak

Diff for: .gitignore

@@ -17,3 +17,5 @@ cert_chain.crt
 localhost.crt
 localhost.key
 privkey.pem
+
+storybook-static

Diff for: .storybook/main.js

@@ -1,3 +1,5 @@
+const path = require('path');
+
 module.exports = {
   stories: ['../client/**/*.stories.(jsx|mdx)'],
   addons: [
@@ -10,6 +12,18 @@ module.exports = {
   webpackFinal: async config => {
     // do mutation to the config
 
+    const rules = config.module.rules;
+
+    // modify storybook's file-loader rule to avoid conflicts with svgr
+    const fileLoaderRule = rules.find(rule => rule.test.test('.svg'));
+    fileLoaderRule.exclude = path.resolve(__dirname, '../client');
+
+    // use svgr for svg files
+    rules.push({
+      test: /\.svg$/,
+      use: ["@svgr/webpack"],
+    })
+
     return config;
   },
 };

Diff for: client/common/Button.jsx

@@ -4,7 +4,6 @@ import styled from 'styled-components';
 import { Link } from 'react-router';
 
 import { remSize, prop } from '../theme';
-import Icon, { ValidIconNameType } from './Icon';
 
 const kinds = {
   block: 'block',
@@ -151,14 +150,8 @@ const StyledIconButton = styled.button`
  * A Button performs an primary action
  */
 const Button = ({
-  children, href, iconAfterName, iconBeforeName, kind, label, to, type, ...props
+  children, href, kind, 'aria-label': ariaLabel, to, type, ...props
 }) => {
-  const IconAfter = Icon[iconAfterName];
-  const IconBefore = Icon[iconBeforeName];
-  const hasChildren = React.Children.count(children) > 0;
-
-  const content = <>{IconBefore}{hasChildren && <span>{children}</span>}{IconAfter}</>;
-
   let StyledComponent = StyledButton;
 
   if (kind === kinds.inline) {
@@ -168,69 +161,58 @@ const Button = ({
   }
 
   if (href) {
-    return <StyledComponent kind={kind} as="a" aria-label={label} href={href} {...props}>{content}</StyledComponent>;
+    return <StyledComponent kind={kind} as="a" aria-label={ariaLabel} href={href} {...props}>{children}</StyledComponent>;
   }
 
   if (to) {
-    return <StyledComponent kind={kind} as={Link} aria-label={label} to={to} {...props}>{content}</StyledComponent>;
+    return <StyledComponent kind={kind} as={Link} aria-label={ariaLabel} to={to} {...props}>{children}</StyledComponent>;
   }
 
-  return <StyledComponent kind={kind} aria-label={label} type={type} {...props}>{content}</StyledComponent>;
+  return <StyledComponent kind={kind} aria-label={ariaLabel} type={type} {...props}>{children}</StyledComponent>;
 };
 
 Button.defaultProps = {
-  children: null,
-  disabled: false,
-  iconAfterName: null,
-  iconBeforeName: null,
-  kind: kinds.block,
-  href: null,
-  label: null,
-  to: null,
-  type: 'button',
+  'children': null,
+  'disabled': false,
+  'kind': kinds.block,
+  'href': null,
+  'aria-label': null,
+  'to': null,
+  'type': 'button',
 };
 
-Button.iconNames = Object.keys(Icon);
 Button.kinds = kinds;
 
 Button.propTypes = {
   /**
    * The visible part of the button, telling the user what
    * the action is
    */
-  children: PropTypes.element,
+  'children': PropTypes.element,
   /**
     If the button can be activated or not
   */
-  disabled: PropTypes.bool,
-  /**
-   * Name of icon to place before child content
-   */
-  iconAfterName: ValidIconNameType,
-  /**
-   * Name of icon to place after child content
-   */
-  iconBeforeName: ValidIconNameType,
+  'disabled': PropTypes.bool,
   /**
    * The kind of button - determines how it appears visually
    */
-  kind: PropTypes.oneOf(Object.values(kinds)),
+  'kind': PropTypes.oneOf(Object.values(kinds)),
   /**
    * Specifying an href will use an <a> to link to the URL
    */
-  href: PropTypes.string,
+  'href': PropTypes.string,
   /*
    * An ARIA Label used for accessibility
    */
-  label: PropTypes.string,
+  'aria-label': PropTypes.string,
   /**
    * Specifying a to URL will use a react-router Link
    */
-  to: PropTypes.string,
+  'to': PropTypes.string,
   /**
    * If using a button, then type is defines the type of button
    */
-  type: PropTypes.oneOf(['button', 'submit']),
+  'type': PropTypes.oneOf(['button', 'submit']),
 };
 
 export default Button;

Diff for: client/common/Button.stories.jsx

@@ -3,6 +3,9 @@ import { action } from '@storybook/addon-actions';
 import { boolean, text } from '@storybook/addon-knobs';
 
 import Button from './Button';
+import Icons from './Icons';
+
+const { Github, DropdownArrow, Plus } = Icons;
 
 export default {
   title: 'Common/Button',
@@ -36,17 +39,28 @@ export const ReactRouterLink = () => (
 );
 
 export const ButtonWithIconBefore = () => (
-  <Button iconBeforeName={Button.iconNames.Github}>Create</Button>
+  <Button>
+    <Github aria-label="Github logo" />
+    <span>Create</span>
+  </Button>
 );
 
 export const ButtonWithIconAfter = () => (
-  <Button iconAfterName={Button.iconNames.Github}>Create</Button>
+  <Button>
+    <span>Create</span>
+    <Github aria-label="Github logo" />
+  </Button>
 );
 
 export const InlineButtonWithIconAfter = () => (
-  <Button kind={Button.kinds.inline} iconAfterName={Button.iconNames.SortArrowDown}>File name</Button>
+  <Button kind={Button.kinds.inline}>
+    <span>File name</span>
+    <DropdownArrow />
+  </Button>
 );
 
 export const InlineIconOnlyButton = () => (
-  <Button kind={Button.kinds.inline} iconAfterName={Button.iconNames.Plus} label="Add to collection" />
+  <Button kind={Button.kinds.inline} aria-label="Add to collection">
+    <Plus />
+  </Button>
 );

Diff for: client/common/Icon.jsx

@@ -0,0 +1,55 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import SortArrowUp from '../images/sort-arrow-up.svg';
+import SortArrowDown from '../images/sort-arrow-down.svg';
+import Github from '../images/github.svg';
+import Google from '../images/google.svg';
+import Plus from '../images/plus-icon.svg';
+import Close from '../images/close.svg';
+import DropdownArrow from '../images/down-filled-triangle.svg';
+
+// HOC that adds the right web accessibility props
+// https://www.scottohara.me/blog/2019/05/22/contextual-images-svgs-and-a11y.html
+
+// could also give these a default size, color, etc. based on the theme
+// Need to add size to these - like small icon, medium icon, large icon. etc.
+function withLabel(Icon) {
+  const render = (props) => {
+    const { 'aria-label': ariaLabel } = props;
+    if (ariaLabel) {
+      return (<Icon
+        {...props}
+        aria-label={ariaLabel}
+        role="img"
+        focusable="false"
+      />);
+    }
+    return (<Icon
+      {...props}
+      aria-hidden
+      focusable="false"
+    />);
+  };
+
+  render.propTypes = {
+    'aria-label': PropTypes.string
+  };
+
+  render.defaultProps = {
+    'aria-label': null
+  };
+
+  return render;
+}
+
+const Icons = {
+  SortArrowUp: withLabel(SortArrowUp),
+  SortArrowDown: withLabel(SortArrowDown),
+  Github: withLabel(Github),
+  Google: withLabel(Google),
+  Plus: withLabel(Plus),
+  Close: withLabel(Close),
+  DropdownArrow: withLabel(DropdownArrow)
+};
+
+export default Icons;

Diff for: client/common/Icons.jsx

@@ -1,17 +1,17 @@
 import React from 'react';
 import { select } from '@storybook/addon-knobs';
 
-import Icon from './Icon';
+import Icons from './Icons';
 
 export default {
-  title: 'Common/Icon',
-  component: Icon
+  title: 'Common/Icons',
+  component: Icons
 };
 
 export const AllIcons = () => {
-  const names = Object.keys(Icon);
+  const names = Object.keys(Icons);
 
-  const SelectedIcon = Icon[select('name', names, names[0])];
+  const SelectedIcon = Icons[select('name', names, names[0])];
   return (
     <SelectedIcon />
   );

Diff for: client/common/Icon.stories.jsx renamed to client/common/Icons.stories.jsx

@@ -2,6 +2,7 @@ import PropTypes from 'prop-types';
 import React from 'react';
 
 import Button from '../../../common/Button';
+import Icons from '../../../common/Icons';
 import CopyableInput from '../../IDE/components/CopyableInput';
 
 import APIKeyList from './APIKeyList';
@@ -80,14 +81,12 @@ class APIKeyForm extends React.Component {
               value={this.state.keyLabel}
             />
             <Button
-              iconBeforeName={Button.iconNames.Plus}
               disabled={this.state.keyLabel === ''}
               type="submit"
               label="Create new key"
             >
-              {/* TODO make sure this aria label is right for the button */}
-              {/* <PlusIcon className="api-key-form__create-icon" focusable="false" aria-hidden="true" /> */}
-              Create
+              <Icons.Plus />
+              <span>Create</span>
             </Button>
           </form>
 

Diff for: client/modules/User/components/APIKeyForm.jsx

@@ -8,6 +8,7 @@ import { bindActionCreators } from 'redux';
 import classNames from 'classnames';
 
 import Button from '../../../common/Button';
+import Icons from '../../../common/Icons';
 import * as ProjectActions from '../../IDE/actions/project';
 import * as ProjectsActions from '../../IDE/actions/projects';
 import * as CollectionsActions from '../../IDE/actions/collections';
@@ -22,7 +23,6 @@ import AddToCollectionSketchList from '../../IDE/components/AddToCollectionSketc
 import CopyableInput from '../../IDE/components/CopyableInput';
 import { SketchSearchbar } from '../../IDE/components/Searchbar';
 
-import DropdownArrowIcon from '../../../images/down-arrow.svg';
 import ArrowUpIcon from '../../../images/sort-arrow-up.svg';
 import ArrowDownIcon from '../../../images/sort-arrow-down.svg';
 import RemoveIcon from '../../../images/close.svg';
@@ -53,17 +53,11 @@ const ShareURL = ({ value }) => {
   return (
     <div className="collection-share" ref={node}>
       <Button
-        iconAfterName={Button.iconNames.SortArrowDown}
         onClick={() => setShowURL(!showURL)}
-      >Share
-      </Button>
-      {/* TODO make sure this has the right aria-label and SVG attributes */}
-      {/* <button>
-        aria-label="Show collection share URL"
       >
-        <span>Share</span>
-        <DropdownArrowIcon className="collection-share__arrow" focusable="false" aria-hidden="true" />
-      </button> */}
+        Share
+        <Icons.DropdownArrow />
+      </Button>
       { showURL &&
         <div className="collection__share-dropdown">
           <CopyableInput value={value} label="Link to Collection" />