From f9dcdb0318891608e67e23757e7b04403c9fab46 Mon Sep 17 00:00:00 2001
From: Brett Sun <brett@ascribe.io>
Date: Fri, 17 Jun 2016 15:07:32 +0200
Subject: [PATCH] Control expanded state of Navbar.Collapse

Controlling the expanded state allows us to close the collapse after
items are selected (as recommended by react-bootstrap maintainers).

Also makes changes to the nav components to pass through props
(required by react-bootstrap to correctly wire everything together).
---
 js/components/header.js                | 33 +++++++++++++++----
 js/components/header_notifications.js  |  6 ++++
 js/components/nav_routes_links.js      | 44 ++++++++++++++------------
 js/components/nav_routes_links_link.js | 26 +++++++--------
 4 files changed, 69 insertions(+), 40 deletions(-)

diff --git a/js/components/header.js b/js/components/header.js
index 52586901..189b57ea 100644
--- a/js/components/header.js
+++ b/js/components/header.js
@@ -21,6 +21,7 @@ import NavRoutesLinks from './nav_routes_links';
 import { currentUserShape, whitelabelShape } from './prop_types';
 
 import { constructHead } from '../utils/dom';
+import { safeMerge } from '../utils/general';
 import { getLangText } from '../utils/lang';
 
 
