Refactor KuiButton and KuiButtonIcon type prop to emphasize passing s…

…tring literals instead of enums.
elastic · Mar 28, 2017 · d50d081 · d50d081
1 parent 4119bfb
commit d50d081
Show file tree

Hide file tree

Showing 18 changed files with 162 additions and 139 deletions.
diff --git a/ui_framework/components/button/__snapshots__/button.test.js.snap b/ui_framework/components/button/__snapshots__/button.test.js.snap
@@ -56,17 +56,7 @@ exports[`KuiButton Props icon is rendered without children 1`] = `
 </button>
 `;
 
-exports[`KuiButton Props isDisabled sets the disabled attribute 1`] = `
-<button
-  className="kuiButton"
-  disabled={true}
-  onClick={[Function]}
->
-  <span />
-</button>
-`;
-
-exports[`KuiButton Props isIconOnRight moves the icon to the right 1`] = `
+exports[`KuiButton Props iconPosition moves the icon to the right 1`] = `
 <button
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
@@ -80,14 +70,24 @@ exports[`KuiButton Props isIconOnRight moves the icon to the right 1`] = `
 </button>
 `;
 
+exports[`KuiButton Props isDisabled sets the disabled attribute 1`] = `
+<button
+  className="kuiButton"
+  disabled={true}
+  onClick={[Function]}
+>
+  <span />
+</button>
+`;
+
 exports[`KuiButton Props isLoading doesn't render the icon prop 1`] = `
 <button
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
 >
   <span>
     <KuiButtonIcon
-      type="LOADING"
+      type="loading"
     />
   </span>
 </button>
@@ -100,7 +100,7 @@ exports[`KuiButton Props isLoading renders a spinner 1`] = `
 >
   <span>
     <KuiButtonIcon
-      type="LOADING"
+      type="loading"
     />
   </span>
 </button>

diff --git a/ui_framework/components/button/__snapshots__/link_button.test.js.snap b/ui_framework/components/button/__snapshots__/link_button.test.js.snap
@@ -66,17 +66,7 @@ exports[`KuiLinkButton Props icon is rendered without children 1`] = `
 </a>
 `;
 
-exports[`KuiLinkButton Props isDisabled sets the disabled attribute 1`] = `
-<a
-  className="kuiButton"
-  disabled={true}
-  onClick={[Function]}
->
-  <span />
-</a>
-`;
-
-exports[`KuiLinkButton Props isIconOnRight moves the icon to the right 1`] = `
+exports[`KuiLinkButton Props iconPosition moves the icon to the right 1`] = `
 <a
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
@@ -90,14 +80,24 @@ exports[`KuiLinkButton Props isIconOnRight moves the icon to the right 1`] = `
 </a>
 `;
 
+exports[`KuiLinkButton Props isDisabled sets the disabled attribute 1`] = `
+<a
+  className="kuiButton"
+  disabled={true}
+  onClick={[Function]}
+>
+  <span />
+</a>
+`;
+
 exports[`KuiLinkButton Props isLoading doesn't render the icon prop 1`] = `
 <a
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
 >
   <span>
     <KuiButtonIcon
-      type="LOADING"
+      type="loading"
     />
   </span>
 </a>
@@ -110,7 +110,7 @@ exports[`KuiLinkButton Props isLoading renders a spinner 1`] = `
 >
   <span>
     <KuiButtonIcon
-      type="LOADING"
+      type="loading"
     />
   </span>
 </a>

diff --git a/ui_framework/components/button/button.js b/ui_framework/components/button/button.js
@@ -8,30 +8,49 @@ import keyMirror from 'keymirror';
 import { KuiButtonIcon } from './button_icon/button_icon';
 
 const commonPropTypes = {
-  type: PropTypes.string,
+  type: PropTypes.oneOf([
+    'basic',
+    'hollow',
+    'danger',
+    'primary',
+  ]),
   testSubject: PropTypes.string,
   isDisabled: PropTypes.bool,
   onClick: PropTypes.func,
   data: PropTypes.any,
   className: PropTypes.string,
 };
 
