Fix the keyboard shortcut being read as part of the menu item contents

.changeset/cyan-cities-rhyme.md

@@ -0,0 +1,8 @@
+---
+'@sl-design-system/shared': patch
+'@sl-design-system/menu': patch
+---
+
+Fix the keyboard shortcut being part of the menu item contents
+
+Use `aria-keyshortcuts` to expose the shortcut to assistive technologies

packages/components/menu/src/menu-item.scss

@@ -97,7 +97,7 @@ sl-icon[name='check'] {
 
 kbd {
   font-family: var(--sl-text-typeset-font-family-body);
-  letter-spacing: 2px;
+  letter-spacing: var(--sl-size-025);
   margin-inline-start: auto;
   padding-inline-start: var(--sl-size-100);
 }

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

@@ -148,6 +148,17 @@ describe('sl-menu-item', () => {
       expect(el.shortcut).to.equal('$mod+Digit1');
     });
 
+    it('should have an aria-keyshortcuts attribute', () => {
+      expect(el).to.have.attribute('aria-keyshortcuts', navigator.platform.indexOf('Mac') > -1 ? 'Meta+1' : 'Ctrl+1');
+    });
+
+    it('should hide the kbd element from assistive technologies', () => {
+      const kbd = el.renderRoot.querySelector('kbd');
+
+      expect(kbd).to.exist;
+      expect(kbd).to.have.attribute('aria-hidden', 'true');
+    });
+
     it('should render the shortcut', () => {
       // Take into account that these tests are also running on Linux
       const text = navigator.platform.indexOf('Mac') > -1 ? '⌘1' : 'Ctrl1';

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

@@ -98,8 +98,10 @@ export class MenuItem extends ScopedElementsMixin(LitElement) {
 
     if (changes.has('shortcut')) {
       if (this.shortcut) {
+        this.setAttribute('aria-keyshortcuts', this.#shortcut.renderAsText(this.shortcut));
         this.#shortcut.bind({ [this.shortcut]: this.#onShortcut.bind(this) });
       } else {
+        this.removeAttribute('aria-keyshortcuts');
         this.#shortcut.unbind();
       }
     }
@@ -108,6 +110,7 @@ export class MenuItem extends ScopedElementsMixin(LitElement) {
       const selectMode = this.parentElement?.matches('[selects="single"]') ? 'menuitemradio' : 'menuitemcheckbox';
       this.role = this.selectable ? selectMode : 'menuitem';
     }
+
     if (changes.has('selected')) {
       this.setAttribute('aria-checked', (this.selected || false).toString());
     }
@@ -120,7 +123,9 @@ export class MenuItem extends ScopedElementsMixin(LitElement) {
         <div part="wrapper">
           ${this.selected ? html`<sl-icon name="check"></sl-icon>` : nothing}
           <slot></slot>
-          ${this.shortcut ? html`<kbd>${this.#shortcut.render(this.shortcut)}</kbd>` : nothing}
+          ${this.shortcut
+            ? html`<kbd aria-hidden="true">${this.#shortcut.renderAsLabel(this.shortcut)}</kbd>`
+            : nothing}
           ${this.submenu ? html`<sl-icon name="chevron-right"></sl-icon>` : nothing}
         </div>
       </div>

packages/components/shared/src/controllers/shortcut.ts

@@ -41,7 +41,7 @@ export class ShortcutController implements ReactiveController {
     this.#unregister = tinykeys(this.#target, shortcuts);
   }
 
-  render(shortcut: string): string {
+  renderAsLabel(shortcut: string): string {
     const keys = shortcut.replace(KEY_REGEX, '$1').split('+');
 
     return keys
@@ -54,4 +54,18 @@ export class ShortcutController implements ReactiveController {
       })
       .join('');
   }
+
+  renderAsText(shortcut: string): string {
+    const keys = shortcut.replace(KEY_REGEX, '$1').split('+');
+
+    return keys
+      .map(key => {
+        if (key === '$mod') {
+          return isMac() ? 'Meta' : 'Ctrl';
+        } else {
+          return key;
+        }
+      })
+      .join('+');
+  }
 }