From f92a08643f268e078b51b032e51ff44cb3aa66a2 Mon Sep 17 00:00:00 2001
From: Ian Bolton <ibolton@redhat.com>
Date: Fri, 1 Mar 2019 14:57:55 -0500
Subject: [PATCH] fix(nav): Fix bug where previous was expanding on select

---
 .../react-core/src/components/Nav/Nav.js      |  4 ++--
 .../src/components/Nav/NavExpandable.d.ts     |  1 +
 .../src/components/Nav/NavExpandable.js       | 24 ++++++++++++-------
 .../src/components/Nav/NavToggle.js           | 19 ++++++++-------
 .../Nav/__snapshots__/Nav.test.js.snap        | 20 ++++++++++------
 .../Nav/examples/NavExpandableList.js         |  2 +-
 .../Nav/examples/NavExpandableTitlesList.js   | 10 ++++++--
 7 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/packages/patternfly-4/react-core/src/components/Nav/Nav.js b/packages/patternfly-4/react-core/src/components/Nav/Nav.js
index 4519cae8bcb..dddf78ba612 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/Nav.js
+++ b/packages/patternfly-4/react-core/src/components/Nav/Nav.js
@@ -42,11 +42,11 @@ class Nav extends React.Component {
   }
 
   // Callback from NavExpandable
