From 73daba78c420010ee584b6351161ac7fe1fb0ad8 Mon Sep 17 00:00:00 2001
From: Jimmy Jia <tesrin@gmail.com>
Date: Tue, 6 Oct 2015 13:46:20 -0400
Subject: [PATCH] [fixed] Show toggle button when using NavBrand

Also simplified the rendering code a bit.
---
 src/NavBrand.js    |  48 ++++++-------
 src/Navbar.js      | 164 +++++++++++++++++++++++++--------------------
 test/NavbarSpec.js |  38 ++++-------
 3 files changed, 125 insertions(+), 125 deletions(-)

diff --git a/src/NavBrand.js b/src/NavBrand.js
index 5aa2eaaffd..397f193c6d 100644
--- a/src/NavBrand.js
+++ b/src/NavBrand.js
@@ -1,38 +1,32 @@
-import React, { cloneElement } from 'react';
-import BootstrapMixin from './BootstrapMixin';
 import classNames from 'classnames';
+import React from 'react';
 
-const NavBrand = React.createClass({
-  mixins: [BootstrapMixin],
-
-  propTypes: {
-    bsRole: React.PropTypes.string,
-    navbar: React.PropTypes.bool
-  },
-
-  getDefaultProps() {
-    return {
-      bsRole: 'brand',
-      navbar: false
-    };
-  },
-
+class NavBrand extends React.Component {
   render() {
-    let brand;
+    const {className, children, ...props} = this.props;
 
-    if (React.isValidElement(this.props.children)) {
-      brand = cloneElement(this.props.children, {
-        className: classNames(this.props.children.props.className, 'navbar-brand'),
-        bsRole: this.props.bsRole,
-        navbar: this.props.navbar
+    if (React.isValidElement(children)) {
+      return React.cloneElement(children, {
+        className: classNames(
+          children.props.className, className, 'navbar-brand'
+        )
       });
-    } else {
-      brand = <span {...this.props} className="navbar-brand">{this.props.children}</span>;
     }
 
-    return brand;
+    return (
+      <span {...props} className={classNames(className, 'navbar-brand')}>
+        {children}
+      </span>
+    );
   }
+}
+
+NavBrand.propTypes = {
+  bsRole: React.PropTypes.string
+};
 
-});
+NavBrand.defaultProps = {
+  bsRole: 'brand'
+};
 
 export default NavBrand;
diff --git a/src/Navbar.js b/src/Navbar.js
index a5f85ecdf2..ceb73e8e19 100644
--- a/src/Navbar.js
+++ b/src/Navbar.js
@@ -1,11 +1,14 @@
-import React, { cloneElement } from 'react';
-import BootstrapMixin from './BootstrapMixin';
 import classNames from 'classnames';
+import React from 'react';
+import elementType from 'react-prop-types/lib/elementType';
+
+import BootstrapMixin from './BootstrapMixin';
+import Grid from './Grid';
+import NavBrand from './NavBrand';
 
-import ValidComponentChildren from './utils/ValidComponentChildren';
 import createChainedFunction from './utils/createChainedFunction';
-import elementType from 'react-prop-types/lib/elementType';
 import deprecationWarning from './utils/deprecationWarning';
+import ValidComponentChildren from './utils/ValidComponentChildren';
 
 const Navbar = React.createClass({
   mixins: [BootstrapMixin],
@@ -58,7 +61,8 @@ const Navbar = React.createClass({
     return !this._isChanging;
   },
 
-  componentDidMount() {
+  componentWillMount() {
+    // TODO: Use the `deprecated` PropType once we're on React 0.14.
     if (this.props.brand) {
       deprecationWarning('Navbar brand attribute', 'NavBrand Component');
     }
@@ -80,106 +84,118 @@ const Navbar = React.createClass({
     return this.props.navExpanded != null ? this.props.navExpanded : this.state.navExpanded;
   },
 
-  navbrandChild() {
-    let navChild =
-      ValidComponentChildren.findValidComponents(this.props.children, child => {
-        return child.props.bsRole === 'brand';
-      });
-
-    return navChild;
-  },
-
-  hasNavbrandChild() {
-    return this.navbrandChild().length > 0;
+  hasNavBrandChild() {
+    return ValidComponentChildren.findValidComponents(
+      this.props.children, child => child.props.bsRole === 'brand'
+    ).length > 0;
   },
 
   render() {
-    let classes = this.getBsClassSet();
-    let ComponentClass = this.props.componentClass;
-
-    classes['navbar-fixed-top'] = this.props.fixedTop;
-    classes['navbar-fixed-bottom'] = this.props.fixedBottom;
-    classes['navbar-static-top'] = this.props.staticTop;
-    classes['navbar-inverse'] = this.props.inverse;
-
-    let displayHeader = (this.props.brand || this.props.toggleButton || this.props.toggleNavKey != null) && !this.hasNavbrandChild();
+    const {
+      brand,
+      toggleButton,
+      toggleNavKey,
+      fixedTop,
+      fixedBottom,
+      staticTop,
+      inverse,
+      componentClass: ComponentClass,
+      fluid,
+      className,
+      children,
+      ...props
+    } = this.props;
+
+    const classes = this.getBsClassSet();
+    classes['navbar-fixed-top'] = fixedTop;
+    classes['navbar-fixed-bottom'] = fixedBottom;
+    classes['navbar-static-top'] = staticTop;
+    classes['navbar-inverse'] = inverse;
+
+    const showHeader =
+      (brand || toggleButton || toggleNavKey != null) &&
+      !this.hasNavBrandChild();
 
     return (
-      <ComponentClass {...this.props} className={classNames(this.props.className, classes)}>
-        <div className={this.props.fluid ? 'container-fluid' : 'container'}>
-          {displayHeader ? this.renderHeader() : null}
-          {ValidComponentChildren.map(this.props.children, this.renderChildren)}
-        </div>
+      <ComponentClass {...props} className={classNames(className, classes)}>
+        <Grid fluid={fluid}>
+          {showHeader ? this.renderBrandHeader() : null}
+          {ValidComponentChildren.map(children, this.renderChild)}
+        </Grid>
       </ComponentClass>
     );
   },
 
-  renderNavBrand(child, index) {
-    let navbrandEl = cloneElement(child, {
-      navbar: true,
-      toggleNavKey: this.props.toggleNavKey,
-      toggleButton: this.props.toggleButton,
-      handleToggle: this.handleToggle,
-      key: child.key ? child.key : index
-    });
-
-    return this.renderHeader(navbrandEl);
-  },
-
-  renderChild(child, index) {
-    return cloneElement(child, {
-      navbar: true,
-      collapsible: this.props.toggleNavKey != null && this.props.toggleNavKey === child.props.eventKey,
-      expanded: this.props.toggleNavKey != null && this.props.toggleNavKey === child.props.eventKey && this.isNavExpanded(),
-      key: child.key ? child.key : index
-    });
-  },
+  renderBrandHeader() {
+    let {brand} = this.props;
+    if (brand) {
+      brand = <NavBrand>{brand}</NavBrand>;
+    }
 
-  renderChildren(child, index) {
-    return (child.props.navbrand) ? this.renderNavBrand(child, index) : this.renderChild(child, index);
+    return this.renderHeader(brand);
   },
 
-  renderHeader(navbrandEl) {
-    let brand = navbrandEl || '';
-
-    if (!brand && this.props.brand) {
-      if (React.isValidElement(this.props.brand)) {
-        brand = cloneElement(this.props.brand, {
-          className: classNames(this.props.brand.props.className, 'navbar-brand')
-        });
-      } else {
-        brand = <span className="navbar-brand">{this.props.brand}</span>;
-      }
-    }
+  renderHeader(brand) {
+    const hasToggle =
+      this.props.toggleButton || this.props.toggleNavKey != null;
 
     return (
       <div className="navbar-header">
         {brand}
-        {(this.props.toggleButton || this.props.toggleNavKey != null) ? this.renderToggleButton() : null}
+        {hasToggle ? this.renderToggleButton() : null}
       </div>
     );
   },
 
-  renderToggleButton() {
-    let children;
+  renderChild(child, index) {
+    const key = child.key != null ? child.key : index;
+
+    if (child.props.bsRole === 'brand') {
+      return React.cloneElement(this.renderHeader(child), {key});
+    }
+
+    const {toggleNavKey} = this.props;
+    const collapsible =
+      toggleNavKey != null && toggleNavKey === child.props.eventKey;
 
-    if (React.isValidElement(this.props.toggleButton)) {
-      return cloneElement(this.props.toggleButton, {
-        className: classNames(this.props.toggleButton.props.className, 'navbar-toggle'),
-        onClick: createChainedFunction(this.handleToggle, this.props.toggleButton.props.onClick)
+    return React.cloneElement(child, {
+      navbar: true,
+      collapsible,
+      expanded: collapsible && this.isNavExpanded(),
+      key
+    });
+  },
+
+  renderToggleButton() {
+    const {toggleButton} = this.props;
+
+    if (React.isValidElement(toggleButton)) {
+      return React.cloneElement(toggleButton, {
+        className: classNames(toggleButton.props.className, 'navbar-toggle'),
+        onClick: createChainedFunction(
+          this.handleToggle, toggleButton.props.onClick
+        )
       });
     }
 
-    children = (this.props.toggleButton != null) ?
-      this.props.toggleButton : [
+    let children;
+    if (toggleButton != null) {
+      children = toggleButton;
+    } else {
+      children = [
         <span className="sr-only" key={0}>Toggle navigation</span>,
         <span className="icon-bar" key={1}></span>,
         <span className="icon-bar" key={2}></span>,
         <span className="icon-bar" key={3}></span>
       ];
+    }
 
     return (
-      <button className="navbar-toggle" type="button" onClick={this.handleToggle}>
+      <button
+        type="button"
+        onClick={this.handleToggle}
+        className="navbar-toggle"
+      >
         {children}
       </button>
     );
diff --git a/test/NavbarSpec.js b/test/NavbarSpec.js
index 13b3aec06b..a0bcddcaa1 100644
--- a/test/NavbarSpec.js
+++ b/test/NavbarSpec.js
@@ -147,30 +147,6 @@ describe('Navbar', () => {
     assert.equal(React.findDOMNode(brands[0]).innerText, 'Brand');
   });
 
-  it('Should pass navbar prop to navbrand', () => {
-    let instance = ReactTestUtils.renderIntoDocument(
-      <Navbar>
-        <NavBrand>Brand</NavBrand>
-      </Navbar>
-    );
-
-    let brand = ReactTestUtils.findRenderedDOMComponentWithClass(instance, 'navbar-brand');
-
-    assert.ok(brand.props.navbar);
-  });
-
-  it('Should pass navbar prop to navbrand inner element', () => {
-    let instance = ReactTestUtils.renderIntoDocument(
-      <Navbar>
-        <NavBrand><a href>Brand</a></NavBrand>
-      </Navbar>
-    );
-
-    let brand = ReactTestUtils.findRenderedDOMComponentWithClass(instance, 'navbar-brand');
-
-    assert.ok(brand.props.navbar);
-  });
-
   it('Should pass navbar prop to navs', () => {
     let instance = ReactTestUtils.renderIntoDocument(
       <Navbar>
@@ -236,4 +212,18 @@ describe('Navbar', () => {
 
     assert.ok(header);
   });
+
+  it('Should show toggle button when using NavBrand', () => {
+    const instance = ReactTestUtils.renderIntoDocument(
+      <Navbar toggleNavKey={0}>
+        <NavBrand>Brand</NavBrand>
+        <Nav eventKey={0} />
+      </Navbar>
+    );
+
+    const toggle = ReactTestUtils.findRenderedDOMComponentWithClass(
+      instance, 'navbar-toggle'
+    );
+    expect(toggle).to.be.ok;
+  });
 });