refactor(menu): improves types and component readbility

improves types and component readbility by screen readers
catho · Apr 30, 2024 · d001939 · d001939
1 parent 3e5bb7f
commit d001939
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 72 deletions.
diff --git a/components/Menu/Menu.jsx b/components/Menu/Menu.jsx
@@ -26,11 +26,16 @@ const Menu = (props) => {
     anchorEl,
     anchorOrigin,
     transformOrigin,
-    handleClose,
+    onClose,
     items,
     keepMounted,
   } = props;
 
+  const itemOnClick = (item) => {
+    const { onClick } = item;
+    onClick();
+    onClose();
+  };
   return (
     <MaterialMenu
       id="menu"
@@ -40,29 +45,22 @@ const Menu = (props) => {
       anchorReference="anchorEl"
       anchorEl={anchorEl}
       open={open}
-      onClose={handleClose}
-      onBlur={handleClose}
+      onClose={onClose}
+      onBlur={onClose}
       anchorOrigin={anchorOrigin}
       transformOrigin={transformOrigin}
       keepMounted={keepMounted}
     >
-      {items.map((item) => {
-        const onClickFunc = () => {
-          const { handleClick } = item;
-          handleClose();
-          handleClick();
-        };
-        return (
-          <MenuItem
-            key={item.id}
-            onClick={onClickFunc}
-            theme={materialThemeOverride}
-            color="textPrimary"
-          >
-            {item.content}
-          </MenuItem>
-        );
-      })}
+      {items.map((item) => (
+        <MenuItem
+          key={item.id}
+          onClick={() => itemOnClick(item)}
+          theme={materialThemeOverride}
+          color="textPrimary"
+        >
+          {item.content}
+        </MenuItem>
+      ))}
     </MaterialMenu>
   );
 };
@@ -71,7 +69,13 @@ Menu.propTypes = {
   /** If true, the component is shown. */
   open: PropTypes.bool,
   /** Menu contents, has a content parameter, an id and a handleClick function. */
-  items: PropTypes.array,
+  items: PropTypes.arrayOf(
+    PropTypes.shape({
+      id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
+      content: PropTypes.ReactNode,
+      onClick: PropTypes.func,
+    }),
+  ),
   /**	An HTML element, or a function that returns one. It's used to set the position of the menu. */
   anchorEl: PropTypes.object,
   /** The point on the anchor where the popover's anchorEl will attach to.
@@ -87,7 +91,7 @@ Menu.propTypes = {
   /** Always keep the children in the DOM. */
   keepMounted: PropTypes.bool,
   /** Callback fired when the component requests to be closed. */
-  handleClose: PropTypes.func,
+  onClose: PropTypes.func,
 };
 
 Menu.defaultProps = {
@@ -103,7 +107,7 @@ Menu.defaultProps = {
     horizontal: 'left',
   },
   keepMounted: false,
-  handleClose: () => {},
+  onClose: () => {},
 };
 
 export default Menu;
