Minor accessibility fixes to new slim sidebar menu items (#8015)

* Consistently set `aria-haspopup="menu"` for all sidebar menu items that have sub-menus (LB (Ben Johnston))
 * Make sure `aria-expanded` is always explicitly set as a string in sidebar (LB (Ben Johnston))
 * Use a button element instead of a link for page explorer menu item, for the correct semantics and behavior (LB (Ben Johnston))

CHANGELOG.txt

@@ -28,6 +28,9 @@ Changelog
  * Fix: Improve the contrast of the “Remember me” checkbox against the login page’s background (Steven Steinwand)
  * Fix: Group permission rows with custom permissions no longer have extra padding (Steven Steinwand)
  * Fix: Make sure the focus outline of checkboxes is fully around the outer border (Steven Steinwand)
+ * Fix: Consistently set `aria-haspopup="menu"` for all sidebar menu items that have sub-menus (LB (Ben Johnston))
+ * Fix: Make sure `aria-expanded` is always explicitly set as a string in sidebar (LB (Ben Johnston))
+ * Fix: Use a button element instead of a link for page explorer menu item, for the correct semantics and behavior (LB (Ben Johnston))
 
 
 2.16.2 (xx.xx.xxxx) - IN DEVELOPMENT

client/src/components/Sidebar/Sidebar.test.js

@@ -0,0 +1,68 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { Sidebar } from './Sidebar';
+
+describe('Sidebar', () => {
+  const strings = {};
+
+  it('should render with the minimum required props', () => {
+    const wrapper = shallow(<Sidebar modules={[]} strings={strings} />);
+
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  it('should toggle the slim mode in the sidebar when outer button clicked', () => {
+    const onExpandCollapse = jest.fn();
+
+    const wrapper = shallow(
+      <Sidebar
+        modules={[]}
+        onExpandCollapse={onExpandCollapse}
+        strings={strings}
+      />,
+    );
+
+    // default expanded (non-slim)
+    expect(
+      wrapper.find('.sidebar__collapse-toggle').prop('aria-expanded'),
+    ).toEqual('true');
+    expect(wrapper.find('.sidebar--slim')).toHaveLength(0);
+    expect(onExpandCollapse).not.toHaveBeenCalled();
+
+    // toggle slim mode
+    wrapper.find('.sidebar__collapse-toggle').simulate('click');
+
+    expect(
+      wrapper.find('.sidebar__collapse-toggle').prop('aria-expanded'),
+    ).toEqual('false');
+    expect(wrapper.find('.sidebar--slim')).toHaveLength(1);
+    expect(onExpandCollapse).toHaveBeenCalledWith(true);
+  });
+
+  it('should toggle the sidebar visibility on click (used on mobile)', () => {
+    const onExpandCollapse = jest.fn();
+
+    const wrapper = shallow(
+      <Sidebar
+        modules={[]}
+        onExpandCollapse={onExpandCollapse}
+        strings={strings}
+      />,
+    );
+
+    // default not expanded
+    expect(wrapper.find('.sidebar-nav-toggle').prop('aria-expanded')).toEqual(
+      'false',
+    );
+    expect(wrapper.find('.sidebar-nav-toggle--open')).toHaveLength(0);
+
+    // toggle expanded mode
+    wrapper.find('.sidebar-nav-toggle').simulate('click');
+
+    // check it is expanded
+    expect(wrapper.find('.sidebar-nav-toggle').prop('aria-expanded')).toEqual(
+      'true',
+    );
+    expect(wrapper.find('.sidebar-nav-toggle--open')).toHaveLength(1);
+  });
+});

client/src/components/Sidebar/Sidebar.tsx

@@ -38,7 +38,7 @@ export interface SidebarProps {
 export const Sidebar: React.FunctionComponent<SidebarProps> = ({
   modules,
   currentPath,
-  collapsedOnLoad,
+  collapsedOnLoad = false,
   strings,
   navigate,
   onExpandCollapse,
@@ -104,8 +104,7 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
     };
   }, [slim]);
 
-  const onClickCollapseToggle = (e: React.MouseEvent) => {
+  const onClickCollapseToggle = () => {
-    e.preventDefault();
     setCollapsed(!collapsed);
 
     if (onExpandCollapse) {
@@ -113,8 +112,7 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
     }
   };
 
-  const onClickOpenCloseToggle = (e: React.MouseEvent) => {
+  const onClickOpenCloseToggle = () => {
-    e.preventDefault();
     setVisibleOnMobile(!visibleOnMobile);
     setExpandingOrCollapsing(true);
 
@@ -185,10 +183,11 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
       >
         <div className="sidebar__inner">
           <button
+            className="button sidebar__collapse-toggle"
             onClick={onClickCollapseToggle}
             aria-label={strings.TOGGLE_SIDEBAR}
-            aria-expanded={!slim}
+            aria-expanded={slim ? 'false' : 'true'}
-            className="button sidebar__collapse-toggle"
+            type="button"
           >
             {collapsed ? (
               <Icon name="angle-double-right" />
@@ -211,12 +210,13 @@ export const Sidebar: React.FunctionComponent<SidebarProps> = ({
       <button
         onClick={onClickOpenCloseToggle}
         aria-label={strings.TOGGLE_SIDEBAR}
-        aria-expanded={visibleOnMobile}
+        aria-expanded={visibleOnMobile ? 'true' : 'false'}
         className={
           'button sidebar-nav-toggle' +
           (isMobile ? ' sidebar-nav-toggle--mobile' : '') +
           (visibleOnMobile ? ' sidebar-nav-toggle--open' : '')
         }
+        type="button"
       >
         {visibleOnMobile ? <Icon name="cross" /> : <Icon name="bars" />}
       </button>

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

@@ -0,0 +1,41 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Sidebar should render with the minimum required props 1`] = `
+<Fragment>
+  <div
+    className="sidebar"
+  >
+    <div
+      className="sidebar__inner"
+    >
+      <button
+        aria-expanded="true"
+        className="button sidebar__collapse-toggle"
+        onClick={[Function]}
+        type="button"
+      >
+        <Icon
+          name="angle-double-left"
+        />
+      </button>
+      <div
+        className="sidebar__peek-hover-area"
+        onBlur={[Function]}
+        onFocus={[Function]}
+        onMouseEnter={[Function]}
+        onMouseLeave={[Function]}
+      />
+    </div>
+  </div>
+  <button
+    aria-expanded="false"
+    className="button sidebar-nav-toggle"
+    onClick={[Function]}
+    type="button"
+  >
+    <Icon
+      name="bars"
+    />
+  </button>
+</Fragment>
+`;

client/src/components/Sidebar/menu/PageExplorerMenuItem.tsx

@@ -1,6 +1,5 @@
 import * as React from 'react';
 
-import Button from '../../Button/Button';
 import Icon from '../../Icon/Icon';
 import { MenuItemProps } from './MenuItem';
 import { LinkMenuItemDefinition } from './LinkMenuItem';
@@ -47,9 +46,7 @@ export const PageExplorerMenuItem: React.FunctionComponent<
     }
   }, [isOpen]);
 
-  const onClick = (e: React.MouseEvent) => {
+  const onClick = () => {
-    e.preventDefault();
-
     // Open/close explorer
     if (isOpen) {
       dispatch({
@@ -65,7 +62,7 @@ export const PageExplorerMenuItem: React.FunctionComponent<
   };
 
   const className =
-    'sidebar-menu-item' +
+    'sidebar-menu-item sidebar-page-explorer-item' +
     (isActive ? ' sidebar-menu-item--active' : '') +
     (isInSubMenu ? ' sidebar-menu-item--in-sub-menu' : '');
 
@@ -75,15 +72,17 @@ export const PageExplorerMenuItem: React.FunctionComponent<
 
   return (
     <li className={className}>
-      <Button
+      <button
-        dialogTrigger={true}
         onClick={onClick}
         className="sidebar-menu-item__link"
+        aria-haspopup="menu"
+        aria-expanded={isOpen ? 'true' : 'false'}
+        type="button"
       >
         <Icon name="folder-open-inverse" className="icon--menuitem" />
         <span className="menuitem-label">{item.label}</span>
         <Icon className={sidebarTriggerIconClassName} name="arrow-right" />
-      </Button>
+      </button>
       <div>
         <SidebarPanel
           isVisible={isVisible}

client/src/components/Sidebar/menu/PageExplorererMenuItem.test.js

@@ -0,0 +1,70 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { PageExplorerMenuItem } from './PageExplorerMenuItem';
+
+describe('PageExplorerMenuItem', () => {
+  const state = { activePath: '.reports.workflows', navigationPath: '' };
+
+  it('should render with the minimum required props', () => {
+    const wrapper = shallow(
+      <PageExplorerMenuItem item={{}} path=".explorer" state={state} />,
+    );
+
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  it('should expand the explorer menu when clicked', () => {
+    const dispatch = jest.fn();
+    const preventDefault = jest.fn();
+
+    const wrapper = shallow(
+      <PageExplorerMenuItem
+        dispatch={dispatch}
+        item={{}}
+        path=".explorer"
+        state={state}
+      />,
+    );
+
+    expect(
+      wrapper.find('.sidebar-menu-item__link').prop('aria-expanded'),
+    ).toEqual('false');
+    expect(wrapper.find('SidebarPanel').prop('isOpen')).toBe(false);
+    expect(dispatch).not.toHaveBeenCalled();
+    expect(preventDefault).not.toHaveBeenCalled();
+
+    // click the button
+    wrapper
+      .find('.sidebar-menu-item__link')
+      .simulate('click', { preventDefault });
+
+    expect(dispatch).toHaveBeenCalledWith({
+      path: '.explorer',
+      type: 'set-navigation-path',
+    });
+    expect(preventDefault).not.toHaveBeenCalled();
+
+    // manually update the state as if the redux action was dispatched
+    wrapper.setProps({
+      state: { activePath: '.reports.workflows', navigationPath: '.explorer' },
+    });
+
+    // check that the expanded state is working
+    expect(
+      wrapper.find('.sidebar-menu-item__link').prop('aria-expanded'),
+    ).toEqual('true');
+    expect(wrapper.find('SidebarPanel').prop('isOpen')).toBe(true);
+
+    // click the button to close
+    wrapper
+      .find('.sidebar-menu-item__link')
+      .simulate('click', { preventDefault });
+
+    expect(dispatch).toHaveBeenCalledTimes(2);
+    expect(dispatch).toHaveBeenLastCalledWith({
+      path: '',
+      type: 'set-navigation-path',
+    });
+    expect(preventDefault).not.toHaveBeenCalled();
+  });
+});

client/src/components/Sidebar/menu/SubMenuItem.test.js

@@ -0,0 +1,71 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { SubMenuItem } from './SubMenuItem';
+
+describe('SubMenuItem', () => {
+  const strings = {};
+  const state = { activePath: '.reports.workflows', navigationPath: '' };
+
+  it('should render with the minimum required props', () => {
+    const wrapper = shallow(
+      <SubMenuItem
+        item={{ classNames: '', menuItems: [] }}
+        items={[]}
+        state={state}
+        path=".reports"
+        strings={strings}
+      />,
+    );
+
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  it('should provide a button to expand the sub-menu', () => {
+    const dispatch = jest.fn();
+
+    const wrapper = shallow(
+      <SubMenuItem
+        dispatch={dispatch}
+        item={{ classNames: '', menuItems: [] }}
+        items={[]}
+        state={state}
+        path=".reports"
+        strings={strings}
+      />,
+    );
+
+    expect(wrapper.find('.sidebar-menu-item__link').prop('aria-expanded')).toBe(
+      'false',
+    );
+    expect(dispatch).not.toHaveBeenCalled();
+    expect(wrapper.find('.sidebar-sub-menu-item--open')).toHaveLength(0);
+
+    // click the sub menu item
+    wrapper.find('.sidebar-menu-item__link').simulate('click');
+
+    // check the dispatch function (redux state) was called
+    expect(dispatch).toHaveBeenCalledWith({
+      path: '.reports',
+      type: 'set-navigation-path',
+    });
+
+    // manually update the state as if the redux action was dispatched
+    wrapper.setProps({
+      state: { navigationPath: '.reports', activePath: '.reports.workflows' },
+    });
+
+    expect(wrapper.find('.sidebar-menu-item__link').prop('aria-expanded')).toBe(
+      'true',
+    );
+    expect(wrapper.find('.sidebar-sub-menu-item--open')).toHaveLength(1);
+
+    // click a second time to 'close'
+    wrapper.find('.sidebar-menu-item__link').simulate('click');
+
+    expect(dispatch).toHaveBeenCalledTimes(2);
+    expect(dispatch).toHaveBeenLastCalledWith({
+      path: '',
+      type: 'set-navigation-path',
+    });
+  });
+});

client/src/components/Sidebar/menu/SubMenuItem.tsx

@@ -37,7 +37,7 @@ export const SubMenuItem: React.FunctionComponent<SubMenuItemProps> = ({
     }
   }, [isOpen]);
 
-  const onClick = (e: React.MouseEvent) => {
+  const onClick = () => {
     if (isOpen) {
       const pathComponents = path.split('.');
       pathComponents.pop();
@@ -52,8 +52,6 @@ export const SubMenuItem: React.FunctionComponent<SubMenuItemProps> = ({
         path,
       });
     }
-
-    e.preventDefault();
   };
 
   const className =
@@ -70,8 +68,9 @@ export const SubMenuItem: React.FunctionComponent<SubMenuItemProps> = ({
       <button
         onClick={onClick}
         className={`sidebar-menu-item__link ${item.classNames}`}
-        aria-haspopup="true"
+        aria-haspopup="menu"
         aria-expanded={isOpen ? 'true' : 'false'}
+        type="button"
       >
         {item.iconName && (
           <Icon name={item.iconName} className="icon--menuitem" />

client/src/components/Sidebar/menu/__snapshots__/PageExplorererMenuItem.test.js.snap

@@ -0,0 +1,51 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`PageExplorerMenuItem should render with the minimum required props 1`] = `
+<li
+  className="sidebar-menu-item sidebar-page-explorer-item"
+>
+  <button
+    aria-expanded="false"
+    aria-haspopup="menu"
+    className="sidebar-menu-item__link"
+    onClick={[Function]}
+    type="button"
+  >
+    <Icon
+      className="icon--menuitem"
+      name="folder-open-inverse"
+    />
+    <span
+      className="menuitem-label"
+    />
+    <Icon
+      className="sidebar-sub-menu-trigger-icon"
+      name="arrow-right"
+    />
+  </button>
+  <div>
+    <SidebarPanel
+      depth={2}
+      isOpen={false}
+      isVisible={false}
+      widthPx={485}
+    >
+      <Provider
+        store={
+          Object {
+            "@@observable": [Function],
+            "dispatch": [Function],
+            "getState": [Function],
+            "replaceReducer": [Function],
+            "subscribe": [Function],
+          }
+        }
+      >
+        <Connect(PageExplorer)
+          isVisible={false}
+        />
+      </Provider>
+    </SidebarPanel>
+  </div>
+</li>
+`;

client/src/components/Sidebar/menu/__snapshots__/SubMenuItem.test.js.snap

@@ -0,0 +1,40 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`SubMenuItem should render with the minimum required props 1`] = `
+<li
+  className="sidebar-menu-item sidebar-sub-menu-item sidebar-menu-item--active"
+>
+  <button
+    aria-expanded="false"
+    aria-haspopup="menu"
+    className="sidebar-menu-item__link "
+    onClick={[Function]}
+    type="button"
+  >
+    <span
+      className="menuitem-label"
+    />
+    <Icon
+      className="sidebar-sub-menu-trigger-icon"
+      name="arrow-right"
+    />
+  </button>
+  <SidebarPanel
+    depth={2}
+    isOpen={false}
+    isVisible={false}
+  >
+    <div
+      className="sidebar-sub-menu-panel"
+    >
+      <h2
+        className=""
+        id="wagtail-sidebar-submenu-reports-title"
+      />
+      <ul
+        aria-labelledby="wagtail-sidebar-submenu-reports-title"
+      />
+    </div>
+  </SidebarPanel>
+</li>
+`;

client/src/components/Sidebar/modules/MainMenu.test.js

@@ -0,0 +1,45 @@
+import React from 'react';
+import { shallow } from 'enzyme';
+import { Menu } from './MainMenu';
+
+describe('Menu', () => {
+  const strings = {};
+  const user = { avatarUrl: 'https://gravatar/profile' };
+
+  it('should render with the minimum required props', () => {
+    const wrapper = shallow(
+      <Menu
+        accountMenuItems={[]}
+        menuItems={[]}
+        strings={strings}
+        user={user}
+      />,
+    );
+
+    expect(wrapper).toMatchSnapshot();
+  });
+
+  it('should toggle the sidebar footer (account) when clicked', () => {
+    const wrapper = shallow(
+      <Menu
+        accountMenuItems={[]}
+        menuItems={[]}
+        strings={strings}
+        user={user}
+      />,
+    );
+
+    // default is closed
+    expect(wrapper.find('.sidebar-footer__account').prop('aria-expanded')).toBe(
+      'false',
+    );
+    expect(wrapper.find('.sidebar-footer--open')).toHaveLength(0);
+
+    wrapper.find('.sidebar-footer__account').simulate('click');
+
+    expect(wrapper.find('.sidebar-footer__account').prop('aria-expanded')).toBe(
+      'true',
+    );
+    expect(wrapper.find('.sidebar-footer--open')).toHaveLength(1);
+  });
+});

client/src/components/Sidebar/modules/MainMenu.tsx

@@ -170,9 +170,7 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
     }
   }, [expandingOrCollapsing]);
 
-  const onClickAccountSettings = (e: React.MouseEvent) => {
+  const onClickAccountSettings = () => {
-    e.preventDefault();
-
     if (accountSettingsOpen) {
       dispatch({
         type: 'set-navigation-path',
@@ -206,10 +204,11 @@ export const Menu: React.FunctionComponent<MenuProps> = ({
         <button
           className="sidebar-footer__account"
           title={strings.EDIT_YOUR_ACCOUNT}
-          aria-label={strings.EDIT_YOUR_ACCOUNT}
           onClick={onClickAccountSettings}
-          aria-haspopup="true"
+          aria-label={strings.EDIT_YOUR_ACCOUNT}
+          aria-haspopup="menu"
           aria-expanded={accountSettingsOpen ? 'true' : 'false'}
+          type="button"
         >
           <div className="avatar square avatar-on-dark">
             <img src={user.avatarUrl} alt="" />

client/src/components/Sidebar/modules/__snapshots__/MainMenu.test.js.snap

@@ -0,0 +1,45 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Menu should render with the minimum required props 1`] = `
+<Fragment>
+  <nav
+    className="sidebar-main-menu"
+  >
+    <ul
+      className="sidebar-main-menu__list"
+    />
+  </nav>
+  <div
+    className="sidebar-footer"
+  >
+    <button
+      aria-expanded="false"
+      aria-haspopup="menu"
+      className="sidebar-footer__account"
+      onClick={[Function]}
+      type="button"
+    >
+      <div
+        className="avatar square avatar-on-dark"
+      >
+        <img
+          alt=""
+          src="https://gravatar/profile"
+        />
+      </div>
+      <div
+        className="sidebar-footer__account-toggle"
+      >
+        <div
+          className="sidebar-footer__account-label"
+        />
+        <Icon
+          className="sidebar-footer__account-icon"
+          name="arrow-up"
+        />
+      </div>
+    </button>
+    <ul />
+  </div>
+</Fragment>
+`;

client/tests/integration/homepage.test.js

@@ -18,7 +18,9 @@ describe('Homepage', () => {
   });
 
   it('axe page explorer', async () => {
-    const trigger = await page.$('[aria-haspopup="dialog"]');
+    const trigger = await page.$(
+      '.sidebar-page-explorer-item [aria-haspopup="menu"]',
+    );
     await trigger.click();
     await expect(page).toPassAxeTests({
       include: '.sidebar-main-menu',
@@ -26,7 +28,9 @@ describe('Homepage', () => {
   });
 
   it('axe sidebar sub-menu', async () => {
-    const trigger = await page.$('[aria-haspopup="true"]');
+    const trigger = await page.$(
+      '.sidebar-sub-menu-item [aria-haspopup="menu"]',
+    );
     await trigger.click();
     await expect(page).toPassAxeTests({
       include: '.sidebar-main-menu',

docs/releases/2.17.md

@@ -53,6 +53,9 @@ The panel types `StreamFieldPanel`, `RichTextFieldPanel`, `ImageChooserPanel`, `
  * Improve the contrast of the “Remember me” checkbox against the login page’s background (Steven Steinwand)
  * Group permission rows with custom permissions no longer have extra padding (Steven Steinwand)
  * Make sure the focus outline of checkboxes is fully around the outer border (Steven Steinwand)
+ * Consistently set `aria-haspopup="menu"` for all sidebar menu items that have sub-menus (LB (Ben Johnston))
+ * Make sure `aria-expanded` is always explicitly set as a string in sidebar (LB (Ben Johnston))
+ * Use a button element instead of a link for page explorer menu item, for the correct semantics and behavior (LB (Ben Johnston))
 
 
 ## Upgrade considerations