focus on disabled menu items and fix aria-checked

docs/resources/changelog.md

@@ -24,6 +24,7 @@ This release includes a complete rewrite of `<sl-select>` to improve accessibili
 - 🚨 BREAKING: improved the accessibility of `<sl-menu-item>` so checked items are announced as such
   - Checkbox menu items must now have `type="checkbox"` before applying the `checked` attribute
   - Checkbox menu items will now toggle their `checked` state on their own when selected
+  - Disabled menu items will now receive focus, but are still not selectable
 - Added the `<sl-option>` component
 - Added Traditional Chinese translation [#1086](https://github.com/shoelace-style/shoelace/pull/1086)
 - Added support for `swatches` to be an attribute of `<sl-color-picker>` so swatches can be defined declaratively (it was previously a property; use a `;` to separate color values)

src/components/menu-item/menu-item.styles.ts

@@ -65,10 +65,11 @@ export default css`
     color: var(--sl-color-neutral-1000);
   }
 
-  :host(:focus-visible:not([aria-disabled='true'])) .menu-item {
+  :host(:focus-visible) .menu-item {
     outline: none;
     background-color: var(--sl-color-primary-600);
     color: var(--sl-color-neutral-0);
+    opacity: 1;
   }
 
   .menu-item .menu-item__check,
@@ -88,7 +89,7 @@ export default css`
 
   @media (forced-colors: active) {
     :host(:hover:not([aria-disabled='true'])) .menu-item,
-    :host(:focus-visible:not([aria-disabled='true'])) .menu-item {
+    :host(:focus-visible) .menu-item {
       outline: dashed 1px SelectedItem;
       outline-offset: -1px;
     }

src/components/menu-item/menu-item.test.ts

@@ -9,6 +9,9 @@ describe('<sl-menu-item>', () => {
         <sl-menu-item>Item 1</sl-menu-item>
         <sl-menu-item>Item 2</sl-menu-item>
         <sl-menu-item>Item 3</sl-menu-item>
+        <sl-divider></sl-divider>
+        <sl-menu-item type="checkbox" checked>Checked</sl-menu-item>
+        <sl-menu-item type="checkbox">Unchecked</sl-menu-item>
       </sl-menu>
     `);
     await expect(el).to.be.accessible();

src/components/menu-item/menu-item.ts

@@ -65,11 +65,18 @@ export default class SlMenuItem extends ShoelaceElement {
 
   @watch('checked')
   handleCheckedChange() {
-    this.setAttribute('aria-checked', this.checked ? 'true' : 'false');
-
+    // For proper accessibility, users have to use type="checkbox" to use the checked attribute
     if (this.checked && this.type !== 'checkbox') {
       this.checked = false;
       console.error('The checked attribute can only be used on menu items with type="checkbox"', this);
+      return;
+    }
+
+    // Only checkbox types can receive the aria-checked attribute
+    if (this.type === 'checkbox') {
+      this.setAttribute('aria-checked', this.checked ? 'true' : 'false');
+    } else {
+      this.removeAttribute('aria-checked');
     }
   }
 
@@ -80,7 +87,13 @@ export default class SlMenuItem extends ShoelaceElement {
 
   @watch('type')
   handleTypeChange() {
-    this.setAttribute('role', this.type === 'checkbox' ? 'menuitemcheckbox' : 'menuitem');
+    if (this.type === 'checkbox') {
+      this.setAttribute('role', 'menuitemcheckbox');
+      this.setAttribute('aria-checked', this.checked ? 'true' : 'false');
+    } else {
+      this.setAttribute('role', 'menuitem');
+      this.removeAttribute('aria-checked');
+    }
   }
 
   /** Returns a text label based on the contents of the menu item's default slot. */

src/components/menu/menu.ts

@@ -29,16 +29,12 @@ export default class SlMenu extends ShoelaceElement {
     this.setAttribute('role', 'menu');
   }
 
-  private getAllItems(options: { includeDisabled: boolean } = { includeDisabled: true }) {
+  private getAllItems() {
     return [...this.defaultSlot.assignedElements({ flatten: true })].filter((el: HTMLElement) => {
       if (!this.isMenuItem(el)) {
         return false;
       }
 
-      if (!options.includeDisabled && (el as SlMenuItem).disabled) {
-        return false;
-      }
-
       return true;
     }) as SlMenuItem[];
   }
@@ -73,7 +69,7 @@ export default class SlMenu extends ShoelaceElement {
 
     // Move the selection when pressing down or up
     if (['ArrowDown', 'ArrowUp', 'Home', 'End'].includes(event.key)) {
-      const items = this.getAllItems({ includeDisabled: false });
+      const items = this.getAllItems();
       const activeItem = this.getCurrentItem();
       let index = activeItem ? items.indexOf(activeItem) : 0;
 
@@ -112,7 +108,7 @@ export default class SlMenu extends ShoelaceElement {
   }
 
   private handleSlotChange() {
-    const items = this.getAllItems({ includeDisabled: false });
+    const items = this.getAllItems();
 
     // Reset the roving tab index when the slotted items change
     if (items.length > 0) {
@@ -132,7 +128,7 @@ export default class SlMenu extends ShoelaceElement {
    * The menu item may or may not have focus, but for keyboard interaction purposes it's considered the "active" item.
    */
   getCurrentItem() {
-    return this.getAllItems({ includeDisabled: false }).find(i => i.getAttribute('tabindex') === '0');
+    return this.getAllItems().find(i => i.getAttribute('tabindex') === '0');
   }
 
   /**
@@ -140,12 +136,11 @@ export default class SlMenu extends ShoelaceElement {
    * `tabindex="-1"` to all other items. This method must be called prior to setting focus on a menu item.
    */
   setCurrentItem(item: SlMenuItem) {
-    const items = this.getAllItems({ includeDisabled: false });
-    const activeItem = item.disabled ? items[0] : item;
+    const items = this.getAllItems();
 
     // Update tab indexes
     items.forEach(i => {
-      i.setAttribute('tabindex', i === activeItem ? '0' : '-1');
+      i.setAttribute('tabindex', i === item ? '0' : '-1');
     });
   }