Move focus to the pages explorer menu when open (#5336, #5394)

* Ensure that when you open the explorer the focus moves to the first link of the menu, and add a label for the explorer navigation
* Add dialog role to pages explorer popup, and ensure that there is an option to close the dialog window when tabbing by making the close button visually hidden rather than display none

CHANGELOG.txt

@@ -56,6 +56,7 @@ Changelog
  * Fix: Remove tab order customisations in CMS admin (Jordan Bauer)
  * Fix: Add labels to permission checkboxes for screen reader users (Helen Chapman, Katie Locke)
  * Fix: Page.copy() no longer copies child objects when the accesssor name is included in `exclude_fields_in_copy` (Karl Hobley)
+ * Fix: Move focus to the pages explorer menu when open (Helen Chapman)
 
 
 2.5.1 (07.05.2019)

client/src/components/Button/Button.js

@@ -38,6 +38,7 @@ const Button = ({
   target,
   preventDefault,
   onClick,
+  dialogTrigger,
 }) => {
   const hasText = children !== null;
   const iconName = isLoading ? 'spinner' : icon;
@@ -54,6 +55,7 @@ const Button = ({
       rel={target === '_blank' ? 'noopener noreferrer' : null}
       href={href}
       target={target}
+      aria-haspopup={dialogTrigger ? 'dialog' : null}
     >
       {hasText ? children : accessibleElt}
     </a>
@@ -73,6 +75,7 @@ Button.propTypes = {
   onClick: PropTypes.func,
   isLoading: PropTypes.bool,
   preventDefault: PropTypes.bool,
+  dialogTrigger: PropTypes.bool,
 };
 
 Button.defaultProps = {
@@ -85,6 +88,7 @@ Button.defaultProps = {
   onClick: null,
   isLoading: false,
   preventDefault: true,
+  dialogTrigger: false,
 };
 
 export default Button;

client/src/components/Button/Button.test.js

@@ -20,6 +20,10 @@ describe('Button', () => {
     expect(shallow(<Button accessibleLabel="I am here in the shadows" />)).toMatchSnapshot();
   });
 
+  it('#dialogTrigger', () => {
+    expect(shallow(<Button dialogTrigger />)).toMatchSnapshot();
+  });
+
   it('#icon', () => {
     expect(shallow(<Button icon="test-icon" />)).toMatchSnapshot();
   });

client/src/components/Button/__snapshots__/Button.test.js.snap

@@ -2,6 +2,7 @@
 
 exports[`Button #accessibleLabel 1`] = `
 <a
+  aria-haspopup={null}
   className=" "
   href="#"
   onClick={[Function]}
@@ -18,6 +19,7 @@ exports[`Button #accessibleLabel 1`] = `
 
 exports[`Button #children 1`] = `
 <a
+  aria-haspopup={null}
   className=" "
   href="#"
   onClick={[Function]}
@@ -28,8 +30,20 @@ exports[`Button #children 1`] = `
 </a>
 `;
 
+exports[`Button #dialogTrigger 1`] = `
+<a
+  aria-haspopup="dialog"
+  className=" "
+  href="#"
+  onClick={[Function]}
+  rel={null}
+  target={null}
+/>
+`;
+
 exports[`Button #icon 1`] = `
 <a
+  aria-haspopup={null}
   className=" icon icon-test-icon"
   href="#"
   onClick={[Function]}
@@ -40,6 +54,7 @@ exports[`Button #icon 1`] = `
 
 exports[`Button #icon changes with #isLoading 1`] = `
 <a
+  aria-haspopup={null}
   className=" icon icon-spinner"
   href="#"
   onClick={[Function]}
@@ -50,6 +65,7 @@ exports[`Button #icon changes with #isLoading 1`] = `
 
 exports[`Button #multiple icons 1`] = `
 <a
+  aria-haspopup={null}
   className=" icon icon-test-icon icon-secondary-icon"
   href="#"
   onClick={[Function]}
@@ -60,6 +76,7 @@ exports[`Button #multiple icons 1`] = `
 
 exports[`Button #target 1`] = `
 <a
+  aria-haspopup={null}
   className=" "
   href="#"
   onClick={[Function]}
@@ -70,6 +87,7 @@ exports[`Button #target 1`] = `
 
 exports[`Button basic 1`] = `
 <a
+  aria-haspopup={null}
   className=" "
   href="#"
   onClick={[Function]}

client/src/components/Explorer/ExplorerPanel.js

@@ -137,18 +137,19 @@ class ExplorerPanel extends React.Component {
 
     return (
       <FocusTrap
-        tag="nav"
+        tag="div"
+        role="dialog"
         className="explorer"
         paused={paused || !page || page.isFetching}
         focusTrapOptions={{
-          initialFocus: '.c-explorer__close',
+          initialFocus: '.c-explorer__header',
           onDeactivate: onClose,
         }}
       >
         <Button className="c-explorer__close u-hidden" onClick={onClose}>
           {STRINGS.CLOSE_EXPLORER}
         </Button>
-        <Transition name={transition} className="c-explorer">
+        <Transition name={transition} className="c-explorer" component="nav" label={STRINGS.PAGE_EXPLORER}>
           <div key={path.length} className="c-transition-group">
             <ExplorerHeader
               depth={path.length}

client/src/components/Explorer/ExplorerToggle.js

@@ -13,6 +13,7 @@ const ExplorerToggle = ({ children, onToggle }) => (
   <Button
     className="submenu-trigger"
     icon="folder-open-inverse"
+    dialogTrigger={true}
     onClick={onToggle}
   >
     {children}

client/src/components/Explorer/__snapshots__/ExplorerHeader.test.js.snap

@@ -4,6 +4,7 @@ exports[`ExplorerHeader #depth at root 1`] = `
 <Button
   accessibleLabel={null}
   className="c-explorer__header"
+  dialogTrigger={false}
   href="/admin/pages/"
   icon=""
   isLoading={false}
@@ -30,6 +31,7 @@ exports[`ExplorerHeader #page 1`] = `
 <Button
   accessibleLabel={null}
   className="c-explorer__header"
+  dialogTrigger={false}
   href="/admin/pages/a/"
   icon=""
   isLoading={false}
@@ -56,6 +58,7 @@ exports[`ExplorerHeader basic 1`] = `
 <Button
   accessibleLabel={null}
   className="c-explorer__header"
+  dialogTrigger={false}
   href="/admin/pages/"
   icon=""
   isLoading={false}

client/src/components/Explorer/__snapshots__/ExplorerItem.test.js.snap

@@ -7,6 +7,7 @@ exports[`ExplorerItem children 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__link"
+    dialogTrigger={false}
     href="/admin/pages/5/"
     icon=""
     isLoading={false}
@@ -28,6 +29,7 @@ exports[`ExplorerItem children 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__action c-explorer__item__action--small"
+    dialogTrigger={false}
     href="/admin/pages/5/edit/"
     icon=""
     isLoading={false}
@@ -44,6 +46,7 @@ exports[`ExplorerItem children 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__action"
+    dialogTrigger={false}
     href="/admin/pages/5/"
     icon=""
     isLoading={false}
@@ -67,6 +70,7 @@ exports[`ExplorerItem renders 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__link"
+    dialogTrigger={false}
     href="/admin/pages/5/"
     icon=""
     isLoading={false}
@@ -83,6 +87,7 @@ exports[`ExplorerItem renders 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__action c-explorer__item__action--small"
+    dialogTrigger={false}
     href="/admin/pages/5/edit/"
     icon=""
     isLoading={false}
@@ -106,6 +111,7 @@ exports[`ExplorerItem should show a publication status if not live 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__link"
+    dialogTrigger={false}
     href="/admin/pages/5/"
     icon=""
     isLoading={false}
@@ -140,6 +146,7 @@ exports[`ExplorerItem should show a publication status if not live 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__action c-explorer__item__action--small"
+    dialogTrigger={false}
     href="/admin/pages/5/edit/"
     icon=""
     isLoading={false}
@@ -156,6 +163,7 @@ exports[`ExplorerItem should show a publication status if not live 1`] = `
   <Button
     accessibleLabel={null}
     className="c-explorer__item__action"
+    dialogTrigger={false}
     href="/admin/pages/5/"
     icon=""
     isLoading={false}
@@ -179,6 +187,7 @@ exports[`ExplorerItem should show a publication status with unpublished changes
   <Button
     accessibleLabel={null}
     className="c-explorer__item__link"
+    dialogTrigger={false}
     href="/admin/pages/5/"
     icon=""
     isLoading={false}
@@ -213,6 +222,7 @@ exports[`ExplorerItem should show a publication status with unpublished changes
   <Button
     accessibleLabel={null}
     className="c-explorer__item__action c-explorer__item__action--small"
+    dialogTrigger={false}
     href="/admin/pages/5/edit/"
     icon=""
     isLoading={false}
@@ -229,6 +239,7 @@ exports[`ExplorerItem should show a publication status with unpublished changes
   <Button
     accessibleLabel={null}
     className="c-explorer__item__action"
+    dialogTrigger={false}
     href="/admin/pages/5/"
     icon=""
     isLoading={false}

client/src/components/Explorer/__snapshots__/ExplorerPanel.test.js.snap

@@ -7,16 +7,18 @@ exports[`ExplorerPanel general rendering #isError 1`] = `
   className="explorer"
   focusTrapOptions={
     Object {
-      "initialFocus": ".c-explorer__close",
+      "initialFocus": ".c-explorer__header",
       "onDeactivate": [MockFunction],
     }
   }
   paused={false}
-  tag="nav"
+  role="dialog"
+  tag="div"
 >
   <Button
     accessibleLabel={null}
     className="c-explorer__close u-hidden"
+    dialogTrigger={false}
     href="#"
     icon=""
     isLoading={false}
@@ -28,8 +30,9 @@ exports[`ExplorerPanel general rendering #isError 1`] = `
   </Button>
   <Transition
     className="c-explorer"
-    component="div"
+    component="nav"
     duration={210}
+    label="Page explorer"
     name="push"
   >
     <div
@@ -83,16 +86,18 @@ exports[`ExplorerPanel general rendering #isFetching 1`] = `
   className="explorer"
   focusTrapOptions={
     Object {
-      "initialFocus": ".c-explorer__close",
+      "initialFocus": ".c-explorer__header",
       "onDeactivate": [MockFunction],
     }
   }
   paused={true}
-  tag="nav"
+  role="dialog"
+  tag="div"
 >
   <Button
     accessibleLabel={null}
     className="c-explorer__close u-hidden"
+    dialogTrigger={false}
     href="#"
     icon=""
     isLoading={false}
@@ -104,8 +109,9 @@ exports[`ExplorerPanel general rendering #isFetching 1`] = `
   </Button>
   <Transition
     className="c-explorer"
-    component="div"
+    component="nav"
     duration={210}
+    label="Page explorer"
     name="push"
   >
     <div
@@ -149,16 +155,18 @@ exports[`ExplorerPanel general rendering #items 1`] = `
   className="explorer"
   focusTrapOptions={
     Object {
-      "initialFocus": ".c-explorer__close",
+      "initialFocus": ".c-explorer__header",
       "onDeactivate": [MockFunction],
     }
   }
   paused={false}
-  tag="nav"
+  role="dialog"
+  tag="div"
 >
   <Button
     accessibleLabel={null}
     className="c-explorer__close u-hidden"
+    dialogTrigger={false}
     href="#"
     icon=""
     isLoading={false}
@@ -170,8 +178,9 @@ exports[`ExplorerPanel general rendering #items 1`] = `
   </Button>
   <Transition
     className="c-explorer"
-    component="div"
+    component="nav"
     duration={210}
+    label="Page explorer"
     name="push"
   >
     <div
@@ -240,16 +249,18 @@ exports[`ExplorerPanel general rendering no children 1`] = `
   className="explorer"
   focusTrapOptions={
     Object {
-      "initialFocus": ".c-explorer__close",
+      "initialFocus": ".c-explorer__header",
       "onDeactivate": [MockFunction],
     }
   }
   paused={false}
-  tag="nav"
+  role="dialog"
+  tag="div"
 >
   <Button
     accessibleLabel={null}
     className="c-explorer__close u-hidden"
+    dialogTrigger={false}
     href="#"
     icon=""
     isLoading={false}
@@ -261,8 +272,9 @@ exports[`ExplorerPanel general rendering no children 1`] = `
   </Button>
   <Transition
     className="c-explorer"
-    component="div"
+    component="nav"
     duration={210}
+    label="Page explorer"
     name="push"
   >
     <div
@@ -300,16 +312,18 @@ exports[`ExplorerPanel general rendering renders 1`] = `
   className="explorer"
   focusTrapOptions={
     Object {
-      "initialFocus": ".c-explorer__close",
+      "initialFocus": ".c-explorer__header",
       "onDeactivate": [MockFunction],
     }
   }
   paused={false}
-  tag="nav"
+  role="dialog"
+  tag="div"
 >
   <Button
     accessibleLabel={null}
     className="c-explorer__close u-hidden"
+    dialogTrigger={false}
     href="#"
     icon=""
     isLoading={false}
@@ -321,8 +335,9 @@ exports[`ExplorerPanel general rendering renders 1`] = `
   </Button>
   <Transition
     className="c-explorer"
-    component="div"
+    component="nav"
     duration={210}
+    label="Page explorer"
     name="push"
   >
     <div

client/src/components/Explorer/__snapshots__/ExplorerToggle.test.js.snap

@@ -4,6 +4,7 @@ exports[`ExplorerToggle basic 1`] = `
 <Button
   accessibleLabel={null}
   className="submenu-trigger"
+  dialogTrigger={true}
   href="#"
   icon="folder-open-inverse"
   isLoading={false}

client/src/components/Transition/Transition.js

@@ -18,6 +18,7 @@ const Transition = ({
   className,
   duration,
   children,
+  label,
 }) => (
   <CSSTransitionGroup
     component={component}
@@ -25,6 +26,7 @@ const Transition = ({
     transitionLeaveTimeout={duration}
     transitionName={`c-transition-${name}`}
     className={className}
+    aria-label={label}
   >
     {children}
   </CSSTransitionGroup>
@@ -36,6 +38,7 @@ Transition.propTypes = {
   className: PropTypes.string,
   duration: PropTypes.number,
   children: PropTypes.node,
+  label: PropTypes.string,
 };
 
 Transition.defaultProps = {
@@ -43,6 +46,7 @@ Transition.defaultProps = {
   children: null,
   className: null,
   duration: TRANSITION_DURATION,
+  label: null,
 };
 
 export default Transition;

client/src/components/Transition/Transition.test.js

@@ -11,4 +11,8 @@ describe('Transition', () => {
   it('basic', () => {
     expect(shallow(<Transition name={PUSH} />)).toMatchSnapshot();
   });
+
+  it('label', () => {
+    expect(shallow(<Transition name={PUSH} label="Page explorer" />)).toMatchSnapshot();
+  });
 });

client/src/components/Transition/__snapshots__/Transition.test.js.snap

@@ -2,6 +2,21 @@
 
 exports[`Transition basic 1`] = `
 <CSSTransitionGroup
+  aria-label={null}
+  className={null}
+  component="div"
+  transitionAppear={false}
+  transitionEnter={true}
+  transitionEnterTimeout={210}
+  transitionLeave={true}
+  transitionLeaveTimeout={210}
+  transitionName="c-transition-push"
+/>
+`;
+
+exports[`Transition label 1`] = `
+<CSSTransitionGroup
+  aria-label="Page explorer"
   className={null}
   component="div"
   transitionAppear={false}

client/tests/stubs.js

@@ -44,6 +44,7 @@ global.wagtailConfig = {
     CLOSE: 'Close',
     EDIT_PAGE: 'Edit \'{title}\'',
     VIEW_CHILD_PAGES_OF_PAGE: 'View child pages of \'{title}\'',
+    PAGE_EXPLORER: 'Page explorer',
   },
 };
 

docs/releases/2.6.rst

@@ -33,6 +33,7 @@ This release also contains many big improvements for screen reader users:
 * Screen readers now treat page-level action dropdowns as navigation instead of menus (Helen Chapman)
 * Fixed occurrences of invalid HTML across the CMS admin (Thibaud Colas)
 * Add empty alt attributes to all images in the CMS admin (Andreas Bernacca)
+* Fixed focus not moving to the pages explorer menu when open (Helen Chapman)
 
 We’ve also had a look at how controls are labeled across the UI for screen reader users:
 

wagtail/admin/templates/wagtailadmin/admin_base.html

@@ -52,6 +52,7 @@
                 CLOSE: "{% trans 'Close' %}",
                 EDIT_PAGE: "{% trans 'Edit \'{title}\'' %}",
                 VIEW_CHILD_PAGES_OF_PAGE: "{% trans 'View child pages of \'{title}\'' %}",
+                PAGE_EXPLORER: "{% trans 'Page explorer' %}",
 
                 MONTHS: [
                     "{% trans 'January' %}",