@@ -35,7 +36,12 @@ let Header = React.createClass({
     },
 
     getInitialState() {
-        return PieceListStore.getState();
+        return safeMerge(
+            PieceListStore.getState(),
+            {
+                expandedCollapse: false
+            }
+        );
     },
 
     componentDidMount() {
@@ -52,7 +58,17 @@ let Header = React.createClass({
         this.setState(state);
     },
 
-    getLogo() {
+    collapseNav() {
+        if (this.state.expandedCollapse) {
+            this.onToggleCollapse(false);
+        }
+    },
+
+    onToggleCollapse(expandedCollapse) {
+        this.setState({ expandedCollapse });
+    },
+
+    renderLogo() {
         const { whitelabel } = this.props;
 
         if (whitelabel.head) {
@@ -74,7 +90,7 @@ let Header = React.createClass({
         }
     },
 
-    getPoweredBy() {
+    renderPoweredBy() {
         return (
             <a className="pull-left ascribe-powered-by" href="https://www.ascribe.io/" target="_blank">
                 <span>{getLangText('powered by')} </span>
@@ -129,6 +145,7 @@ let Header = React.createClass({
                     navbar
                     pullRight
                     hasPieces={!!unfilteredPieceListCount}
+                    onSelect={this.collapseNav}
                     routes={routes}
                     userAcl={currentUser.acl} />
             );
@@ -154,20 +171,22 @@ let Header = React.createClass({
                 <Navbar
                     ref="navbar"
                     fixedTop
-                    className="hidden-print">
+                    className="hidden-print"
+                    expanded={this.state.expandedCollapse}
+                    onToggle={this.onToggleCollapse}>
                     <Navbar.Header>
                         <Navbar.Brand>
-                            {this.getLogo()}
+                            {this.renderLogo()}
                         </Navbar.Brand>
                         <AclProxy
                             aclName="acl_view_powered_by"
                             aclObject={whitelabel}>
-                            {this.getPoweredBy()}
+                            {this.renderPoweredBy()}
                         </AclProxy>
                         <Navbar.Toggle />
                     </Navbar.Header>
                     <Navbar.Collapse>
-                        <Nav navbar pullRight>
+                        <Nav navbar pullRight onSelect={this.collapseNav}>
                             <HeaderNotificationDebug show={false} />
                             <HeaderNotifications />
                             {account}
diff --git a/js/components/header_notifications.js b/js/components/header_notifications.js
index 7d7c61b2..a93ddcab 100644
--- a/js/components/header_notifications.js
+++ b/js/components/header_notifications.js
@@ -12,6 +12,7 @@ import NotificationStore from '../stores/notification_store';
 import withContext from './context/with_context';
 import { currentUserShape } from './prop_types';
 
+import { omitFromObject } from '../utils/general';
 import { getLangText } from '../utils/lang';
 
 
@@ -98,6 +99,8 @@ const HeaderNotifications = React.createClass({
         // Injected through HOCs
         currentUser: currentUserShape.isRequired,
         isLoggedIn: bool.isRequired
+
+        // All other props are passed down to the backing NavDropdown
     },
 
     getInitialState() {
@@ -133,6 +136,8 @@ const HeaderNotifications = React.createClass({
 
     render() {
         const { editionListNotifications, pieceListNotifications } = this.state;
+        const dropdownProps = omitFromObject(this.props, ['currentUser'], ['isLoggedIn']);
+
         if (pieceListNotifications.length || editionListNotifications.length) {
             let numNotifications = 0;
 
@@ -145,6 +150,7 @@ const HeaderNotifications = React.createClass({
 
             return (
                 <NavDropdown
+                    {...dropdownProps}
                     ref="dropdownButton"
                     className="notification-menu"
                     id="header-notification-dropdown"
diff --git a/js/components/nav_routes_links.js b/js/components/nav_routes_links.js
index 6d86af6c..be9a8323 100644
--- a/js/components/nav_routes_links.js
+++ b/js/components/nav_routes_links.js
@@ -1,5 +1,3 @@
-'use strict';
-
 import React from 'react';
 
 import Nav from 'react-bootstrap/lib/Nav';
@@ -12,26 +10,30 @@ import { sanitizeList } from '../utils/general';
 
 const DISABLE_ENUM = ['hasPieces', 'noPieces'];
 
-let NavRoutesLinks = React.createClass({
+const NavRoutesLinks = React.createClass({
     propTypes: {
         hasPieces: React.PropTypes.bool,
         routes: React.PropTypes.arrayOf(React.PropTypes.object),
         userAcl: React.PropTypes.object
+
+        // All other props are passed to the backing Nav
     },
 
     isRouteDisabled(disableOn) {
         const { hasPieces } = this.props;
 
-        if (disableOn) {
-            if (!DISABLE_ENUM.includes(disableOn)) {
-                throw new Error(`"disableOn" must be one of: [${DISABLE_ENUM.join(', ')}] got "${disableOn}" instead`);
-            }
+        if (disableOn && !DISABLE_ENUM.includes(disableOn)) {
+            throw new Error(
+                `"disableOn" must be one of: [${DISABLE_ENUM.join(', ')}] got "${disableOn}" instead`
+            );
+        }
 
-            if (disableOn === 'hasPieces') {
-                return hasPieces;
-            } else if (disableOn === 'noPieces') {
-                return !hasPieces;
-            }
+        if (disableOn === 'hasPieces') {
+            return hasPieces;
+        } else if (disableOn === 'noPieces') {
+            return !hasPieces;
+        } else {
+            return false;
         }
     },
 
@@ -48,11 +50,11 @@ let NavRoutesLinks = React.createClass({
      */
     extractLinksFromRoutes(node, userAcl, i) {
         if (!node) {
-            return;
+            return null;
         }
 
         const links = node.childRoutes.map((child, j) => {
-            const { aclName, disableOn, headerTitle, path, childRoutes } = child;
+            const { aclName, disableOn, headerTitle, path } = child;
 
             // We validate if the user has set the title correctly,
             // otherwise we're not going to render his route
@@ -88,15 +90,12 @@ let NavRoutesLinks = React.createClass({
                     );
                 } else {
                     return (
-                        <NavRoutesLinksLink
-                            key={j}
-                            {...navLinkProps} />
+                        <NavRoutesLinksLink key={j} {...navLinkProps} />
                     );
                 }
             } else {
                 return null;
             }
-
         });
 
         // remove all nulls from the list of generated links
@@ -104,10 +103,15 @@ let NavRoutesLinks = React.createClass({
     },
 
     render() {
-        const {routes, userAcl} = this.props;
+        const {
+            routes,
+            userAcl,
+            hasPieces: ignoredHasPieces,
+            ...props
+        } = this.props;
 
         return (
-            <Nav {...this.props}>
+            <Nav {...props}>
                 {this.extractLinksFromRoutes(routes[0], userAcl, 0)}
             </Nav>
         );
diff --git a/js/components/nav_routes_links_link.js b/js/components/nav_routes_links_link.js
index 94fd91a2..1560ec69 100644
--- a/js/components/nav_routes_links_link.js
+++ b/js/components/nav_routes_links_link.js
@@ -1,46 +1,45 @@
-'use strict';
-
 import React from 'react';
 
-import DropdownButton from 'react-bootstrap/lib/DropdownButton';
 import MenuItem from 'react-bootstrap/lib/MenuItem';
 import NavItem from 'react-bootstrap/lib/NavItem';
+import NavDropdown from 'react-bootstrap/lib/NavDropdown';
 
 import LinkContainer from 'react-router-bootstrap/lib/LinkContainer';
 
 
-let NavRoutesLinksLink = React.createClass({
+const NavRoutesLinksLink = React.createClass({
     propTypes: {
-        children: React.PropTypes.oneOfType([
-            React.PropTypes.arrayOf(React.PropTypes.element),
-            React.PropTypes.element
-        ]),
-        disabled: React.PropTypes.bool,
+        children: React.PropTypes.node,
         depth: React.PropTypes.number,
+        disabled: React.PropTypes.bool,
         headerTitle: React.PropTypes.string,
         routePath: React.PropTypes.string
+
+        // All other props are passed through to the backing NavItem, or NavDropdown
     },
 
     render() {
-        const { children, headerTitle, depth, disabled, routePath } = this.props;
+        const { children, headerTitle, depth, disabled, routePath, ...props } = this.props;
 
         // if the route has children, we're returning a DropdownButton that will get filled
         // with MenuItems
         if (children) {
             return (
-                <DropdownButton
+                <NavDropdown
+                    {...props}
                     disabled={disabled}
                     id={`nav-route-${headerTitle.toLowerCase()}-dropdown`}
                     title={headerTitle}>
                     {children}
-                </DropdownButton>
+                </NavDropdown>
             );
         } else {
             if (depth === 1) {
                 // if the node's child is actually a node of level one (a child of a node), we're
-                // returning a DropdownButton matching MenuItem
+                // returning a MenuItem for the containing NavDropdown
                 return (
                     <LinkContainer
+                        {...props}
                         disabled={disabled}
                         to={routePath}>
                         <MenuItem>{headerTitle}</MenuItem>
@@ -49,6 +48,7 @@ let NavRoutesLinksLink = React.createClass({
             } else if (depth === 0) {
                 return (
                     <LinkContainer
+                        {...props}
                         disabled={disabled}
                         to={routePath}>
                         <NavItem>{headerTitle}</NavItem>