diff --git a/libs/components/src/lib/menu-item/menu-item.ts b/libs/components/src/lib/menu-item/menu-item.ts index ad9b0c8950..20affb9ae3 100644 --- a/libs/components/src/lib/menu-item/menu-item.ts +++ b/libs/components/src/lib/menu-item/menu-item.ts @@ -355,9 +355,9 @@ export class MenuItem extends FoundationElement { case keyArrowLeft: //close submenu if (this.expanded) { + this.#emitSyntheticClick(); this.expanded = false; this.focus(); - this.#emitSyntheticClick(); return false; } } diff --git a/libs/components/src/lib/menu/README.md b/libs/components/src/lib/menu/README.md index 088bee57e5..644399f915 100644 --- a/libs/components/src/lib/menu/README.md +++ b/libs/components/src/lib/menu/README.md @@ -332,10 +332,10 @@ Use the `--menu-block-size` variable to set the menu's block size.
-| Name | Returns | Description | -| ---------------------- | ------- | ----------------------------------- | -| `focus` | `void` | Focuses the first item in the menu. | -| `collapseExpandedItem` | `void` | Collapses any expanded menu items. | +| Name | Returns | Description | +| ---------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------- | +| `focus` | `void` | Moves focus into the Menu. If there is a child with the `autofocus` attribute, it will be focused. Otherwise, the first Menu Item will be focused. | +| `collapseExpandedItem` | `void` | Collapses any expanded Menu Items. |
@@ -371,6 +371,10 @@ When the menu has focus: - `End` - Moves focus to the last menu item. - `Escape` - Closes the menu. +## Focus Management + +When the Menu opens or `.focus()` is called, focus moves to the first Menu Item by default. If there is a child with the `autofocus` attribute, it will be focused instead. + ## Use Cases ### Dropdown menu with checkbox @@ -387,6 +391,7 @@ When the menu has focus: slot="header" placeholder="Search" icon="search" + autofocus > diff --git a/libs/components/src/lib/menu/menu.spec.ts b/libs/components/src/lib/menu/menu.spec.ts index 37c580cd1d..a4108ba28d 100644 --- a/libs/components/src/lib/menu/menu.spec.ts +++ b/libs/components/src/lib/menu/menu.spec.ts @@ -68,6 +68,14 @@ describe('vwc-menu', () => { expect(popup.hasAttribute('open')).toBe(isOpen); } ); + + it('should not throw an error when setting open to true in unconnected state', async () => { + const menu = document.createElement(COMPONENT_TAG) as Menu; + + expect(() => { + menu.open = true; + }).not.toThrow(); + }); }); describe('trigger', () => { @@ -256,7 +264,18 @@ describe('vwc-menu', () => { return div; } - it('should focus the first menuitem in the menu', async () => { + it('should focus the first descendant with the autofocus attribute', async () => { + const input = document.createElement('input'); + input.setAttribute('autofocus', ''); + element.slot = 'header'; + element.appendChild(input); + + element.focus(); + + expect(document.activeElement).toEqual(input); + }); + + it('should focus the first menuitem in the menu if there is no descendant with autofocus attribute', async () => { const div = createMenuItem(); await elementUpdated(element); @@ -522,6 +541,18 @@ describe('vwc-menu', () => { expect(element.open).toEqual(true); }); + + it('should move focus into the menu when opened', async () => { + element.innerHTML = ` + + `; + await elementUpdated(element); + + element.open = true; + await elementUpdated(element); + + expect(document.activeElement).toEqual(element.querySelector('#id1')); + }); }); describe('menu header', () => { @@ -730,19 +761,6 @@ describe('vwc-menu', () => { expect(item1.expanded).toBe(false); }); - - it('should not collapse other submenus when expanded-change event is cancelled', async () => { - item1.expanded = true; - await elementUpdated(element); - item2.addEventListener('expanded-change', (e) => e.preventDefault(), { - capture: true, - }); - - item2.expanded = true; - await elementUpdated(element); - - expect(item1.expanded).toBe(true); - }); }); function focusOutOfBody() { diff --git a/libs/components/src/lib/menu/menu.template.ts b/libs/components/src/lib/menu/menu.template.ts index 3a07382048..e67b6cffff 100644 --- a/libs/components/src/lib/menu/menu.template.ts +++ b/libs/components/src/lib/menu/menu.template.ts @@ -1,6 +1,7 @@ import { type ElementViewTemplate, html, + ref, slotted, } from '@microsoft/fast-element'; import { classNames } from '@microsoft/fast-web-utilities'; @@ -56,6 +57,7 @@ export const MenuTemplate: ( @focusout="${(x, c) => x._onFocusout(c.event as FocusEvent)}"> ${anchorSlotTemplate} <${popupTag} + ${ref('_popupEl')} :placement=${(x) => x.placement} :open=${(x) => x.open} :anchor=${(x) => x._anchorEl} diff --git a/libs/components/src/lib/menu/menu.ts b/libs/components/src/lib/menu/menu.ts index 05721185ef..763a24f151 100644 --- a/libs/components/src/lib/menu/menu.ts +++ b/libs/components/src/lib/menu/menu.ts @@ -10,6 +10,7 @@ import { } from '@microsoft/fast-web-utilities'; import { type Anchored, anchored } from '../../shared/patterns/anchored'; import { MenuItem, MenuItemRole } from '../menu-item/menu-item'; +import type { Popup } from '../popup/popup'; /** * @public @@ -76,16 +77,24 @@ export class Menu extends FoundationElement { } /** - * Focuses the first item in the menu. + * Moves focus into the menu. If there is a child with the `autofocus` attribute, it will be focused. + * Otherwise, the first focusable child will be focused. * * @public */ override focus(): void { - this.setFocus(0, 1); + const autoFocusElement = this.querySelector( + '[autofocus]:not([slot="anchor"])' + ); + if (autoFocusElement instanceof HTMLElement) { + autoFocusElement.focus(); + } else { + this.setFocus(0, 1); + } } /** - * Collapses any expanded menu items. + * Collapses any expanded Menu Items. * * @public */ @@ -165,16 +174,6 @@ export class Menu extends FoundationElement { }; private handleExpandedChanged = (e: Event): void => { - if ( - e.defaultPrevented || - e.target === null || - this.menuItems === undefined || - this.menuItems.indexOf(e.target as Element) < 0 - ) { - return; - } - - e.preventDefault(); const changedItem = e.target as MenuItem; // closing an expanded item without opening another @@ -184,17 +183,10 @@ export class Menu extends FoundationElement { changedItem.expanded === false ) { this.expandedItem = null; - return; } if (changedItem.expanded) { - if (this.expandedItem !== null && this.expandedItem !== changedItem) { - this.expandedItem.expanded = false; - } - this.menuItems[this.focusIndex].setAttribute('tabindex', '-1'); this.expandedItem = changedItem; - this.focusIndex = this.menuItems.indexOf(changedItem); - changedItem.setAttribute('tabindex', '0'); } }; @@ -337,6 +329,15 @@ export class Menu extends FoundationElement { */ @attr({ mode: 'boolean' }) open = false; openChanged(_: boolean, newValue: boolean): void { + if (newValue) { + // Ensure popup is shown so that focus can be set + this._popupEl?.show(); + this.focus(); + } else { + // TODO: Focus should be restored to the anchor element when the menu is closed + // However, it cannot be implemented without triggering visible focus + } + newValue ? this.$emit('open', undefined, { bubbles: false }) : this.$emit('close', undefined, { bubbles: false }); @@ -450,6 +451,11 @@ export class Menu extends FoundationElement { */ @observable headerSlottedContent?: HTMLElement[]; @observable actionItemsSlottedContent?: HTMLElement[]; + + /** + * @internal + */ + _popupEl?: Popup; } export interface Menu extends Anchored {} diff --git a/libs/components/src/lib/popup/popup.spec.ts b/libs/components/src/lib/popup/popup.spec.ts index f5c4b090e5..c99e25e2d0 100644 --- a/libs/components/src/lib/popup/popup.spec.ts +++ b/libs/components/src/lib/popup/popup.spec.ts @@ -206,6 +206,18 @@ describe('vwc-popup', () => { expect(element.open).toEqual(true); }); + it('should show the popup synchronously', async () => { + const popoverEl = element.shadowRoot!.querySelector( + '[popover]' + ) as HTMLElement; + popoverEl.showPopover = jest.fn(); + + element.show(); + + expect(getControlElement(element).classList).toContain('open'); + expect(popoverEl.showPopover).toHaveBeenCalled(); + }); + it('should fire vwc-popup:open event', async function () { const spyOpen = jest.fn(); element.addEventListener('vwc-popup:open', spyOpen); diff --git a/libs/components/src/lib/popup/popup.ts b/libs/components/src/lib/popup/popup.ts index d935c1d9ab..8ffe680fae 100644 --- a/libs/components/src/lib/popup/popup.ts +++ b/libs/components/src/lib/popup/popup.ts @@ -97,9 +97,9 @@ export class Popup extends FoundationElement { }) open = false; openChanged(_: boolean, newValue: boolean): void { - newValue ? this.$emit('vwc-popup:open') : this.$emit('vwc-popup:close'); this.#togglePopover(); this.#updateAutoUpdate(); + this.$emit(newValue ? 'vwc-popup:open' : 'vwc-popup:close'); } /** @@ -188,6 +188,9 @@ export class Popup extends FoundationElement { this.#updateAutoUpdate(); } + /** + * @internal + */ strategyChanged() { this.#togglePopover(); } @@ -205,11 +208,14 @@ export class Popup extends FoundationElement { #updateAutoUpdate() { this.#cleanup?.(); - if (this.anchorEl && this.open && this.popupEl) { + + if (this.open && this.controlEl) { // Ensure open is synced with the control element so that popup can be measured // Otherwise, position will not be computed correctly - this.controlEl!.classList.add('open'); + this.controlEl.classList.add('open'); + } + if (this.anchorEl && this.open && this.popupEl) { this.#cleanup = autoUpdate( this.anchorEl, this.popupEl, @@ -282,8 +288,14 @@ export class Popup extends FoundationElement { return this.anchor ?? null; } + /** + * Shows the popup. + * Unlike toggling the `open` attribute, show() will ensure the popup becomes visible synchronously. + */ show(): void { this.open = true; + this.#togglePopover(); + this.#updateAutoUpdate(); } hide(): void {