Improve accessibility of toggle and checkbox menu items

src/components/Menu/CheckboxMenuItem.stories.tsx

@@ -17,7 +17,7 @@ type Props = Omit<
 const Template: React.FC<Props> = (props: Props) => {
   const [firstChecked, setFirstChecked] = useState(false);
   return (
-    <div style={{ width: 300 }}>
+    <div style={{ width: 300 }} role="menu">
       <CheckboxMenuItemComponent
         {...props}
         label="First item"

src/components/Menu/CheckboxMenuItem.tsx

@@ -39,15 +39,16 @@ export const CheckboxMenuItem = forwardRef<HTMLInputElement, Props>(
     const onChange = useCallback(() => {}, []);
 
     // <label> elements are not allowed to have a role like menuitemcheckbox, so
-    // we must instead use a plain <div> for the menu item and use aria-checked
+    // we must instead use a <button> for the menu item and use aria-checked
     // etc. to communicate its state.
     return (
       <MenuItem
-        as="div"
+        as="button"
         role="menuitemcheckbox"
         aria-checked={checked}
         className={className}
         label={label}
+        hideChevron
         onSelect={onSelect}
         disabled={disabled}
         Icon={
@@ -56,6 +57,7 @@ export const CheckboxMenuItem = forwardRef<HTMLInputElement, Props>(
             ref={ref}
             // This is purely cosmetic; really the whole MenuItem is the toggle.
             aria-hidden
+            tabIndex={-1}
             checked={checked}
             disabled={disabled}
             onChange={onChange}

src/components/Menu/ToggleMenuItem.stories.tsx

@@ -20,7 +20,7 @@ type Props = Omit<
 const Template: React.FC<Props> = (props: Props) => {
   const [firstChecked, setFirstChecked] = useState(false);
   return (
-    <div style={{ width: 300 }}>
+    <div style={{ width: 300 }} role="menu">
       <ToggleMenuItemComponent
         {...props}
         Icon={ChatIcon}

src/components/Menu/ToggleMenuItem.tsx

@@ -43,12 +43,13 @@ export const ToggleMenuItem = forwardRef<HTMLInputElement, Props>(
     // etc. to communicate its state.
     return (
       <MenuItem
-        as="div"
+        as="button"
         role="menuitemcheckbox"
         aria-checked={checked}
         className={className}
         Icon={Icon}
         label={label}
+        hideChevron
         onSelect={onSelect}
         disabled={disabled}
         onClick={onClick}
@@ -58,6 +59,7 @@ export const ToggleMenuItem = forwardRef<HTMLInputElement, Props>(
           ref={ref}
           // This is purely cosmetic; really the whole MenuItem is the toggle.
           aria-hidden
+          tabIndex={-1}
           checked={checked}
           disabled={disabled}
           onChange={onChange}

src/components/Menu/__snapshots__/CheckboxMenuItem.test.tsx.snap

@@ -2,7 +2,7 @@
 
 exports[`CheckboxMenuItem > renders 1`] = `
 <DocumentFragment>
-  <div
+  <button
     aria-checked="false"
     class="_item_9618f6 _interactive_9618f6"
     data-kind="primary"
@@ -15,6 +15,7 @@ exports[`CheckboxMenuItem > renders 1`] = `
         aria-hidden="true"
         class="_input_937330"
         id="_r_0_"
+        tabindex="-1"
         type="checkbox"
       />
       <div
@@ -39,6 +40,6 @@ exports[`CheckboxMenuItem > renders 1`] = `
     >
       Always show
     </span>
-  </div>
+  </button>
 </DocumentFragment>
 `;

src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap

@@ -2,7 +2,7 @@
 
 exports[`ToggleMenuItem > renders 1`] = `
 <DocumentFragment>
-  <div
+  <button
     aria-checked="false"
     class="_item_9618f6 _interactive_9618f6"
     data-kind="primary"
@@ -34,12 +34,13 @@ exports[`ToggleMenuItem > renders 1`] = `
         class="_input_d9763f"
         id="_r_0_"
         role="switch"
+        tabindex="-1"
         type="checkbox"
       />
       <div
         class="_ui_d9763f"
       />
     </div>
-  </div>
+  </button>
 </DocumentFragment>
 `;