diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index 1884c143..1393a6a4 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -24,6 +24,7 @@ This release includes a complete rewrite of `` to improve accessibili - 🚨 BREAKING: improved the accessibility of `` 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 `` component - Added Traditional Chinese translation [#1086](https://github.com/shoelace-style/shoelace/pull/1086) - Added support for `swatches` to be an attribute of `` so swatches can be defined declaratively (it was previously a property; use a `;` to separate color values) diff --git a/src/components/menu-item/menu-item.styles.ts b/src/components/menu-item/menu-item.styles.ts index 8b7794b6..e7572371 100644 --- a/src/components/menu-item/menu-item.styles.ts +++ b/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; } diff --git a/src/components/menu-item/menu-item.test.ts b/src/components/menu-item/menu-item.test.ts index d7517b26..7d45ef86 100644 --- a/src/components/menu-item/menu-item.test.ts +++ b/src/components/menu-item/menu-item.test.ts @@ -9,6 +9,9 @@ describe('', () => { Item 1 Item 2 Item 3 + + Checked + Unchecked `); await expect(el).to.be.accessible(); diff --git a/src/components/menu-item/menu-item.ts b/src/components/menu-item/menu-item.ts index 9b28ceb3..d940b06b 100644 --- a/src/components/menu-item/menu-item.ts +++ b/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. */ diff --git a/src/components/menu/menu.ts b/src/components/menu/menu.ts index 8450a5d4..363cd399 100644 --- a/src/components/menu/menu.ts +++ b/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'); }); }