diff --git a/docs/getting-started/changelog.md b/docs/getting-started/changelog.md index 0c5d17c6a..3f4f7b55c 100644 --- a/docs/getting-started/changelog.md +++ b/docs/getting-started/changelog.md @@ -12,6 +12,10 @@ _During the beta period, these restrictions may be relaxed in the event of a mis - Fixed a bug where hoisted dropdowns would render in the wrong position when place inside an `sl-dialog` - Improved `sl-dropdown` accessibility by attaching `aria-haspopup` and `aria-expanded` to the slotted trigger - Removed `console.log` from modal utility +- 🚨 BREAKING CHANGE: Refactored `sl-menu` and `sl-menu-item` to improve accessibility by using proper focus states + - Moved `tabindex` from `sl-menu` to `sl-menu-item` + - Removed the `active` prop from `sl-menu-item` because synthetic focus states are bad for accessibility + - Removed the `sl-activate` and `sl-deactivate` events from `sl-menu-item` (listen for `focus` and `blur` instead) ## 2.0.0-beta.21 diff --git a/src/components.d.ts b/src/components.d.ts index 3d2f418b3..a8b790db5 100644 --- a/src/components.d.ts +++ b/src/components.d.ts @@ -686,10 +686,6 @@ export namespace Components { interface SlMenuDivider { } interface SlMenuItem { - /** - * Draws the menu in an active (i.e. or hover/focus), state to indicate the current menu selection. This is used in lieu of standard :hover and :focus states to prevent concurrent interactions from different devices, such as focusing with the keyboard and hovering with the mouse. - */ - "active": boolean; /** * Set to true to draw the item in a checked state. */ @@ -698,6 +694,14 @@ export namespace Components { * Set to true to draw the menu item in a disabled state. */ "disabled": boolean; + /** + * Removes focus from the button. + */ + "removeFocus": () => Promise; + /** + * Sets focus on the button. + */ + "setFocus": () => Promise; /** * A unique value to store in the menu item. */ @@ -2195,10 +2199,6 @@ declare namespace LocalJSX { interface SlMenuDivider { } interface SlMenuItem { - /** - * Draws the menu in an active (i.e. or hover/focus), state to indicate the current menu selection. This is used in lieu of standard :hover and :focus states to prevent concurrent interactions from different devices, such as focusing with the keyboard and hovering with the mouse. - */ - "active"?: boolean; /** * Set to true to draw the item in a checked state. */ @@ -2207,14 +2207,6 @@ declare namespace LocalJSX { * Set to true to draw the menu item in a disabled state. */ "disabled"?: boolean; - /** - * Emitted when the menu item becomes active. - */ - "onSl-activate"?: (event: CustomEvent) => void; - /** - * Emitted when the menu item becomes inactive. - */ - "onSl-deactivate"?: (event: CustomEvent) => void; /** * A unique value to store in the menu item. */ diff --git a/src/components/menu-item/menu-item.scss b/src/components/menu-item/menu-item.scss index e339fdcc5..e5d344b49 100644 --- a/src/components/menu-item/menu-item.scss +++ b/src/components/menu-item/menu-item.scss @@ -21,12 +21,14 @@ white-space: nowrap; cursor: pointer; - &.menu-item--active:not(.menu-item--disabled) { + &.menu-item--focused:not(.menu-item--disabled) { + outline: none; background-color: var(--sl-color-primary-95); color: var(--sl-color-primary-50); } &.menu-item--disabled { + outline: none; color: var(--sl-color-gray-70); cursor: not-allowed; } diff --git a/src/components/menu-item/menu-item.tsx b/src/components/menu-item/menu-item.tsx index a034adf9d..5db28a0cc 100644 --- a/src/components/menu-item/menu-item.tsx +++ b/src/components/menu-item/menu-item.tsx @@ -1,4 +1,4 @@ -import { Component, Event, EventEmitter, Prop, Watch, h } from '@stencil/core'; +import { Component, Method, Prop, State, h } from '@stencil/core'; /** * @since 2.0 @@ -21,46 +21,73 @@ import { Component, Event, EventEmitter, Prop, Watch, h } from '@stencil/core'; shadow: true }) export class MenuItem { + menuItem: HTMLElement; + + @State() hasFocus = false; + /** Set to true to draw the item in a checked state. */ @Prop({ reflect: true }) checked = false; - /** - * Draws the menu in an active (i.e. or hover/focus), state to indicate the current menu selection. This is used in - * lieu of standard :hover and :focus states to prevent concurrent interactions from different devices, such as - * focusing with the keyboard and hovering with the mouse. - */ - @Prop({ reflect: true }) active = false; - /** A unique value to store in the menu item. */ @Prop({ reflect: true }) value = ''; /** Set to true to draw the menu item in a disabled state. */ @Prop({ reflect: true }) disabled = false; - @Watch('active') - handleActiveChange() { - this.active ? this.slActivate.emit() : this.slDeactivate.emit(); + connectedCallback() { + this.handleBlur = this.handleBlur.bind(this); + this.handleFocus = this.handleFocus.bind(this); + this.handleMouseOver = this.handleMouseOver.bind(this); + this.handleMouseOut = this.handleMouseOut.bind(this); } - /** Emitted when the menu item becomes active. */ - @Event({ eventName: 'sl-activate' }) slActivate: EventEmitter; + /** Sets focus on the button. */ + @Method() + async setFocus() { + this.menuItem.focus(); + } - /** Emitted when the menu item becomes inactive. */ - @Event({ eventName: 'sl-deactivate' }) slDeactivate: EventEmitter; + /** Removes focus from the button. */ + @Method() + async removeFocus() { + this.menuItem.blur(); + } + + handleBlur() { + this.hasFocus = false; + } + + handleFocus() { + this.hasFocus = true; + } + + handleMouseOver() { + this.setFocus(); + } + + handleMouseOut() { + this.removeFocus(); + } render() { return (
(this.menuItem = el)} part="base" class={{ 'menu-item': true, 'menu-item--checked': this.checked, - 'menu-item--active': this.active, - 'menu-item--disabled': this.disabled + 'menu-item--disabled': this.disabled, + 'menu-item--focused': this.hasFocus }} role="menuitem" aria-disabled={this.disabled} aria-selected={this.checked} + tabIndex={!this.disabled ? 0 : null} + onFocus={this.handleFocus} + onBlur={this.handleBlur} + onMouseOver={this.handleMouseOver} + onMouseOut={this.handleMouseOut} > diff --git a/src/components/menu/menu.tsx b/src/components/menu/menu.tsx index a160575e0..7ed7f20fb 100644 --- a/src/components/menu/menu.tsx +++ b/src/components/menu/menu.tsx @@ -1,4 +1,4 @@ -import { Component, Event, EventEmitter, Method, State, h } from '@stencil/core'; +import { Component, Element, Event, EventEmitter, Method, State, h } from '@stencil/core'; import { getTextContent } from '../../utilities/slot'; /** @@ -20,6 +20,8 @@ export class Menu { typeToSelectString = ''; typeToSelectTimeout: any; + @Element() host: HTMLSlMenuElement; + @State() hasFocus = false; /** Emitted when the menu gains focus. */ @@ -32,12 +34,8 @@ export class Menu { @Event({ eventName: 'sl-select' }) slSelect: EventEmitter<{ item: HTMLSlMenuItemElement }>; connectedCallback() { - this.handleBlur = this.handleBlur.bind(this); this.handleClick = this.handleClick.bind(this); - this.handleFocus = this.handleFocus.bind(this); this.handleKeyDown = this.handleKeyDown.bind(this); - this.handleMouseOver = this.handleMouseOver.bind(this); - this.handleMouseOut = this.handleMouseOut.bind(this); } /** Sets focus on the menu. */ @@ -65,13 +63,12 @@ export class Menu { clearTimeout(this.typeToSelectTimeout); this.typeToSelectTimeout = setTimeout(() => (this.typeToSelectString = ''), 750); this.typeToSelectString += key.toLowerCase(); - const items = this.getItems(); for (const item of items) { const slot = item.shadowRoot.querySelector('slot:not([name])') as HTMLSlotElement; const label = getTextContent(slot).toLowerCase().trim(); if (label.substring(0, this.typeToSelectString.length) === this.typeToSelectString) { - items.map(i => (i.active = i === item)); + item.setFocus(); break; } } @@ -85,27 +82,11 @@ export class Menu { } getActiveItem() { - return this.getItems().find(i => i.active); + return this.getItems().find(i => i === document.activeElement); } - setActiveItem(item?: HTMLSlMenuItemElement) { - this.getItems().map(i => (i.active = i === item)); - } - - handleFocus() { - this.slFocus.emit(); - - // Activate the first item if no other item is active - const activeItem = this.getActiveItem(); - if (!activeItem) { - const items = this.getItems(); - this.setActiveItem(items[0]); - } - } - - handleBlur() { - this.setActiveItem(); - this.slBlur.emit(); + setActiveItem(item: HTMLSlMenuItemElement) { + item.setFocus(); } handleClick(event: MouseEvent) { @@ -164,17 +145,6 @@ export class Menu { this.typeToSelect(event.key); } - handleMouseOver(event: MouseEvent) { - const target = event.target as HTMLElement; - const item = target.closest('sl-menu-item'); - - this.setActiveItem(item); - } - - handleMouseOut() { - this.setActiveItem(null); - } - render() { return (