feat(menu): move focus into menu on open (VIV-2196) (#2033)

Move focus into menu on open

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;
 				}
 		}

libs/components/src/lib/menu/README.md

@@ -332,10 +332,10 @@ Use the `--menu-block-size` variable to set the menu's block size.
 
 <div class="table-wrapper">
 
-| 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.                                                                                                                 |
 
 </div>
 
@@ -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
 	></vwc-text-field>
 	<vwc-menu-item role="menuitemcheckbox" text="Checkbox 1"></vwc-menu-item>
 	<vwc-menu-item role="menuitemcheckbox" text="Checkbox 2"></vwc-menu-item>

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 = `
+				<div role="menuitem" id="id1">Menu Item 1</div>
+			`;
+			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() {

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}

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 {}

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);

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 {