diff --git a/components/Menu/Menu.unit.test.jsx b/components/Menu/Menu.unit.test.jsx
@@ -4,8 +4,8 @@ import Menu from './Menu';
 
 describe('<Menu />', () => {
   const mockItems = [
-    { id: 'item-1', content: 'Item 1', handleClick: jest.fn() },
-    { id: 'item-2', content: 'Item 2', handleClick: jest.fn() },
+    { id: 'item-1', content: 'Item 1', onClick: jest.fn() },
+    { id: 'item-2', content: 'Item 2', onClick: jest.fn() },
   ];
 
   const mockAnchorEl = document.createElement('div');
@@ -21,7 +21,7 @@ describe('<Menu />', () => {
       open: true,
       items: mockItems,
       anchorEl: mockAnchorEl,
-      handleClose: mockHandleClose,
+      onClose: mockHandleClose,
     });
 
     mockItems.forEach((item) => {
@@ -30,29 +30,29 @@ describe('<Menu />', () => {
     });
   });
 
-  it('calls handleClose when an item is clicked', () => {
+  it('calls onClose when an item is clicked', () => {
     const { getByText } = renderMenu({
       open: true,
       items: mockItems,
       anchorEl: mockAnchorEl,
-      handleClose: mockHandleClose,
+      onClose: mockHandleClose,
     });
 
     mockItems.forEach((item) => {
       const renderedItem = getByText(item.content);
       fireEvent.click(renderedItem);
-      expect(item.handleClick).toHaveBeenCalledTimes(1);
+      expect(item.onClick).toHaveBeenCalledTimes(1);
     });
   });
 
   it('renders with anchorEl when open is true', () => {
-    const { getByTestId } = renderMenu({
+    const { getByRole } = renderMenu({
       open: true,
       items: mockItems,
       anchorEl: mockAnchorEl,
-      handleClose: mockHandleClose,
+      onClose: mockHandleClose,
     });
-    const menu = getByTestId('menu');
+    const menu = getByRole('presentation');
     expect(menu).toBeInTheDocument();
   });
 
@@ -61,20 +61,20 @@ describe('<Menu />', () => {
       open: false,
       items: mockItems,
       anchorEl: mockAnchorEl,
-      handleClose: mockHandleClose,
+      onClose: mockHandleClose,
     });
     const menu = queryByTestId('menu');
     expect(menu).toBeNull();
   });
 
-  it('calls handleClose when onBlur event is triggered', () => {
-    const { getByTestId } = renderMenu({
+  it('calls onClose when onBlur event is triggered', () => {
+    const { getByRole } = renderMenu({
       open: true,
       items: mockItems,
       anchorEl: mockAnchorEl,
-      handleClose: mockHandleClose,
+      onClose: mockHandleClose,
     });
-    const menu = getByTestId('menu');
+    const menu = getByRole('presentation');
     fireEvent.blur(menu);
     expect(mockHandleClose).toHaveBeenCalledTimes(1);
   });
@@ -84,7 +84,7 @@ describe('<Menu />', () => {
       open: true,
       items: [],
       anchorEl: mockAnchorEl,
-      handleClose: mockHandleClose,
+      onClose: mockHandleClose,
     });
     mockItems.forEach((item) => {
       const renderedItem = queryByText(item.content);

diff --git a/components/Menu/index.d.ts b/components/Menu/index.d.ts
@@ -4,7 +4,7 @@ import { FC } from 'react';
 export interface MenuItemProps {
   id: string | number;
   content: React.ReactNode;
-  handleClick: Function;
+  onClick: Function;
 }
 
 export interface MenuProps {
@@ -20,7 +20,7 @@ export interface MenuProps {
     horizontal: 'left' | 'center' | 'right';
   };
   keepMounted?: boolean;
-  handleClose?: (event: {}, reason: 'backdropClick' | 'escapeKeyDown') => void;
+  onClose?: (event: {}, reason: 'backdropClick' | 'escapeKeyDown') => void;
 }
 
 declare const Menu: FC<MenuProps>;

diff --git a/stories/Menu/Menu.mdx b/stories/Menu/Menu.mdx
@@ -42,18 +42,17 @@ export default function BasicMenu() {
   return (
     <div>
       <Button
-        id="button"
-        aria-controls={open ? 'menu' : undefined}
+        aria-controls="presentation"
         aria-haspopup="true"
-        aria-expanded={open ? 'true' : undefined}
+        aria-expanded={open}
         onClick={handleClick}
       >
         Menu
       </Button>
       <Menu
         open={open}
         anchorEl={anchorEl}
-        handleClose={handleClose}
+        onClose={handleClose}
         items={menuItems}
         anchorOrigin={{
           vertical: 'bottom',
@@ -90,22 +89,21 @@ export default function MenuWithIcon() {
 
   return (
     <div>
-      <div
-        aria-hidden="true"
+      <button
+        type="button"
+        aria-labelledby="Menu"
         aria-label="Menu"
-        role="button"
-        id="button"
-        aria-controls={open ? 'menu' : undefined}
+        aria-controls="presentation"
         aria-haspopup="true"
-        aria-expanded={open ? 'true' : undefined}
+        aria-expanded={open}
         onClick={handleClick}
       >
         <Icon name="menu" />{' '}
-      </div>
+      </button>
       <Menu
         open={open}
         anchorEl={anchorEl}
-        handleClose={handleClose}
+        onClose={handleClose}
         items={menuItems}
         {...args}
       />

diff --git a/stories/Menu/Menu.regression-test.story.jsx b/stories/Menu/Menu.regression-test.story.jsx
@@ -12,12 +12,12 @@ const menuItems = [
   {
     id: 'test-candidate',
     content: 'Testar candidato',
-    handleClick: () => {},
+    onClick: () => {},
   },
   {
     id: 'print-cv',
     content: 'Imprimir currículo',
-    handleClick: () => {},
+    onClick: () => {},
   },
 ];
 
@@ -34,18 +34,17 @@ const Template = (args) => {
   return (
     <div>
       <Button
-        id="button"
-        aria-controls={open ? 'menu' : undefined}
+        aria-controls="presentation"
         aria-haspopup="true"
-        aria-expanded={open ? 'true' : undefined}
+        aria-expanded={open}
         onClick={handleClick}
       >
         Menu
       </Button>
       <Menu
         open={open}
         anchorEl={anchorEl}
-        handleClose={handleClose}
+        onClose={handleClose}
         items={menuItems}
         anchorOrigin={{
           vertical: 'bottom',

diff --git a/stories/Menu/Menu.stories.jsx b/stories/Menu/Menu.stories.jsx
@@ -11,12 +11,12 @@ const menuItems = [
   {
     id: 'test-candidate',
     content: 'Testar candidato',
-    handleClick: () => {},
+    onClick: () => {},
   },
   {
     id: 'print-cv',
     content: 'Imprimir currículo',
-    handleClick: () => {},
+    onClick: () => {},
   },
 ];
 
@@ -33,18 +33,17 @@ const Template = (args) => {
   return (
     <div>
       <Button
-        id="button"
-        aria-controls={open ? 'menu' : undefined}
+        aria-controls="presentation"
         aria-haspopup="true"
-        aria-expanded={open ? 'true' : undefined}
+        aria-expanded={open}
         onClick={handleClick}
       >
         Menu
       </Button>
       <Menu
         open={open}
         anchorEl={anchorEl}
-        handleClose={handleClose}
+        onClose={handleClose}
         items={menuItems}
         anchorOrigin={{
           vertical: 'bottom',
@@ -72,22 +71,21 @@ const TemplateButtonIcon = (args) => {
 
   return (
     <div>
-      <div
-        aria-hidden="true"
+      <button
+        type="button"
+        aria-labelledby="Menu"
         aria-label="Menu"
-        role="button"
-        id="button"
-        aria-controls={open ? 'menu' : undefined}
+        aria-controls="presentation"
         aria-haspopup="true"
-        aria-expanded={open ? 'true' : undefined}
+        aria-expanded={open}
         onClick={handleClick}
       >
         <Icon name="menu" />{' '}
-      </div>
+      </button>
       <Menu
         open={open}
         anchorEl={anchorEl}
-        handleClose={handleClose}
+        onClose={handleClose}
         items={menuItems}
         {...args}
       />