+const nonVoidPropTypes = {
+  icon: PropTypes.node,
+  iconPosition: PropTypes.oneOf([
+    'left',
+    'right',
+  ]),
+  children: PropTypes.node,
+  isLoading: PropTypes.bool,
+};
+
+const nonVoidDefaultProps = {
+  iconPosition: 'left',
+};
+
 const getIcon = props => (
   props.isLoading
-    ? <KuiButtonIcon type={KuiButtonIcon.TYPE.LOADING} />
+    ? <KuiButtonIcon type="loading" />
     : props.icon
 );
 
 const getClassName = (props, icon) => {
   const typeToClassNameMap = {
-    [KuiButton.TYPE.BASIC]: 'kuiButton--basic',
-    [KuiButton.TYPE.HOLLOW]: 'kuiButton--hollow',
-    [KuiButton.TYPE.DANGER]: 'kuiButton--danger',
-    [KuiButton.TYPE.PRIMARY]: 'kuiButton--primary',
+    basic: 'kuiButton--basic',
+    hollow: 'kuiButton--hollow',
+    danger: 'kuiButton--danger',
+    primary: 'kuiButton--primary',
   };
 
   return classNames('kuiButton', props.className, {
-    [typeToClassNameMap[KuiButton.TYPE[props.type]]]: props.type,
+    [typeToClassNameMap[props.type]]: props.type,
     'kuiButton--iconText': icon,
   });
 };
