Merge pull request #178 from element-hq/t3chguy/menuitem-roles

element-hq · Jun 10, 2024 · dea0cfe · dea0cfe
2 parents 5282d35 + 5a34068
commit dea0cfe
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 8 deletions.
diff --git a/src/components/Menu/MenuItem.tsx b/src/components/Menu/MenuItem.tsx
@@ -53,6 +53,10 @@ type Props<C extends MenuItemElement> = {
    */
   // This prop is required because it's rare to not want a label
   label: string | null;
+  /**
+   * Additional properties to pass to the Text label component.
+   */
+  labelProps?: ComponentPropsWithoutRef<typeof Text>;
   /**
    * Event callback for when the item is selected via mouse, touch, or keyboard.
    * Calling event.preventDefault in this handler will prevent the menu from
@@ -71,12 +75,14 @@ type Props<C extends MenuItemElement> = {
 /**
  * An item within a menu, acting either as a navigation button, or simply a
  * container for other interactive elements.
+ * Must be used within a compound Menu or other `menu` or `menubar` aria role subtree.
  */
 export const MenuItem = <C extends MenuItemElement = "button">({
   as,
   className,
   Icon,
   label,
+  labelProps,
   onSelect,
   kind = "primary",
   children,
@@ -133,7 +139,13 @@ export const MenuItem = <C extends MenuItemElement = "button">({
       )}
 
       {label !== null && (
-        <Text className={styles.label} size="md" weight="medium" as="span">
+        <Text
+          className={styles.label}
+          size="md"
+          weight="medium"
+          as="span"
+          {...labelProps}
+        >
           {label}
         </Text>
       )}

diff --git a/src/components/Menu/ToggleMenuItem.tsx b/src/components/Menu/ToggleMenuItem.tsx
@@ -28,22 +28,28 @@ type Props = Pick<
 /**
  * A menu item with a toggle control. Clicking anywhere on the surface will
  * activate the toggle.
+ * Must be used within a compound Menu or other `menu` or `menubar` aria role subtree.
  */
 export const ToggleMenuItem = forwardRef<HTMLInputElement, Props>(
   function ToggleMenuItem(
     { className, Icon, label, onSelect, ...toggleProps },
     ref,
   ) {
     const toggleId = useId();
+    // We render the label ourselves as a `label` element cannot have role `menuitemcheckbox`
     return (
       <MenuItem
-        as="label"
-        htmlFor={toggleId}
+        as="div"
         role="menuitemcheckbox"
         className={className}
         Icon={Icon}
         label={label}
+        labelProps={{
+          as: "label",
+          htmlFor: toggleId,
+        }}
         onSelect={onSelect}
+        aria-checked={toggleProps.checked}
       >
         <ToggleInput id={toggleId} ref={ref} {...toggleProps} />
       </MenuItem>

diff --git a/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap b/src/components/Menu/__snapshots__/ToggleMenuItem.test.tsx.snap
@@ -2,10 +2,9 @@
 
 exports[`ToggleMenuItem > renders 1`] = `
 <DocumentFragment>
-  <label
+  <div
     class="_item_831119 _interactive_831119"
     data-kind="primary"
-    for=":r0:"
     role="menuitemcheckbox"
   >
     <svg
@@ -21,11 +20,12 @@ exports[`ToggleMenuItem > renders 1`] = `
         d="M12 16c1.25 0 2.313-.438 3.188-1.313.874-.874 1.312-1.937 1.312-3.187 0-1.25-.438-2.313-1.313-3.188C14.313 7.439 13.25 7 12 7c-1.25 0-2.312.438-3.187 1.313C7.938 9.187 7.5 10.25 7.5 11.5c0 1.25.438 2.313 1.313 3.188C9.688 15.562 10.75 16 12 16Zm0-1.8c-.75 0-1.387-.262-1.912-.787A2.604 2.604 0 0 1 9.3 11.5c0-.75.263-1.387.787-1.912A2.604 2.604 0 0 1 12 8.8c.75 0 1.387.262 1.912.787.525.526.788 1.163.788 1.913s-.262 1.387-.787 1.912A2.604 2.604 0 0 1 12 14.2Zm0 4.8c-2.317 0-4.433-.613-6.35-1.837-1.917-1.226-3.367-2.88-4.35-4.963a.812.812 0 0 1-.1-.313 2.93 2.93 0 0 1 0-.774.812.812 0 0 1 .1-.313c.983-2.083 2.433-3.738 4.35-4.963C7.567 4.614 9.683 4 12 4c2.317 0 4.433.612 6.35 1.838 1.917 1.224 3.367 2.879 4.35 4.962a.81.81 0 0 1 .1.313 2.925 2.925 0 0 1 0 .774.81.81 0 0 1-.1.313c-.983 2.083-2.433 3.738-4.35 4.963C16.433 18.387 14.317 19 12 19Zm0-2a9.544 9.544 0 0 0 5.188-1.488A9.773 9.773 0 0 0 20.8 11.5a9.773 9.773 0 0 0-3.613-4.013A9.544 9.544 0 0 0 12 6a9.545 9.545 0 0 0-5.187 1.487A9.773 9.773 0 0 0 3.2 11.5a9.773 9.773 0 0 0 3.613 4.012A9.544 9.544 0 0 0 12 17Z"
       />
     </svg>
-    <span
+    <label
       class="_typography_489030 _font-body-md-medium_489030 _label_831119"
+      for=":r0:"
     >
       Always show
-    </span>
+    </label>
     <div
       class="_container_adcb29"
     >
@@ -38,6 +38,6 @@ exports[`ToggleMenuItem > renders 1`] = `
         class="_ui_adcb29"
       />
     </div>
-  </label>
+  </div>
 </DocumentFragment>
 `;