diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index a1a50c77..2af7e50c 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -18,6 +18,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti - Fixed a bug [in the localize dependency](https://github.com/shoelace-style/localize/issues/20) that caused underscores in language codes to throw a `RangeError` - Fixed a bug in the focus trapping utility used by modals that caused unexpected focus behavior. [#1583] - Fixed a bug in `` that prevented exported tooltip parts from being styled [#1586] +- Fixed a bug in `` that caused it not to fire the `sl-select` event if you clicked an element inside of a `` [#1599] - Improved submenu selection by implementing the [safe triangle](https://www.smashingmagazine.com/2023/08/better-context-menus-safe-triangles/) method [#1550] - Updated `@shoelace-style/localize` to 3.1.0 diff --git a/src/components/menu/menu.component.ts b/src/components/menu/menu.component.ts index 6297a027..879d033d 100644 --- a/src/components/menu/menu.component.ts +++ b/src/components/menu/menu.component.ts @@ -1,9 +1,9 @@ import { html } from 'lit'; import { query } from 'lit/decorators.js'; import ShoelaceElement from '../../internal/shoelace-element.js'; -import SlMenuItem from '../menu-item/menu-item.component.js'; import styles from './menu.styles.js'; import type { CSSResultGroup } from 'lit'; +import type SlMenuItem from '../menu-item/menu-item.component.js'; export interface MenuSelectEventDetail { item: SlMenuItem; } @@ -29,11 +29,14 @@ export default class SlMenu extends ShoelaceElement { } private handleClick(event: MouseEvent) { - if (!(event.target instanceof SlMenuItem)) { - return; - } + const menuItemTypes = ['menuitem', 'menuitemcheckbox']; - const item: SlMenuItem = event.target; + const target = event.composedPath().find((el: Element) => menuItemTypes.includes(el?.getAttribute?.('role') || '')); + + if (!target) return; + + // This isn't true. But we use it for TypeScript checks below. + const item = target as SlMenuItem; if (item.type === 'checkbox') { item.checked = !item.checked; diff --git a/src/components/menu/menu.test.ts b/src/components/menu/menu.test.ts index 50bcf515..6620c881 100644 --- a/src/components/menu/menu.test.ts +++ b/src/components/menu/menu.test.ts @@ -101,3 +101,23 @@ describe('', () => { 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(() => {}); + + const menu: SlMenu = await fixture(html` + + + Menu item + + + `); + + menu.addEventListener('sl-select', selectHandler); + const span = menu.querySelector('span')!; + await clickOnElement(span); + + expect(selectHandler).to.have.been.calledOnce; +});