From 274d7f72f7ea8b0a46c8b7a2ff8da8c7f749aa43 Mon Sep 17 00:00:00 2001
From: boaz0 <ripcurld.github@gmail.com>
Date: Mon, 28 Jan 2019 18:35:02 +0200
Subject: [PATCH] fix(Page): improve a11y in sidebar and header (#1145)

Signed-off-by: Boaz Shuster <boaz.shuster.github@gmail.com>
---
 .../src/components/Page/PageHeader.d.ts          |  1 +
 .../react-core/src/components/Page/PageHeader.js | 14 +++++++++++---
 .../src/components/Page/PageSidebar.js           |  5 +++--
 .../Page/__snapshots__/Page.test.js.snap         | 16 +++++++++++++---
 .../Page/__snapshots__/PageHeader.test.js.snap   |  2 ++
 .../src/components/Page/examples/VerticalPage.js |  1 +
 .../PageLayout/examples/PageLayoutDefaultNav.js  |  1 +
 .../examples/PageLayoutExpandableNav.js          |  1 +
 .../PageLayout/examples/PageLayoutGroupsNav.js   |  1 +
 .../PageLayout/examples/PageLayoutSimpleNav.js   |  1 +
 10 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/packages/patternfly-4/react-core/src/components/Page/PageHeader.d.ts b/packages/patternfly-4/react-core/src/components/Page/PageHeader.d.ts
index a4e609d210b..fbd5da942d0 100644
--- a/packages/patternfly-4/react-core/src/components/Page/PageHeader.d.ts
+++ b/packages/patternfly-4/react-core/src/components/Page/PageHeader.d.ts
@@ -8,6 +8,7 @@ export interface PageHeaderProps extends HTMLProps<HTMLDivElement> {
   avatar?: ReactNode;
   topNav?: ReactNode;
   showNavToggle?: boolean;
+  isNavOpen?: boolean;
   onNavToggle?: Function;
 }
 
diff --git a/packages/patternfly-4/react-core/src/components/Page/PageHeader.js b/packages/patternfly-4/react-core/src/components/Page/PageHeader.js
index f33d286bdea..6d452f49656 100644
--- a/packages/patternfly-4/react-core/src/components/Page/PageHeader.js
+++ b/packages/patternfly-4/react-core/src/components/Page/PageHeader.js
@@ -20,8 +20,12 @@ const propTypes = {
   topNav: PropTypes.node,
   /** True to show the nav toggle button (toggles side nav) */
   showNavToggle: PropTypes.bool,
+  /** True if the side nav is shown  */
+  isNavOpen: PropTypes.bool,
   /** Callback function to handle the side nav toggle button */
   onNavToggle: PropTypes.func,
+  /** Callback function to handle the side nav toggle button */
+  'aria-label': PropTypes.string,
   /** Additional props are spread to the container <header> */
   '': PropTypes.any
 };
@@ -34,10 +38,12 @@ const defaultProps = {
   avatar: null,
   topNav: null,
   showNavToggle: false,
-  onNavToggle: () => undefined
+  isNavOpen: true,
+  onNavToggle: () => undefined,
+  'aria-label': "Toggle global navigation",
 };
 
-const PageHeader = ({ className, logo, logoProps, toolbar, avatar, topNav, showNavToggle, onNavToggle, ...props }) => (
+const PageHeader = ({ className, logo, logoProps, toolbar, avatar, topNav, isNavOpen, showNavToggle, onNavToggle, 'aria-label': ariaLabel, ...props }) => (
   <header role="banner" className={css(styles.pageHeader, className)} {...props}>
     <div className={css(styles.pageHeaderBrand)}>
       {showNavToggle && (
@@ -45,7 +51,9 @@ const PageHeader = ({ className, logo, logoProps, toolbar, avatar, topNav, showN
           <Button
             id="nav-toggle"
             onClick={onNavToggle}
-            aria-label="Toggle primary navigation"
+            aria-label={ariaLabel}
+            aria-controls="page-sidebar"
+            aria-expanded={isNavOpen ? "true" : "false"}
             variant={ButtonVariant.plain}
           >
             <BarsIcon />
diff --git a/packages/patternfly-4/react-core/src/components/Page/PageSidebar.js b/packages/patternfly-4/react-core/src/components/Page/PageSidebar.js
index 7c08694883f..92094fa3908 100644
--- a/packages/patternfly-4/react-core/src/components/Page/PageSidebar.js
+++ b/packages/patternfly-4/react-core/src/components/Page/PageSidebar.js
@@ -21,7 +21,8 @@ const defaultProps = {
 };
 
 const PageSidebar = ({ className, nav, isNavOpen, ...props }) => (
-  <aside
+  <div
+    id="page-sidebar"
     className={css(
       styles.pageSidebar,
       isNavOpen && styles.modifiers.expanded,
@@ -31,7 +32,7 @@ const PageSidebar = ({ className, nav, isNavOpen, ...props }) => (
     {...props}
   >
     {nav}
-  </aside>
+  </div>
 );
 
 PageSidebar.propTypes = propTypes;
diff --git a/packages/patternfly-4/react-core/src/components/Page/__snapshots__/Page.test.js.snap b/packages/patternfly-4/react-core/src/components/Page/__snapshots__/Page.test.js.snap
index 36a8bccb902..239688af8a5 100644
--- a/packages/patternfly-4/react-core/src/components/Page/__snapshots__/Page.test.js.snap
+++ b/packages/patternfly-4/react-core/src/components/Page/__snapshots__/Page.test.js.snap
@@ -77,8 +77,10 @@ exports[`Check page horizontal layout example against snapshot 1`] = `
   className="my-page-class"
   header={
     <PageHeader
+      aria-label="Toggle global navigation"
       avatar=" | Avatar"
       className=""
+      isNavOpen={true}
       logo="Logo"
       logoProps={null}
       nav="Navigation"
@@ -105,8 +107,10 @@ exports[`Check page horizontal layout example against snapshot 1`] = `
     layout="horizontal"
   >
     <PageHeader
+      aria-label="Toggle global navigation"
       avatar=" | Avatar"
       className=""
+      isNavOpen={true}
       logo="Logo"
       logoProps={null}
       nav="Navigation"
@@ -142,8 +146,9 @@ exports[`Check page horizontal layout example against snapshot 1`] = `
       isNavOpen={true}
       nav={null}
     >
-      <aside
+      <div
         className="pf-c-page__sidebar pf-m-expanded"
+        id="page-sidebar"
       />
     </PageSidebar>
     <main
@@ -272,8 +277,10 @@ exports[`Check page vertical layout example against snapshot 1`] = `
   className="my-page-class"
   header={
     <PageHeader
+      aria-label="Toggle global navigation"
       avatar=" | Avatar"
       className=""
+      isNavOpen={true}
       logo="Logo"
       logoProps={null}
       onNavToggle={[Function]}
@@ -297,8 +304,10 @@ exports[`Check page vertical layout example against snapshot 1`] = `
     id="PageId"
   >
     <PageHeader
+      aria-label="Toggle global navigation"
       avatar=" | Avatar"
       className=""
+      isNavOpen={true}
       logo="Logo"
       logoProps={null}
       onNavToggle={[Function]}
@@ -332,11 +341,12 @@ exports[`Check page vertical layout example against snapshot 1`] = `
       isNavOpen={true}
       nav="Navigation"
     >
-      <aside
+      <div
         className="pf-c-page__sidebar pf-m-expanded"
+        id="page-sidebar"
       >
         Navigation
-      </aside>
+      </div>
     </PageSidebar>
     <main
       className="pf-c-page__main"
diff --git a/packages/patternfly-4/react-core/src/components/Page/__snapshots__/PageHeader.test.js.snap b/packages/patternfly-4/react-core/src/components/Page/__snapshots__/PageHeader.test.js.snap
index 36fcf71b26c..a771695d76a 100644
--- a/packages/patternfly-4/react-core/src/components/Page/__snapshots__/PageHeader.test.js.snap
+++ b/packages/patternfly-4/react-core/src/components/Page/__snapshots__/PageHeader.test.js.snap
@@ -25,8 +25,10 @@ exports[`Check page vertical layout example against snapshot 1`] = `
 }
 
 <PageHeader
+  aria-label="Toggle global navigation"
   avatar=" | Avatar"
   className=""
+  isNavOpen={true}
   logo="Logo"
   logoProps={null}
   onNavToggle={[Function]}
diff --git a/packages/patternfly-4/react-core/src/components/Page/examples/VerticalPage.js b/packages/patternfly-4/react-core/src/components/Page/examples/VerticalPage.js
index 82fec5bd913..c2c1a530e7a 100644
--- a/packages/patternfly-4/react-core/src/components/Page/examples/VerticalPage.js
+++ b/packages/patternfly-4/react-core/src/components/Page/examples/VerticalPage.js
@@ -30,6 +30,7 @@ class VerticalPage extends React.Component {
         toolbar="Toolbar"
         avatar=" | Avatar"
         showNavToggle
+        isNavOpen={isNavOpen}
         onNavToggle={this.onNavToggle}
       />
     );
diff --git a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutDefaultNav.js b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutDefaultNav.js
index e1e5d8d1dea..4a37572d72b 100644
--- a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutDefaultNav.js
+++ b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutDefaultNav.js
@@ -184,6 +184,7 @@ class PageLayoutDefaultNav extends React.Component {
         avatar={<Avatar src={avatarImg} alt="Avatar image" />}
         showNavToggle
         onNavToggle={this.onNavToggle}
+        isNavOpen={isNavOpen}
       />
     );
     const Sidebar = <PageSidebar nav={PageNav} isNavOpen={isNavOpen} />;
diff --git a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutExpandableNav.js b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutExpandableNav.js
index 5421503de43..f2680e9fd4c 100644
--- a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutExpandableNav.js
+++ b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutExpandableNav.js
@@ -208,6 +208,7 @@ class PageLayoutExpandableNav extends React.Component {
         avatar={<Avatar src={avatarImg} alt="Avatar image" />}
         showNavToggle
         onNavToggle={this.onNavToggle}
+        isNavOpen={isNavOpen}
       />
     );
     const Sidebar = <PageSidebar nav={PageNav} isNavOpen={isNavOpen} />;
diff --git a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutGroupsNav.js b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutGroupsNav.js
index aec0a4e136b..e657dbb7b67 100644
--- a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutGroupsNav.js
+++ b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutGroupsNav.js
@@ -194,6 +194,7 @@ class PageLayoutGroupsNav extends React.Component {
         avatar={<Avatar src={avatarImg} alt="Avatar image" />}
         showNavToggle
         onNavToggle={this.onNavToggle}
+        isNavOpen={isNavOpen}
       />
     );
     const Sidebar = <PageSidebar nav={PageNav} isNavOpen={isNavOpen} />;
diff --git a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutSimpleNav.js b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutSimpleNav.js
index 0ee0664c3b6..0f8843ed9ed 100644
--- a/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutSimpleNav.js
+++ b/packages/patternfly-4/react-core/src/demos/PageLayout/examples/PageLayoutSimpleNav.js
@@ -185,6 +185,7 @@ class PageLayoutSimpleNav extends React.Component {
         avatar={<Avatar src={avatarImg} alt="Avatar image" />}
         showNavToggle
         onNavToggle={this.onNavToggle}
+        isNavOpen={isNavOpen}
       />
     );
     const Sidebar = <PageSidebar nav={PageNav} isNavOpen={isNavOpen} />;