diff --git a/CHANGELOG.txt b/CHANGELOG.txt index eb34f934e7..4da30dd4f1 100644 --- a/CHANGELOG.txt +++ b/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 diff --git a/client/src/components/Sidebar/Sidebar.test.js b/client/src/components/Sidebar/Sidebar.test.js new file mode 100644 index 0000000000..8c10b90f46 --- /dev/null +++ b/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(); + + expect(wrapper).toMatchSnapshot(); + }); + + it('should toggle the slim mode in the sidebar when outer button clicked', () => { + const onExpandCollapse = jest.fn(); + + const wrapper = shallow( + , + ); + + // 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( + , + ); + + // 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); + }); +}); diff --git a/client/src/components/Sidebar/Sidebar.tsx b/client/src/components/Sidebar/Sidebar.tsx index 541709a1f4..db114d92dc 100644 --- a/client/src/components/Sidebar/Sidebar.tsx +++ b/client/src/components/Sidebar/Sidebar.tsx @@ -38,7 +38,7 @@ export interface SidebarProps { export const Sidebar: React.FunctionComponent = ({ modules, currentPath, - collapsedOnLoad, + collapsedOnLoad = false, strings, navigate, onExpandCollapse, @@ -104,8 +104,7 @@ export const Sidebar: React.FunctionComponent = ({ }; }, [slim]); - const onClickCollapseToggle = (e: React.MouseEvent) => { - e.preventDefault(); + const onClickCollapseToggle = () => { setCollapsed(!collapsed); if (onExpandCollapse) { @@ -113,8 +112,7 @@ export const Sidebar: React.FunctionComponent = ({ } }; - const onClickOpenCloseToggle = (e: React.MouseEvent) => { - e.preventDefault(); + const onClickOpenCloseToggle = () => { setVisibleOnMobile(!visibleOnMobile); setExpandingOrCollapsing(true); @@ -185,10 +183,11 @@ export const Sidebar: React.FunctionComponent = ({ >
diff --git a/client/src/components/Sidebar/__snapshots__/Sidebar.test.js.snap b/client/src/components/Sidebar/__snapshots__/Sidebar.test.js.snap new file mode 100644 index 0000000000..2889aa448c --- /dev/null +++ b/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`] = ` + +
+
+ +
+
+
+ + +`; diff --git a/client/src/components/Sidebar/menu/PageExplorerMenuItem.tsx b/client/src/components/Sidebar/menu/PageExplorerMenuItem.tsx index 862cc00bd0..f17f525ef3 100644 --- a/client/src/components/Sidebar/menu/PageExplorerMenuItem.tsx +++ b/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) => { - e.preventDefault(); - + const onClick = () => { // 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 (
  • - +
    { + const state = { activePath: '.reports.workflows', navigationPath: '' }; + + it('should render with the minimum required props', () => { + const wrapper = shallow( + , + ); + + expect(wrapper).toMatchSnapshot(); + }); + + it('should expand the explorer menu when clicked', () => { + const dispatch = jest.fn(); + const preventDefault = jest.fn(); + + const wrapper = shallow( + , + ); + + 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(); + }); +}); diff --git a/client/src/components/Sidebar/menu/SubMenuItem.test.js b/client/src/components/Sidebar/menu/SubMenuItem.test.js new file mode 100644 index 0000000000..f3ffd53568 --- /dev/null +++ b/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( + , + ); + + expect(wrapper).toMatchSnapshot(); + }); + + it('should provide a button to expand the sub-menu', () => { + const dispatch = jest.fn(); + + const wrapper = shallow( + , + ); + + 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', + }); + }); +}); diff --git a/client/src/components/Sidebar/menu/SubMenuItem.tsx b/client/src/components/Sidebar/menu/SubMenuItem.tsx index 1efbb9162a..45d04efea9 100644 --- a/client/src/components/Sidebar/menu/SubMenuItem.tsx +++ b/client/src/components/Sidebar/menu/SubMenuItem.tsx @@ -37,7 +37,7 @@ export const SubMenuItem: React.FunctionComponent = ({ } }, [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 = ({ path, }); } - - e.preventDefault(); }; const className = @@ -70,8 +68,9 @@ export const SubMenuItem: React.FunctionComponent = ({ +
    + + + + + +
    +
  • +`; diff --git a/client/src/components/Sidebar/menu/__snapshots__/SubMenuItem.test.js.snap b/client/src/components/Sidebar/menu/__snapshots__/SubMenuItem.test.js.snap new file mode 100644 index 0000000000..31af2f2bb3 --- /dev/null +++ b/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`] = ` +
  • + + +
    +

    +
      +

    +
    +
  • +`; diff --git a/client/src/components/Sidebar/modules/MainMenu.test.js b/client/src/components/Sidebar/modules/MainMenu.test.js new file mode 100644 index 0000000000..d6f902860c --- /dev/null +++ b/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( + , + ); + + expect(wrapper).toMatchSnapshot(); + }); + + it('should toggle the sidebar footer (account) when clicked', () => { + const wrapper = shallow( + , + ); + + // 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); + }); +}); diff --git a/client/src/components/Sidebar/modules/MainMenu.tsx b/client/src/components/Sidebar/modules/MainMenu.tsx index 6dd379bb8c..3309c5fa1e 100644 --- a/client/src/components/Sidebar/modules/MainMenu.tsx +++ b/client/src/components/Sidebar/modules/MainMenu.tsx @@ -170,9 +170,7 @@ export const Menu: React.FunctionComponent = ({ } }, [expandingOrCollapsing]); - const onClickAccountSettings = (e: React.MouseEvent) => { - e.preventDefault(); - + const onClickAccountSettings = () => { if (accountSettingsOpen) { dispatch({ type: 'set-navigation-path', @@ -206,10 +204,11 @@ export const Menu: React.FunctionComponent = ({ +
      +
    +
    +`; diff --git a/client/tests/integration/homepage.test.js b/client/tests/integration/homepage.test.js index 83fe98fd25..33b2159a9d 100644 --- a/client/tests/integration/homepage.test.js +++ b/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', diff --git a/docs/releases/2.17.md b/docs/releases/2.17.md index a06e04a21f..3d0b67ff1c 100644 --- a/docs/releases/2.17.md +++ b/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