From 9d4cebca0e4667463cb377d7b3ac57d94b81cd1a Mon Sep 17 00:00:00 2001
From: Robin <robin@robin.town>
Date: Thu, 14 Aug 2025 13:49:19 +0200
Subject: [PATCH 1/2] Improve accessibility of toggle and checkbox menu items

MenuItems can be used either inside a Menu component (which creates a roving tab index) or simply with an element of role menu. In the latter case, focus would previously land on the cosmetic <input> element, which is a bad idea as it's hidden from accessibility technologies. By turning the parent <div role="menuitemcheckbox"> into a <button> instead, we can ensure that focus always lands on the parent element. This is possible because ToggleMenuItem and CheckboxMenuItem are controlled components these days.
---
 src/components/Menu/CheckboxMenuItem.tsx                    | 6 ++++--
 src/components/Menu/ToggleMenuItem.tsx                      | 4 +++-
 .../Menu/__snapshots__/CheckboxMenuItem.test.tsx.snap       | 5 +++--
 .../Menu/__snapshots__/ToggleMenuItem.test.tsx.snap         | 5 +++--
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/components/Menu/CheckboxMenuItem.tsx b/src/components/Menu/CheckboxMenuItem.tsx
index 91631b2a..5e231017 100644
--- a/src/components/Menu/CheckboxMenuItem.tsx
+++ b/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}
diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx
index b12c67d2..58c82f89 100644
--- a/src/components/Menu/ToggleMenuItem.tsx
+++ b/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}
diff --git a/src/components/Menu/__snapshots__/CheckboxMenuItem.test.tsx.snap b/src/components/Menu/__snapshots__/CheckboxMenuItem.test.tsx.snap
index 63f3506f..bdcc9cd9 100644
--- a/src/components/Menu/__snapshots__/CheckboxMenuItem.test.tsx.snap
+++ b/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="«r0»"
+        tabindex="-1"
         type="checkbox"
       />
       <div
@@ -39,6 +40,6 @@ exports[`CheckboxMenuItem > renders 1`] = `
     >
       Always show
     </span>
-  </div>
+  </button>
 </DocumentFragment>
 `;
diff --git a/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap b/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap
index 9b3b59f0..142866a8 100644
--- a/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap
+++ b/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="«r0»"
         role="switch"
+        tabindex="-1"
         type="checkbox"
       />
       <div
         class="_ui_d9763f"
       />
     </div>
-  </div>
+  </button>
 </DocumentFragment>
 `;

From bec2645d05fda676b2e3f3954b01873a942ba4a0 Mon Sep 17 00:00:00 2001
From: Robin <robin@robin.town>
Date: Thu, 14 Aug 2025 15:06:26 +0200
Subject: [PATCH 2/2] Add menu role to menu containers in stories

---
 src/components/Menu/CheckboxMenuItem.stories.tsx | 2 +-
 src/components/Menu/ToggleMenuItem.stories.tsx   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/components/Menu/CheckboxMenuItem.stories.tsx b/src/components/Menu/CheckboxMenuItem.stories.tsx
index 9cd715f1..205295c6 100644
--- a/src/components/Menu/CheckboxMenuItem.stories.tsx
+++ b/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"
diff --git a/src/components/Menu/ToggleMenuItem.stories.tsx b/src/components/Menu/ToggleMenuItem.stories.tsx
index 66db5793..06546cf9 100644
--- a/src/components/Menu/ToggleMenuItem.stories.tsx
+++ b/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}