fixes #1107

docs/resources/changelog.md

@@ -10,6 +10,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 
 ## Next
 
+- Added support for the `inert` attribute on `<sl-menu-item>` to allow hidden menu items to not accept focus [#1107](https://github.com/shoelace-style/shoelace/issues/1107)
 - Fixed a bug in `<sl-select>` that prevented placeholders from showing when `multiple` was used [#1109](https://github.com/shoelace-style/shoelace/issues/1109)
 - Fixed a bug in `<sl-color-picker>` that logged a console error when parsing swatches with whitespace
 - Fixed a bug in `<sl-color-picker>` that caused selected colors to be wrong due to incorrect HSV calculations

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

@@ -8,6 +8,10 @@ export default css`
     display: block;
   }
 
+  :host([inert]) {
+    display: none;
+  }
+
   .menu-item {
     position: relative;
     display: flex;

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

@@ -3,7 +3,7 @@ import sinon from 'sinon';
 import type SlMenuItem from './menu-item';
 
 describe('<sl-menu-item>', () => {
-  it('passes accessibility test', async () => {
+  it('should pass accessibility tests', async () => {
     const el = await fixture<SlMenuItem>(html`
       <sl-menu>
         <sl-menu-item>Item 1</sl-menu-item>
@@ -17,7 +17,7 @@ describe('<sl-menu-item>', () => {
     await expect(el).to.be.accessible();
   });
 
-  it('default properties', async () => {
+  it('should have the correct default properties', async () => {
     const el = await fixture<SlMenuItem>(html` <sl-menu-item>Test</sl-menu-item> `);
 
     expect(el.value).to.equal('');
@@ -25,7 +25,7 @@ describe('<sl-menu-item>', () => {
     expect(el.getAttribute('aria-disabled')).to.equal('false');
   });
 
-  it('changes aria attributes', async () => {
+  it('should render the correct aria attributes when disabled', async () => {
     const el = await fixture<SlMenuItem>(html` <sl-menu-item>Test</sl-menu-item> `);
 
     el.disabled = true;
@@ -33,12 +33,12 @@ describe('<sl-menu-item>', () => {
     expect(el.getAttribute('aria-disabled')).to.equal('true');
   });
 
-  it('get text label', async () => {
+  it('should return a text label when calling getTextLabel()', async () => {
     const el = await fixture<SlMenuItem>(html` <sl-menu-item>Test</sl-menu-item> `);
     expect(el.getTextLabel()).to.equal('Test');
   });
 
-  it('emits the slotchange event when the label changes', async () => {
+  it('should emit the slotchange event when the label changes', async () => {
     const el = await fixture<SlMenuItem>(html` <sl-menu-item>Text</sl-menu-item> `);
     const slotChangeHandler = sinon.spy();
 
@@ -48,4 +48,17 @@ describe('<sl-menu-item>', () => {
 
     expect(slotChangeHandler).to.have.been.calledOnce;
   });
+
+  it('should render a hidden menu item when the inert attribute is used', async () => {
+    const menu = await fixture<SlMenuItem>(html`
+      <sl-menu>
+        <sl-menu-item inert>Item 1</sl-menu-item>
+        <sl-menu-item>Item 2</sl-menu-item>
+        <sl-menu-item>Item 3</sl-menu-item>
+      </sl-menu>
+    `);
+    const item1 = menu.querySelector('sl-menu-item')!;
+
+    expect(getComputedStyle(item1).display).to.equal('none');
+  });
 });

src/components/menu/menu.test.ts

@@ -1,78 +1,102 @@
-import { expect, fixture, waitUntil } from '@open-wc/testing';
+import { expect, fixture } from '@open-wc/testing';
 import { sendKeys } from '@web/test-runner-commands';
 import { html } from 'lit';
 import sinon from 'sinon';
+import { clickOnElement } from '../../internal/test';
 import type SlMenuItem from '../menu-item/menu-item';
 import type SlMenu from './menu';
 
-interface Payload {
-  item: SlMenuItem;
-}
-
-const createTestMenu = (): Promise<SlMenu> => {
-  return fixture<SlMenu>(html`
-    <sl-menu>
-      <sl-menu-item value="test1">test1</sl-menu-item>
-      <sl-menu-item value="test2">test2</sl-menu-item>
-      <sl-menu-item value="test3">test3</sl-menu-item>
-      <sl-menu-item value="testDisabled" disabled>testDisabled</sl-menu-item>
-    </sl-menu>
-  `);
-};
-
-const clickOnItemWithValue = (menu: SlMenu, value: string) => {
-  const clickedItem = menu.querySelector(`[value=${value}]`);
-  if (clickedItem) {
-    (clickedItem as SlMenuItem).click();
-  }
-};
-
-const spyOnSelectHandler = (menu: SlMenu): sinon.SinonSpy => {
-  const selectHandler = sinon.spy();
-  menu.addEventListener('sl-select', selectHandler);
-  return selectHandler;
-};
-
-const expectSelectHandlerToHaveBeenCalledOn = async (
-  selectHandler: sinon.SinonSpy,
-  expectedValue: string
-): Promise<void> => {
-  await waitUntil(() => selectHandler.called);
-  expect(selectHandler).to.have.been.calledOnce;
-  const event = selectHandler.args[0][0] as CustomEvent;
-  const detail = event.detail as Payload;
-  expect(detail.item.value).to.equal(expectedValue);
-};
-
 describe('<sl-menu>', () => {
-  it('emits sl-select on click of an item returning the selected item as payload', async () => {
-    const menu = await createTestMenu();
-    const selectHandler = spyOnSelectHandler(menu);
+  it('emits sl-select with the correct event detail when clicking an item', async () => {
+    const menu = await fixture<SlMenu>(html`
+      <sl-menu>
+        <sl-menu-item value="item-1">Item 1</sl-menu-item>
+        <sl-menu-item value="item-2">Item 2</sl-menu-item>
+        <sl-menu-item value="item-3">Item 3</sl-menu-item>
+        <sl-menu-item value="item-4">Item 4</sl-menu-item>
+      </sl-menu>
+    `);
+    const item2 = menu.querySelectorAll('sl-menu-item')[1];
+    const selectHandler = sinon.spy((event: CustomEvent) => {
+      const item = event.detail.item as SlMenuItem; // eslint-disable-line
+      if (item !== item2) {
+        expect.fail('Incorrect event detail emitted with sl-select');
+      }
+    });
 
-    clickOnItemWithValue(menu, 'test1');
+    menu.addEventListener('sl-select', selectHandler);
+    await clickOnElement(item2);
 
-    await expectSelectHandlerToHaveBeenCalledOn(selectHandler, 'test1');
+    expect(selectHandler).to.have.been.calledOnce;
   });
 
   it('can be selected via keyboard', async () => {
-    const menu = await createTestMenu();
-    const selectHandler = spyOnSelectHandler(menu);
+    const menu = await fixture<SlMenu>(html`
+      <sl-menu>
+        <sl-menu-item value="item-1">Item 1</sl-menu-item>
+        <sl-menu-item value="item-2">Item 2</sl-menu-item>
+        <sl-menu-item value="item-3">Item 3</sl-menu-item>
+        <sl-menu-item value="item-4">Item 4</sl-menu-item>
+      </sl-menu>
+    `);
+    const [item1, item2] = menu.querySelectorAll('sl-menu-item');
+    const selectHandler = sinon.spy((event: CustomEvent) => {
+      const item = event.detail.item as SlMenuItem; // eslint-disable-line
+      if (item !== item2) {
+        expect.fail('Incorrect item selected');
+      }
+    });
 
-    await sendKeys({ press: 'Tab' });
+    menu.addEventListener('sl-select', selectHandler);
+
+    item1.focus();
+    await item1.updateComplete;
     await sendKeys({ press: 'ArrowDown' });
     await sendKeys({ press: 'Enter' });
 
-    await expectSelectHandlerToHaveBeenCalledOn(selectHandler, 'test2');
+    expect(selectHandler).to.have.been.calledOnce;
   });
 
-  it('does not select disabled items', async () => {
-    const menu = await createTestMenu();
-    const selectHandler = spyOnSelectHandler(menu);
+  it('does not select disabled items when clicking', async () => {
+    const menu = await fixture<SlMenu>(html`
+      <sl-menu>
+        <sl-menu-item value="item-1">Item 1</sl-menu-item>
+        <sl-menu-item value="item-2" disabled>Item 2</sl-menu-item>
+        <sl-menu-item value="item-3">Item 3</sl-menu-item>
+        <sl-menu-item value="item-4">Item 4</sl-menu-item>
+      </sl-menu>
+    `);
+    const item2 = menu.querySelectorAll('sl-menu-item')[1];
+    const selectHandler = sinon.spy();
 
-    await sendKeys({ press: 'Tab' });
-    await sendKeys({ type: 'testDisabled' });
+    menu.addEventListener('sl-select', selectHandler);
+
+    await clickOnElement(item2);
+
+    expect(selectHandler).to.not.have.been.calledOnce;
+  });
+
+  it('does not select disabled items when pressing enter', async () => {
+    const menu = await fixture<SlMenu>(html`
+      <sl-menu>
+        <sl-menu-item value="item-1">Item 1</sl-menu-item>
+        <sl-menu-item value="item-2" disabled>Item 2</sl-menu-item>
+        <sl-menu-item value="item-3">Item 3</sl-menu-item>
+        <sl-menu-item value="item-4">Item 4</sl-menu-item>
+      </sl-menu>
+    `);
+    const [item1, item2] = menu.querySelectorAll('sl-menu-item');
+    const selectHandler = sinon.spy();
+
+    menu.addEventListener('sl-select', selectHandler);
+
+    item1.focus();
+    await item1.updateComplete;
+    await sendKeys({ press: 'ArrowDown' });
+    expect(document.activeElement).to.equal(item2);
     await sendKeys({ press: 'Enter' });
+    await item2.updateComplete;
 
-    await expectSelectHandlerToHaveBeenCalledOn(selectHandler, 'test1');
+    expect(selectHandler).to.not.have.been.called;
   });
 });

src/components/menu/menu.ts

@@ -31,7 +31,7 @@ export default class SlMenu extends ShoelaceElement {
 
   private getAllItems() {
     return [...this.defaultSlot.assignedElements({ flatten: true })].filter((el: HTMLElement) => {
-      if (!this.isMenuItem(el)) {
+      if (el.inert || !this.isMenuItem(el)) {
         return false;
       }
 
@@ -43,13 +43,15 @@ export default class SlMenu extends ShoelaceElement {
     const target = event.target as HTMLElement;
     const item = target.closest('sl-menu-item');
 
-    if (item?.disabled === false) {
-      if (item.type === 'checkbox') {
-        item.checked = !item.checked;
-      }
-
-      this.emit('sl-select', { detail: { item } });
+    if (!item || item.disabled || item.inert) {
+      return;
     }
+
+    if (item.type === 'checkbox') {
+      item.checked = !item.checked;
+    }
+
+    this.emit('sl-select', { detail: { item } });
   }
 
   private handleKeyDown(event: KeyboardEvent) {