From bd88880f78e54e88cb5083a74134ac4e130f3228 Mon Sep 17 00:00:00 2001 From: Material Web Team Date: Mon, 4 Dec 2023 16:22:22 -0800 Subject: [PATCH] fix(menu): escape not closing menus with submenus Ensure pressing escape while on a closed submenu anchor, closes the parent menu Previously we were not letting a user close a menu when they were focused on any item with keepOpen = true. This included submenu anchors. This change adds an escape (hehe) hatch to ensure that even on items with keepOpen = true, escape closes the menu. This is what most users expect and is what is the standard on menus around the web. PiperOrigin-RevId: 587874713 --- .../controllers/menuItemController.ts | 9 +++- menu/menu_test.ts | 49 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/menu/internal/controllers/menuItemController.ts b/menu/internal/controllers/menuItemController.ts index 2587166e03..28f3cb2507 100644 --- a/menu/internal/controllers/menuItemController.ts +++ b/menu/internal/controllers/menuItemController.ts @@ -223,10 +223,15 @@ export class MenuItemController implements ReactiveController { } } - if (this.host.keepOpen || event.defaultPrevented) return; + if (event.defaultPrevented) return; + + // If the host has keepOpen = true we should ignore clicks & Space/Enter, + // however we always maintain the ability to close a menu with a explicit + // `escape` keypress. const keyCode = event.code; + if (this.host.keepOpen && keyCode !== 'Escape') return; - if (!event.defaultPrevented && isClosableKey(keyCode)) { + if (isClosableKey(keyCode)) { event.preventDefault(); this.host.dispatchEvent( createDefaultCloseMenuEvent(this.host, { diff --git a/menu/menu_test.ts b/menu/menu_test.ts index 0b01c73e16..1d0a4c2f41 100644 --- a/menu/menu_test.ts +++ b/menu/menu_test.ts @@ -6,11 +6,13 @@ // import 'jasmine'; (google3-only) import './menu.js'; +import './sub-menu.js'; import {html, render} from 'lit'; import {createTokenTests} from '../testing/tokens.js'; +import {MenuItemHarness} from './harness.js'; import {MdMenu} from './menu.js'; import {MdMenuItem} from './menu-item.js'; @@ -60,6 +62,53 @@ describe('', () => { expect(menu.open).toBeFalse(); expect(escapeKeydownEvent.defaultPrevented).toBeTrue(); }); + + // Regression test for b/314706578. + it('escape on submenu items closes menu', async () => { + render( + html` + + + + +
Link Item 1
+
+ + +
Submenu Item 1
+
+
+
+
+ `, + root, + ); + + const button = root.querySelector('button')!; + const menu = root.querySelector('md-menu')!; + const submenuItemHarness = new MenuItemHarness( + menu.querySelector('#submenu-item')!, + ); + menu.anchorElement = button; + menu.show(); + + expect(menu.open).toBeTrue(); + + const escapeKeydownEvent = new KeyboardEvent('keydown', { + key: 'Escape', + code: 'Escape', + bubbles: true, + composed: true, + cancelable: true, + }); + const interactiveElement = await submenuItemHarness.getInteractiveElement(); + interactiveElement.dispatchEvent(escapeKeydownEvent); + + await menu.updateComplete; + + expect(menu.open).toBeFalse(); + expect(escapeKeydownEvent.defaultPrevented).toBeTrue(); + }); }); describe('', () => {