@@ -41,18 +60,23 @@ const getChildren = (props, icon) => {
   // correctly.
   const wrappedChildren = props.children ? <span>{props.children}</span> : undefined;
 
-  return props.isIconOnRight
-    ? (
-      <span>
-        {wrappedChildren}
-        {icon}
-      </span>
-    ) : (
-      <span>
-        {icon}
-        {wrappedChildren}
-      </span>
-    );
+  switch(props.iconPosition) {
+    case 'left':
+      return (
+        <span>
+          {icon}
+          {wrappedChildren}
+        </span>
+      );
+
+    case 'right':
+      return (
+        <span>
+          {wrappedChildren}
+          {icon}
+        </span>
+      );
+  }
 };
 
 const getOnClick = props => (
@@ -82,13 +106,14 @@ const KuiButton = props => {
 };
 
 KuiButton.propTypes = {
-  icon: PropTypes.node,
-  isIconOnRight: PropTypes.bool,
-  children: PropTypes.node,
-  isLoading: PropTypes.bool,
+  ...nonVoidPropTypes,
   ...commonPropTypes,
 };
 
+KuiButton.defaultProps = {
+  ...nonVoidDefaultProps,
+};
+
 const KuiLinkButton = props => {
   const icon = getIcon(props);
   const children = getChildren(props, icon);
@@ -108,13 +133,14 @@ const KuiLinkButton = props => {
 KuiLinkButton.propTypes = {
   href: PropTypes.string,
   target: PropTypes.string,
-  icon: PropTypes.node,
-  isIconOnRight: PropTypes.bool,
-  children: PropTypes.node,
-  isLoading: PropTypes.bool,
+  ...nonVoidPropTypes,
   ...commonPropTypes,
 };
 
+KuiLinkButton.defaultProps = {
+  ...nonVoidDefaultProps,
+};
+
 const KuiSubmitButton = props => {
   const commonProps = getCommonProps(props);
 
@@ -132,13 +158,6 @@ KuiSubmitButton.propTypes = {
   ...commonPropTypes,
 };
 
-KuiButton.TYPE = KuiSubmitButton.TYPE = KuiLinkButton.TYPE = keyMirror({
-  BASIC: null,
-  HOLLOW: null,
-  DANGER: null,
-  PRIMARY: null,
-});
-
 export {
   KuiButton,
   KuiLinkButton,

diff --git a/ui_framework/components/button/button.test.js b/ui_framework/components/button/button.test.js
@@ -26,7 +26,7 @@ describe('KuiButton', () => {
       describe('basic', () => {
         test('renders the basic class', () => {
           const $button = render(
-            <KuiButton type={KuiButton.TYPE.BASIC} />
+            <KuiButton type="basic" />
           );
 
           expect($button)
@@ -37,7 +37,7 @@ describe('KuiButton', () => {
       describe('hollow', () => {
         test('renders the hollow class', () => {
           const $button = render(
-            <KuiButton type={KuiButton.TYPE.HOLLOW} />
+            <KuiButton type="hollow" />
           );
 
           expect($button)
@@ -48,7 +48,7 @@ describe('KuiButton', () => {
       describe('danger', () => {
         test('renders the danger class', () => {
           const $button = render(
-            <KuiButton type={KuiButton.TYPE.DANGER} />
+            <KuiButton type="danger" />
           );
 
           expect($button)
@@ -59,7 +59,7 @@ describe('KuiButton', () => {
       describe('primary', () => {
         test('renders the primary class', () => {
           const $button = render(
-            <KuiButton type={KuiButton.TYPE.PRIMARY} />
+            <KuiButton type="primary" />
           );
 
           expect($button)
@@ -101,10 +101,13 @@ describe('KuiButton', () => {
       });
     });
 
-    describe('isIconOnRight', () => {
+    describe('iconPosition', () => {
       test('moves the icon to the right', () => {
         const $button = shallow(
-          <KuiButton icon="Icon" isIconOnRight>
+          <KuiButton
+            icon="Icon"
+            iconPosition="right"
+          >
             Hello
           </KuiButton>
         );

diff --git a/ui_framework/components/button/button_icon/button_icon.js b/ui_framework/components/button/button_icon/button_icon.js
@@ -7,32 +7,30 @@ import keyMirror from 'keymirror';
 
 const KuiButtonIcon = props => {
   const typeToClassNameMap = {
-    [KuiButtonIcon.TYPE.CREATE]: 'fa-plus',
-    [KuiButtonIcon.TYPE.DELETE]: 'fa-trash',
-    [KuiButtonIcon.TYPE.PREVIOUS]: 'fa-chevron-left',
-    [KuiButtonIcon.TYPE.NEXT]: 'fa-chevron-right',
-    [KuiButtonIcon.TYPE.LOADING]: 'fa-spinner fa-spin',
+    create: 'fa-plus',
+    delete: 'fa-trash',
+    previous: 'fa-chevron-left',
+    next: 'fa-chevron-right',
+    loading: 'fa-spinner fa-spin',
   };
 
   const iconClasses = classNames('kuiButton__icon kuiIcon', props.className, {
-    [typeToClassNameMap[KuiButtonIcon.TYPE[props.type]]]: props.type,
+    [typeToClassNameMap[props.type]]: props.type,
   });
 
   return (
     <span className={iconClasses} />
   );
 };
 
-KuiButtonIcon.TYPE = keyMirror({
-  CREATE: null,
-  DELETE: null,
-  PREVIOUS: null,
-  NEXT: null,
-  LOADING: null,
-});
-
 KuiButtonIcon.propTypes = {
-  type: PropTypes.string,
+  type: PropTypes.oneOf([
+    'create',
+    'delete',
+    'previous',
+    'next',
+    'loading',
+  ]),
   className: PropTypes.string,
 };
 

diff --git a/ui_framework/components/button/button_icon/button_icon.test.js b/ui_framework/components/button/button_icon/button_icon.test.js
@@ -26,7 +26,7 @@ describe('KuiButtonIcon', () => {
       describe('create', () => {
         test('renders the create class', () => {
           const $buttonIcon = render(
-            <KuiButtonIcon type={KuiButtonIcon.TYPE.CREATE} />
+            <KuiButtonIcon type="create" />
           );
 
           expect($buttonIcon)
@@ -37,7 +37,7 @@ describe('KuiButtonIcon', () => {
       describe('delete', () => {
         test('renders the delete class', () => {
           const $buttonIcon = render(
-            <KuiButtonIcon type={KuiButtonIcon.TYPE.DELETE} />
+            <KuiButtonIcon type="delete" />
           );
 
           expect($buttonIcon)
@@ -48,7 +48,7 @@ describe('KuiButtonIcon', () => {
       describe('previous', () => {
         test('renders the previous class', () => {
           const $buttonIcon = render(
-            <KuiButtonIcon type={KuiButtonIcon.TYPE.PREVIOUS} />
+            <KuiButtonIcon type="previous" />
           );
 
           expect($buttonIcon)
@@ -59,7 +59,7 @@ describe('KuiButtonIcon', () => {
       describe('next', () => {
         test('renders the next class', () => {
           const $buttonIcon = render(
-            <KuiButtonIcon type={KuiButtonIcon.TYPE.NEXT} />
+            <KuiButtonIcon type="next" />
           );
 
           expect($buttonIcon)
@@ -70,7 +70,7 @@ describe('KuiButtonIcon', () => {
       describe('loading', () => {
         test('renders the loading class', () => {
           const $buttonIcon = render(
-            <KuiButtonIcon type={KuiButtonIcon.TYPE.LOADING} />
+            <KuiButtonIcon type="loading" />
           );
 
           expect($buttonIcon)