From 90808de5a104db3813c82b7b481288fe7595e80c Mon Sep 17 00:00:00 2001
From: Ian Bolton <ibolton@redhat.com>
Date: Mon, 4 Mar 2019 15:16:03 -0500
Subject: [PATCH] fix(navExpand): remove defaultExpand in favor of just using
 isExpanded

---
 .../src/components/Nav/NavExpandable.d.ts     |  2 +-
 .../src/components/Nav/NavExpandable.js       | 55 +++++++++++--------
 .../src/components/Nav/NavToggle.js           | 29 ++++------
 .../Nav/__snapshots__/Nav.test.js.snap        | 25 ++++-----
 .../Nav/examples/NavExpandableList.js         |  4 +-
 .../Nav/examples/NavExpandableTitlesList.js   |  8 +--
 6 files changed, 56 insertions(+), 67 deletions(-)

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 c521dee2927..d789c13dab0 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,13 +3,13 @@ import { FunctionComponent, HTMLProps, ReactNode } from 'react';
 export interface NavExpandableProps extends HTMLProps<HTMLDivElement> {
   title: string;
   srText?: string;
-  defaultExpanded?: boolean;
   isExpanded?: boolean;
   children?: ReactNode;
   className?: string;
   groupId?: string | number;
   isActive?: boolean;
   id?: string;
+  onExpand?(e: Event, val: boolean): void;
 }
 
 declare const NavExpandable: FunctionComponent<NavExpandableProps>;
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 b37b4da9f39..b4be7e5a6fa 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.js
+++ b/packages/patternfly-4/react-core/src/components/Nav/NavExpandable.js
@@ -13,8 +13,6 @@ const propTypes = {
   title: PropTypes.string.isRequired,
   /** 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 */
@@ -28,45 +26,54 @@ const propTypes = {
   /** Identifier to use for the section aria label */
   id: PropTypes.string,
   /** Additional props are spread to the container <li> */
-  '': PropTypes.any
+  '': PropTypes.any,
+  /** allow consumer to optionally override this callback and manage expand state externally */
+  onExpand: PropTypes.func
 };
 
 const defaultProps = {
   srText: '',
-  defaultExpanded: false,
-  isExpanded: null,
+  isExpanded: false,
   children: null,
   className: '',
   groupId: null,
   isActive: false,
-  id: ''
+  id: '',
+  onExpand: undefined
 };
 
 class NavExpandable extends React.Component {
   id = this.props.id || getUniqueId();
+  state = {
+    expandedState: false
+  };
+
+  componentDidMount() {
+    this.setState({ expandedState: this.props.isExpanded });
+  }
+
+  componentDidUpdate(prevProps) {
+    if (this.props.isExpanded !== prevProps.isExpanded) {
+      this.setState({ expandedState: this.props.isExpanded });
+    }
+  }
+
+  onExpand = (e, val) => {
+    if (this.props.onExpand) {
+      this.props.onExpand(e, val);
+    } else {
+      this.setState({ expandedState: val });
+    }
+  };
+
   render() {
-    const {
-      id,
-      title,
-      srText,
-      isExpanded,
-      defaultExpanded,
-      children,
-      className,
-      groupId,
-      isActive,
-      ...props
-    } = this.props;
+    const { id, title, srText, isExpanded, children, className, groupId, isActive, ...props } = this.props;
+    const { expandedState } = this.state;
 
     return (
       <NavContext.Consumer>
         {context => (
-          <NavToggle
-            defaultValue={defaultExpanded}
-            isExpanded={isExpanded}
-            groupId={groupId}
-            onToggle={context.onToggle}
-          >
+          <NavToggle groupId={groupId} onToggle={context.onToggle} onExpand={this.onExpand} isExpanded={expandedState}>
             {({ toggleValue, toggle }) => (
               <li
                 className={css(
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 7db115ba148..01abb1c04d9 100644
--- a/packages/patternfly-4/react-core/src/components/Nav/NavToggle.js
+++ b/packages/patternfly-4/react-core/src/components/Nav/NavToggle.js
@@ -3,45 +3,36 @@ import PropTypes from 'prop-types';
 
 const propTypes = {
   children: PropTypes.func.isRequired,
-  defaultValue: PropTypes.bool,
+  isExpanded: PropTypes.bool,
   groupId: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
-  onToggle: PropTypes.func
+  onToggle: PropTypes.func,
+  onExpand: PropTypes.func
 };
 
 const defaultProps = {
-  defaultValue: false,
+  isExpanded: false,
   groupId: 0,
-  onToggle: () => undefined
+  onToggle: () => undefined,
+  onExpand: () => undefined
 };
 
 class NavToggle extends React.Component {
   static propTypes = propTypes;
   static defaultProps = defaultProps;
-  state = {
-    toggleValue: this.props.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 { toggleValue } = this.state;
-    const { groupId, onToggle } = this.props;
-    this.setState({
-      toggleValue: !toggleValue
-    });
-    onToggle(e, groupId, !toggleValue);
+    const { groupId, onToggle, onExpand, isExpanded } = this.props;
+    onToggle(e, groupId, !isExpanded);
+    onExpand(e, !isExpanded);
   };
 
   render() {
     return this.props.children({
-      toggleValue: this.state.toggleValue,
+      toggleValue: this.props.isExpanded,
       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 b3367720826..a836c14ff20 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.expandable-group {
+.pf-c-nav__item.pf-m-expanded.expandable-group {
   display: block;
 }
 .pf-c-nav__list {
@@ -388,7 +388,6 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
       >
         <NavExpandable
           className="expandable-group"
-          defaultExpanded={false}
           groupId={null}
           id="grp-1"
           isActive={false}
@@ -397,17 +396,17 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
           title="Section 1"
         >
           <NavToggle
-            defaultValue={false}
             groupId={null}
             isExpanded={true}
+            onExpand={[Function]}
             onToggle={[Function]}
           >
             <li
-              className="pf-c-nav__item expandable-group"
+              className="pf-c-nav__item pf-m-expanded expandable-group"
               onClick={[Function]}
             >
               <a
-                aria-expanded={false}
+                aria-expanded={true}
                 className="pf-c-nav__link"
                 data-component="pf-nav-expandable"
                 href="#"
@@ -450,7 +449,7 @@ exports[`Expandable Nav List - Trigger toggle 1`] = `
               <section
                 aria-labelledby="grp-1"
                 className="pf-c-nav__subnav"
-                hidden={true}
+                hidden={null}
               >
                 <ul
                   className="pf-c-nav__simple-list"
@@ -642,18 +641,17 @@ exports[`Expandable Nav List 1`] = `
       >
         <NavExpandable
           className=""
-          defaultExpanded={false}
           groupId={null}
           id="grp-1"
           isActive={false}
-          isExpanded={null}
+          isExpanded={false}
           srText=""
           title="Section 1"
         >
           <NavToggle
-            defaultValue={false}
             groupId={null}
-            isExpanded={null}
+            isExpanded={false}
+            onExpand={[Function]}
             onToggle={[Function]}
           >
             <li
@@ -904,18 +902,17 @@ exports[`Expandable Nav List with aria label 1`] = `
       >
         <NavExpandable
           className=""
-          defaultExpanded={false}
           groupId={null}
           id="grp-1"
           isActive={false}
-          isExpanded={null}
+          isExpanded={false}
           srText="Section 1 - Example sub-navigation"
           title="Section 1"
         >
           <NavToggle
-            defaultValue={false}
             groupId={null}
-            isExpanded={null}
+            isExpanded={false}
+            onExpand={[Function]}
             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 c7100f513cf..aba4bd8cefe 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'} defaultExpanded>
+          <NavExpandable title="Link 1" groupId="grp-1" isActive={activeGroup === 'grp-1'} isExpanded>
             <NavItem
               preventDefault
               to="#expandable-1"
@@ -52,7 +52,7 @@ class NavExpandableList extends React.Component {
               Subnav Link 3
             </NavItem>
           </NavExpandable>
-          <NavExpandable title="Link 2" groupId="grp-2" isActive={activeGroup === 'grp-2'}>
+          <NavExpandable title="Link 2" groupId="grp-2" isActive={activeGroup === 'grp-2'} isExpanded>
             <NavItem
               preventDefault
               onClick={this.handleItemOnclick}
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 6748cc6298d..35c72ef0aaf 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
@@ -19,13 +19,7 @@ class NavExpandableTitlesList extends React.Component {
     return (
       <Nav onSelect={this.onSelect}>
         <NavList>
-          <NavExpandable
-            title="Link 1"
-            srText="SR Link"
-            groupId="grp-1"
-            isActive={activeGroup === 'grp-1'}
-            defaultExpanded
-          >
+          <NavExpandable title="Link 1" srText="SR Link" groupId="grp-1" isActive={activeGroup === 'grp-1'} isExpanded>
             <NavItem
               preventDefault
               to="#sr-expandable-1"