-  onToggle(event, groupId, isExpanded) {
+  onToggle(event, groupId, toggleValue) {
     this.props.onToggle({
       event,
       groupId,
-      isExpanded
+      toggleValue
     });
   }
 
diff --git a/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.d.ts b/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.d.ts
index fe2d0cd0970..c521dee2927 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.d.ts
+++ b/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.d.ts
@@ -3,6 +3,7 @@ import { FunctionComponent, HTMLProps, ReactNode } from 'react';
 export interface NavExpandableProps extends HTMLProps<HTMLDivElement> {
   title: string;
   srText?: string;
+  defaultExpanded?: boolean;
   isExpanded?: boolean;
   children?: ReactNode;
   className?: string;
diff --git a/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.js b/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.js
index 7c0aa872132..b37b4da9f39 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.js
+++ b/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.js
@@ -14,6 +14,8 @@ const propTypes = {
   /** If defined, screen readers will read this text instead of the list title */
   srText: PropTypes.string,
   /** If true will default the list to be expanded */
+  defaultExpanded: PropTypes.bool,
+  /** Boolean to programatically expand or collapse section */
   isExpanded: PropTypes.bool,
   /** Anything that can be rendered inside of the expandable list */
   children: PropTypes.node,
@@ -31,7 +33,8 @@ const propTypes = {
 
 const defaultProps = {
   srText: '',
-  isExpanded: false,
+  defaultExpanded: false,
+  isExpanded: null,
   children: null,
   className: '',
   groupId: null,
@@ -41,13 +44,13 @@ const defaultProps = {
 
 class NavExpandable extends React.Component {
   id = this.props.id || getUniqueId();
-
   render() {
     const {
       id,
       title,
       srText,
-      isExpanded: defaultExpanded,
+      isExpanded,
+      defaultExpanded,
       children,
       className,
       groupId,
@@ -58,12 +61,17 @@ class NavExpandable extends React.Component {
     return (
       <NavContext.Consumer>
         {context => (
-          <NavToggle defaultValue={defaultExpanded} groupId={groupId} onToggle={context.onToggle}>
-            {({ value: isExpanded, toggle }) => (
+          <NavToggle
+            defaultValue={defaultExpanded}
+            isExpanded={isExpanded}
+            groupId={groupId}
+            onToggle={context.onToggle}
+          >
+            {({ toggleValue, toggle }) => (
               <li
                 className={css(
                   styles.navItem,
-                  isExpanded && styles.modifiers.expanded,
+                  toggleValue && styles.modifiers.expanded,
                   isActive && styles.modifiers.current,
                   className
                 )}
@@ -77,14 +85,14 @@ class NavExpandable extends React.Component {
                   href="#"
                   onClick={e => e.preventDefault()}
                   onMouseDown={e => e.preventDefault()}
-                  aria-expanded={isExpanded}
+                  aria-expanded={toggleValue}
                 >
                   {title}
                   <span className={css(styles.navToggle)}>
                     <AngleRightIcon aria-hidden="true" />
                   </span>
                 </a>
-                <section className={css(styles.navSubnav)} aria-labelledby={this.id} hidden={isExpanded ? null : true}>
+                <section className={css(styles.navSubnav)} aria-labelledby={this.id} hidden={toggleValue ? null : true}>
                   {srText && (
                     <h2 className={css(a11yStyles.screenReader)} id={this.id}>
                       {srText}
diff --git a/packages/patternfly-4/react-core/src/components/Nav/NavToggle.js b/packages/patternfly-4/react-core/src/components/Nav/NavToggle.js
index 6bdbdabdbd5..7db115ba148 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/NavToggle.js
+++ b/packages/patternfly-4/react-core/src/components/Nav/NavToggle.js
@@ -9,7 +9,7 @@ const propTypes = {
 };
 
 const defaultProps = {
-  defaultValue: true,
+  defaultValue: false,
   groupId: 0,
   onToggle: () => undefined
 };
@@ -18,29 +18,30 @@ class NavToggle extends React.Component {
   static propTypes = propTypes;
   static defaultProps = defaultProps;
   state = {
-    value: this.props.defaultValue
+    toggleValue: this.props.defaultValue
   };
-  componentWillReceiveProps(nextProps) {
-    if (nextProps.defaultValue !== this.state.value) {
-      this.setState({ value: nextProps.defaultValue });
+  componentDidUpdate(prevProps) {
+    if (this.props.isExpanded !== prevProps.isExpanded) {
+      this.setState({ toggleValue: this.props.isExpanded });
     }
   }
+
   handleToggle = e => {
     // Item events can bubble up, ignore those
     if (e.target.getAttribute('data-component') !== 'pf-nav-expandable') {
       return;
     }
-    const { value } = this.state;
+    const { toggleValue } = this.state;
     const { groupId, onToggle } = this.props;
     this.setState({
-      value: !value
+      toggleValue: !toggleValue
     });
-    onToggle(e, groupId, !value);
+    onToggle(e, groupId, !toggleValue);
   };
 
   render() {
     return this.props.children({
-      value: this.state.value,
+      toggleValue: this.state.toggleValue,
       toggle: this.handleToggle
     });
   }
diff --git a/packages/patternfly-4/react-core/src/components/Nav/__snapshots__/Nav.test.js.snap b/packages/patternfly-4/react-core/src/components/Nav/__snapshots__/Nav.test.js.snap
index 8eaa03dbf53..b3367720826 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/__snapshots__/Nav.test.js.snap
+++ b/packages/patternfly-4/react-core/src/components/Nav/__snapshots__/Nav.test.js.snap
@@ -356,7 +356,7 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
   overflow: hidden;
   opacity: 0;
 }
-.pf-c-nav__item.pf-m-expanded.expandable-group {
+.pf-c-nav__item.expandable-group {
   display: block;
 }
 .pf-c-nav__list {
@@ -388,6 +388,7 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
       >
         <NavExpandable
           className="expandable-group"
+          defaultExpanded={false}
           groupId={null}
           id="grp-1"
           isActive={false}
@@ -396,16 +397,17 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
           title="Section 1"
         >
           <NavToggle
-            defaultValue={true}
+            defaultValue={false}
             groupId={null}
+            isExpanded={true}
             onToggle={[Function]}
           >
             <li
-              className="pf-c-nav__item pf-m-expanded expandable-group"
+              className="pf-c-nav__item expandable-group"
               onClick={[Function]}
             >
               <a
-                aria-expanded={true}
+                aria-expanded={false}
                 className="pf-c-nav__link"
                 data-component="pf-nav-expandable"
                 href="#"
@@ -448,7 +450,7 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
               <section
                 aria-labelledby="grp-1"
                 className="pf-c-nav__subnav"
-                hidden={null}
+                hidden={true}
               >
                 <ul
                   className="pf-c-nav__simple-list"
@@ -640,16 +642,18 @@ exports[`Expandable Nav List 1`] = `
       >
         <NavExpandable
           className=""
+          defaultExpanded={false}
           groupId={null}
           id="grp-1"
           isActive={false}
-          isExpanded={false}
+          isExpanded={null}
           srText=""
           title="Section 1"
         >
           <NavToggle
             defaultValue={false}
             groupId={null}
+            isExpanded={null}
             onToggle={[Function]}
           >
             <li
@@ -900,16 +904,18 @@ exports[`Expandable Nav List with aria label 1`] = `
       >
         <NavExpandable
           className=""
+          defaultExpanded={false}
           groupId={null}
           id="grp-1"
           isActive={false}
-          isExpanded={false}
+          isExpanded={null}
           srText="Section 1 - Example sub-navigation"
           title="Section 1"
         >
           <NavToggle
             defaultValue={false}
             groupId={null}
+            isExpanded={null}
             onToggle={[Function]}
           >
             <li
diff --git a/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableList.js b/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableList.js
index 7e7518b70d8..c7100f513cf 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableList.js
+++ b/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableList.js
@@ -29,7 +29,7 @@ class NavExpandableList extends React.Component {
     return (
       <Nav onSelect={this.onSelect} onToggle={this.onToggle}>
         <NavList>
-          <NavExpandable title="Link 1" groupId="grp-1" isActive={activeGroup === 'grp-1'} isExpanded>
+          <NavExpandable title="Link 1" groupId="grp-1" isActive={activeGroup === 'grp-1'} defaultExpanded>
             <NavItem
               preventDefault
               to="#expandable-1"
diff --git a/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableTitlesList.js b/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableTitlesList.js
index 67817d62a0c..6748cc6298d 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableTitlesList.js
+++ b/packages/patternfly-4/react-core/src/components/Nav/examples/NavExpandableTitlesList.js
@@ -17,9 +17,15 @@ class NavExpandableTitlesList extends React.Component {
   render() {
     const { activeGroup, activeItem } = this.state;
     return (
-      <Nav onSelect={this.onSelect} onToggle={this.onToggle}>
+      <Nav onSelect={this.onSelect}>
         <NavList>
-          <NavExpandable title="Link 1" srText="SR Link" groupId="grp-1" isActive={activeGroup === 'grp-1'} isExpanded>
+          <NavExpandable
+            title="Link 1"
+            srText="SR Link"
+            groupId="grp-1"
+            isActive={activeGroup === 'grp-1'}
+            defaultExpanded
+          >
             <NavItem
               preventDefault
               to="#sr-expandable-1"