Refactor kuiButton to not use createElement, and instead exit early a…

…nd return the correct element.
elastic · Mar 28, 2017 · 31219b3 · 31219b3
1 parent 0132a55
commit 31219b3
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 59 deletions.
diff --git a/ui_framework/components/button/__snapshots__/button.test.js.snap b/ui_framework/components/button/__snapshots__/button.test.js.snap
@@ -12,8 +12,6 @@ exports[`KuiButton Baseline is rendered 1`] = `
 <button
   className="kuiButton"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span />
 </button>
@@ -23,8 +21,6 @@ exports[`KuiButton Props children is rendered 1`] = `
 <button
   className="kuiButton"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span>
     <span>
@@ -38,8 +34,6 @@ exports[`KuiButton Props className renders the classes 1`] = `
 <button
   className="kuiButton testClass1 testClass2"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span />
 </button>
@@ -50,8 +44,6 @@ exports[`KuiButton Props href changes the element from a button to an anchor tag
   className="kuiButton"
   href="#"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span />
 </a>
@@ -61,8 +53,6 @@ exports[`KuiButton Props icon is rendered with children 1`] = `
 <button
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span>
     Icon
@@ -77,8 +67,6 @@ exports[`KuiButton Props icon is rendered without children 1`] = `
 <button
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span>
     Icon
@@ -91,8 +79,6 @@ exports[`KuiButton Props isDisabled sets the disabled attribute 1`] = `
   className="kuiButton"
   disabled={true}
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span />
 </button>
@@ -102,8 +88,6 @@ exports[`KuiButton Props isIconOnRight moves the icon to the right 1`] = `
 <button
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span>
     <span>
@@ -118,8 +102,6 @@ exports[`KuiButton Props isLoading doesn't render the icon prop 1`] = `
 <button
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span>
     <KuiLoadingButtonIcon />
@@ -131,8 +113,6 @@ exports[`KuiButton Props isLoading renders a spinner 1`] = `
 <button
   className="kuiButton kuiButton--iconText"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span>
     <KuiLoadingButtonIcon />
@@ -171,9 +151,6 @@ exports[`KuiButton Props target is rendered 1`] = `
 <button
   className="kuiButton"
   onClick={[Function]}
-  target="_blank"
-  type={null}
-  value={null}
 >
   <span />
 </button>
@@ -184,8 +161,6 @@ exports[`KuiButton Props testSubject is rendered 1`] = `
   className="kuiButton"
   data-test-subj="test subject string"
   onClick={[Function]}
-  type={null}
-  value={null}
 >
   <span />
 </button>

diff --git a/ui_framework/components/button/button.js b/ui_framework/components/button/button.js
@@ -8,20 +8,6 @@ import keyMirror from 'keymirror';
 import { KuiLoadingButtonIcon } from './button_icon';
 
 const KuiButton = props => {
-  if (props.isSubmit) {
-    // input is a void element tag and can't have children.
-    if ((props.children && typeof props.children !== 'string') || props.icon) {
-      throw new Error(
-        `KuiButton with isSubmit prop set to true can only accept string children, and can't
-        display icons. This is because input is a void element and can't contain children.`
-      );
-    }
-  }
-
-  function onClick() {
-    props.onClick(props.data);
-  }
-
   const icon =
     props.isLoading
     ? <KuiLoadingButtonIcon />
@@ -41,17 +27,39 @@ const KuiButton = props => {
     );
   }
 
-  const elementType =
-    props.isSubmit
-    ? 'input'
-    : props.href
-    ? 'a'
-    : 'button';
+  // onClick is optional, so don't even call it if it's not passed in, or if we're disabled.
+  const onClick =
+    props.onClick && !props.isDisabled
+    ? () => props.onClick(props.data)
+    : () => {};
+
+  const baseProps = {
+    'data-test-subj': props.testSubject,
+    className,
+    disabled: props.isDisabled,
+    onClick,
+  };
+
+  if (props.isSubmit) {
+    // input is a void element tag and can't have children.
+    if ((props.children && typeof props.children !== 'string') || props.icon) {
+      throw new Error(
+        `KuiButton with isSubmit prop set to true can only accept string children, and can't
+        display icons. This is because input is a void element and can't contain children.`
+      );
+    }
+
+    return (
+      <input
+        type="submit"
+        value={props.children}
+        {...baseProps}
+      />
+    );
+  }
 
   const children =
-    props.isSubmit
-    ? null
-    : props.isIconOnRight
+    props.isIconOnRight
     ? (
       <span>
         {wrappedChildren}
@@ -64,17 +72,23 @@ const KuiButton = props => {
       </span>
     );
 
-  return React.createElement(elementType, {
-    'data-test-subj': props.testSubject,
-    className,
-    href: props.href,
-    target: props.target,
-    type: props.isSubmit ? 'submit' : null,
-    disabled: props.isDisabled,
-    // onClick is optional, so don't even call it if it's not passed in, or if we're disabled.
-    onClick: props.onClick && !props.isDisabled ? onClick : () => {},
-    value: props.isSubmit ? props.children : null,
-  }, children);
+  if (props.href) {
+    return (
+      <a
+        href={props.href}
+        target={props.target}
+        {...baseProps}
+      >
+        {children}
+      </a>
+    );
+  }
+
+  return (
+    <button {...baseProps}>
+      {children}
+    </button>
+  );
 };
 
 KuiButton.propTypes = {