fix(react-button): add prop validation

- Fix large/block props to be booleans, not strings [#86938454]
pivotal-cf · Jan 30, 2015 · 5fd8f4a · 5fd8f4a
1 parent a2faeb7
commit 5fd8f4a
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 104 deletions.
diff --git a/src/pivotal-ui/components/buttons.scss b/src/pivotal-ui/components/buttons.scss
@@ -150,15 +150,15 @@ Buttons use the button tag by default. If you'd like a link rather than a button
 To make a button large, set the `large` property to true.
 
 ```react_example_table
-<UI.HighlightButton large="true">
+<UI.HighlightButton large={true}>
   Big Button
 </UI.HighlightButton>
 ```
 
 To make a button full-width, set the `block` property to true.
 
 ```react_example
-<UI.DangerButton block="true" href="http://i.imgur.com/oq2qkUN.gif">
+<UI.DangerButton block={true} href="http://i.imgur.com/oq2qkUN.gif">
   Danger Zone
 </UI.DangerButton>
 ```

diff --git a/src/pivotal-ui/components/pivnet_homepage.scss b/src/pivotal-ui/components/pivnet_homepage.scss
@@ -22,7 +22,7 @@ var PivnetHomepage = React.createClass({
         <UI.Pane className="bg-shadow-1">
           <div className="row">
             <div className="col-md-8 col-md-offset-8">
-              <UI.HighlightAltButton className="mtxxl" block="true" large="true">Join</UI.HighlightAltButton>
+              <UI.HighlightAltButton className="mtxxl" block={true} large={true}>Join</UI.HighlightAltButton>
               <p className="type-sm type-neutral-6">To download and evaluate all products and services</p>
             </div>
           </div>

diff --git a/src/pivotal-ui/javascripts/buttons.jsx b/src/pivotal-ui/javascripts/buttons.jsx
@@ -1,130 +1,86 @@
 'use strict';
 
-var React = require('react');
-var _ = require('lodash');
+var React = require('react/addons');
+var classSet = React.addons.classSet;
+
+var ButtonMixin = require('./mixins/button-mixin');
 
 var UIButton = React.createClass({
-  acceptedTypes: ['default', 'default-alt', 'primary', 'lowlight', 'danger', 'highlight', 'highlight-alt'],
-  render: function () {
-    var classes = ['btn'];
+  mixins: [ButtonMixin],
 
-    if (_.contains(this.acceptedTypes, this.props.type)) {
-      classes.push('btn-' + this.props.type);
-    } else {
-      classes.push('btn-default');
-    }
+  render: function () {
+    var {block, href, large, type, children, ...others} = this.props;
 
-    if (this.props.block) {
-      classes.push('btn-block');
-    }
+    var classes = classSet({
+      btn: true,
+      'btn-block': block,
+      'btn-lg': large
+    });
 
-    if (this.props.large) {
-      classes.push('btn-lg');
+    if (type) {
+      classes += ' btn-' + type;
+    } else {
+      classes += ' btn-default';
     }
 
-    classes = classes.join(" ");
-
-    if (this.props.href) {
+    if (href) {
       return (
-        <a {...this.props} className={classes} href={this.props.href}>{this.props.children}</a>
+        <a {...others} className={classes} href={href}>{children}</a>
       );
     } else {
       return (
-        <button {...this.props} className={classes}>{this.props.children}</button>
+        <button {...others} className={classes}>{children}</button>
       );
     }
   }
 });
 
 var DefaultButton = React.createClass({
-  getDefaultProps: function () {
-    return {
-      type: 'default'
-    };
-  },
-
+  mixins: [ButtonMixin],
   render: function render() {
-    return (
-      <UIButton {...this.props}>{this.props.children}</UIButton>
-      );
+    return <UIButton {...this.props} type='default' />;
   }
 });
-var DefaultAltButton = React.createClass({
-  getDefaultProps: function () {
-    return {
-      type: 'default-alt'
-    };
-  },
 
+var DefaultAltButton = React.createClass({
+  mixins: [ButtonMixin],
   render: function render() {
-    return (
-      <UIButton {...this.props}>{this.props.children}</UIButton>
-      );
+    return <UIButton {...this.props} type='default-alt' />;
   }
 });
-var PrimaryButton = React.createClass({
-  getDefaultProps: function () {
-    return {
-      type: 'primary'
-    };
-  },
 
+var PrimaryButton = React.createClass({
+  mixins: [ButtonMixin],
   render: function render() {
-    return (
-      <UIButton {...this.props}>{this.props.children}</UIButton>
-      );
+    return <UIButton {...this.props} type='primary' />;
   }
 });
-var LowlightButton = React.createClass({
-  getDefaultProps: function () {
-    return {
-      type: 'lowlight'
-    };
-  },
 
+var LowlightButton = React.createClass({
+  mixins: [ButtonMixin],
   render: function render() {
-    return (
-      <UIButton {...this.props}>{this.props.children}</UIButton>
-      );
+    return <UIButton {...this.props} type='lowlight' />;
   }
 });
-var DangerButton = React.createClass({
-  getDefaultProps: function () {
-    return {
-      type: 'danger'
-    };
-  },
 
+var DangerButton = React.createClass({
+  mixins: [ButtonMixin],
   render: function render() {
-    return (
-      <UIButton {...this.props}>{this.props.children}</UIButton>
-      );
+    return <UIButton {...this.props} type='danger' />;
   }
 });
-var HighlightButton = React.createClass({
-  getDefaultProps: function () {
-    return {
-      type: 'highlight'
-    };
-  },
 
+var HighlightButton = React.createClass({
+  mixins: [ButtonMixin],
   render: function render() {
-    return (
-      <UIButton {...this.props}>{this.props.children}</UIButton>
-      );
+    return <UIButton {...this.props} type='highlight' />;
   }
 });
-var HighlightAltButton = React.createClass({
-  getDefaultProps: function () {
-    return {
-      type: 'highlight-alt'
-    };
-  },
 
+var HighlightAltButton = React.createClass({
+  mixins: [ButtonMixin],
   render: function render() {
-    return (
-      <UIButton {...this.props}>{this.props.children}</UIButton>
-      );
+    return <UIButton {...this.props} type='highlight-alt' />;
   }
 });
 

diff --git a/src/pivotal-ui/javascripts/mixins/button-mixin.js b/src/pivotal-ui/javascripts/mixins/button-mixin.js
@@ -0,0 +1,22 @@
+'use strict';
+
+var React = require('react');
+
+var ButtonMixin = {
+  propTypes: {
+    block: React.PropTypes.bool,
+    href: React.PropTypes.string,
+    large: React.PropTypes.bool,
+    type: React.PropTypes.oneOf([
+      'default',
+      'default-alt',
+      'primary',
+      'lowlight',
+      'danger',
+      'highlight',
+      'highlight-alt'
+    ])
+  }
+};
+
+module.exports = ButtonMixin;
diff --git a/test/javascripts/button_spec.js b/test/javascripts/button_spec.js
@@ -60,27 +60,11 @@ describe('UIButton', function() {
     });
   });
 
-  describe("when type attribute is invalid", function() {
-    beforeEach(function(){
-      React.render(
-        UIButton({
-          type: "fop"
-        }),
-        this.node
-      );
-    });
-
-    it("does not add the type class to the button", function() {
-      expect($('#container button.btn')).not.toHaveClass('btn-fop');
-      expect($('#container button.btn')).toHaveClass('btn-default');
-    });
-  });
-
   describe("when block is true", function() {
     beforeEach(function(){
       React.render(
         UIButton({
-          block: "true"
+          block: true
         }),
         this.node
       );
@@ -95,7 +79,7 @@ describe('UIButton', function() {
     beforeEach(function(){
       React.render(
         UIButton({
-          large: "true"
+          large: true
         }),
         this.node
       );