Fix submenu checkboxes (#2116)

* fix submenu checkboxes not being checkable

* add changelog entry

* prettier

* add changelog entry

---------

Co-authored-by: Cory LaViska <cory@abeautifulsite.net>

docs/pages/resources/changelog.md

@@ -18,6 +18,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti
 - Added the `base__popup` part to `<sl-dropdown>` [#2078]
 - `<sl-tab>` `closable` property now reflects. [#2041]
 - `<sl-tab-group>` now implements a proper "roving tabindex" and `<sl-tab>` is no longer tabbable by default. This aligns closer to the APG pattern for tabs. [#2041]
+- Fixed a bug in `<sl-menu>` that did not allow checkboxes to be checked that were in submenus. [#2116]
 - Fixed a bug in `<sl-details>` / `<sl-drawer>` that was accidentally detecting overflows and showing a scrollbar. [#2121]
 - Fixed a bug in the submenu controller that prevented submenus from rendering in RTL without explicitly setting `dir` on the parent menu item [#1992]
 - Fixed a bug where `<sl-relative-time>` would announce the full time instead of the relative time in screen readers
@@ -1762,4 +1763,4 @@ The following pages demonstrate why this change was necessary.
 
 ## 2.0.0-beta.1
 
-- Initial release
+- Initial release

src/components/menu/menu.component.ts

@@ -33,10 +33,17 @@ export default class SlMenu extends ShoelaceElement {
   private handleClick(event: MouseEvent) {
     const menuItemTypes = ['menuitem', 'menuitemcheckbox'];
 
-    const target = event.composedPath().find((el: Element) => menuItemTypes.includes(el?.getAttribute?.('role') || ''));
+    const composedPath = event.composedPath();
+    const target = composedPath.find((el: Element) => menuItemTypes.includes(el?.getAttribute?.('role') || ''));
 
     if (!target) return;
 
+    const closestMenu = composedPath.find((el: Element) => el?.getAttribute?.('role') === 'menu');
+    const clickHasSubmenu = closestMenu !== this;
+
+    // Make sure we're the menu thats supposed to be handling the click event.
+    if (clickHasSubmenu) return;
+
     // This isn't true. But we use it for TypeScript checks below.
     const item = target as SlMenuItem;
 

src/components/menu/menu.test.ts

@@ -6,6 +6,7 @@ import { sendKeys } from '@web/test-runner-commands';
 import sinon from 'sinon';
 import type { SlSelectEvent } from '../../events/sl-select.js';
 import type SlMenu from './menu.js';
+import type SlMenuItem from '../menu-item/menu-item.component.js';
 
 describe('<sl-menu>', () => {
   it('emits sl-select with the correct event detail when clicking an item', async () => {
@@ -100,24 +101,49 @@ describe('<sl-menu>', () => {
 
     expect(selectHandler).to.not.have.been.called;
   });
-});
 
-// @see https://github.com/shoelace-style/shoelace/issues/1596
-it('Should fire "sl-select" when clicking an element within a menu-item', async () => {
-  // eslint-disable-next-line
-  const selectHandler = sinon.spy(() => {});
+  // @see https://github.com/shoelace-style/shoelace/issues/1596
+  it('Should fire "sl-select" when clicking an element within a menu-item', async () => {
+    // eslint-disable-next-line
+    const selectHandler = sinon.spy(() => {});
+
+    const menu: SlMenu = await fixture(html`
+      <sl-menu>
+        <sl-menu-item>
+          <span>Menu item</span>
+        </sl-menu-item>
+      </sl-menu>
+    `);
+
+    menu.addEventListener('sl-select', selectHandler);
+    const span = menu.querySelector('span')!;
+    await clickOnElement(span);
+
+    expect(selectHandler).to.have.been.calledOnce;
+  });
 
-  const menu: SlMenu = await fixture(html`
-    <sl-menu>
-      <sl-menu-item>
-        <span>Menu item</span>
-      </sl-menu-item>
-    </sl-menu>
-  `);
+  // @see https://github.com/shoelace-style/shoelace/issues/2115
+  it('Should be able to check a checkbox menu item in a submenu', async () => {
+    const menu: SlMenu = await fixture(html`
+      <sl-menu style="max-width: 200px;">
+        <sl-menu-item>
+          <span>Menu item</span>
+          <sl-menu slot="submenu">
+            <sl-menu-item type="checkbox" checked>Checkbox</sl-menu-item>
+          </sl-menu>
+        </sl-menu-item>
+      </sl-menu>
+    `);
 
-  menu.addEventListener('sl-select', selectHandler);
-  const span = menu.querySelector('span')!;
-  await clickOnElement(span);
+    const menuItem = menu.querySelector<SlMenuItem>('sl-menu-item')!;
+    const checkbox = menu.querySelector<SlMenuItem>("[type='checkbox']")!;
 
-  expect(selectHandler).to.have.been.calledOnce;
+    expect(checkbox.checked).to.equal(true);
+    await clickOnElement(menuItem); // Focus the menu item
+    await sendKeys({ press: 'ArrowRight' }); // Open the submenu
+    await clickOnElement(checkbox); // Click the checkbox
+    await checkbox.updateComplete;
+    expect(checkbox.checked).to.equal(false);
+  });
 });
+