new mnemonics behavior (#56617)

* new mnemonics behavior

* fixing bubbling and mnemonics showing in menus

* updating rendering and removing conflicts with keybindings

* fix submenu dismissal behavior

src/vs/base/browser/ui/menu/menu.ts

@@ -7,13 +7,15 @@
 
 import 'vs/css!./menu';
 import * as nls from 'vs/nls';
+import * as strings from 'vs/base/common/strings';
 import { IActionRunner, IAction, Action } from 'vs/base/common/actions';
 import { ActionBar, IActionItemProvider, ActionsOrientation, Separator, ActionItem, IActionItemOptions, BaseActionItem } from 'vs/base/browser/ui/actionbar/actionbar';
-import { ResolvedKeybinding, KeyCode } from 'vs/base/common/keyCodes';
-import { addClass, EventType, EventHelper, EventLike, removeTabIndexAndUpdateFocus, isAncestor, hasClass } from 'vs/base/browser/dom';
+import { ResolvedKeybinding, KeyCode, KeyCodeUtils } from 'vs/base/common/keyCodes';
+import { addClass, EventType, EventHelper, EventLike, removeTabIndexAndUpdateFocus, isAncestor, hasClass, addDisposableListener } from 'vs/base/browser/dom';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { $, Builder } from 'vs/base/browser/builder';
 import { RunOnceScheduler } from 'vs/base/common/async';
+import { IDisposable } from 'vs/base/common/lifecycle';
 
 export interface IMenuOptions {
 	context?: any;
@@ -21,6 +23,7 @@ export interface IMenuOptions {
 	actionRunner?: IActionRunner;
 	getKeyBinding?: (action: IAction) => ResolvedKeybinding;
 	ariaLabel?: string;
+	enableMnemonics?: boolean;
 }
 
 
@@ -36,8 +39,11 @@ interface ISubMenuData {
 }
 
 export class Menu extends ActionBar {
+	private mnemonics: Map<KeyCode, Array<MenuActionItem>>;
+	private menuDisposables: IDisposable[];
 
 	constructor(container: HTMLElement, actions: IAction[], options: IMenuOptions = {}) {
+
 		addClass(container, 'monaco-menu-container');
 		container.setAttribute('role', 'presentation');
 		let menuContainer = document.createElement('div');
@@ -57,6 +63,34 @@ export class Menu extends ActionBar {
 
 		this.domNode.tabIndex = 0;
 
+		this.menuDisposables = [];
+
+		if (options.enableMnemonics) {
+			this.menuDisposables.push(addDisposableListener(menuContainer, EventType.KEY_DOWN, (e) => {
+				const key = KeyCodeUtils.fromString(e.key);
+				if (this.mnemonics.has(key)) {
+					EventHelper.stop(e, true);
+					const actions = this.mnemonics.get(key);
+
+					if (actions.length === 1) {
+						if (actions[0] instanceof SubmenuActionItem) {
+							this.focusItemByElement(actions[0].container);
+						}
+
+						actions[0].onClick(event);
+					}
+
+					if (actions.length > 1) {
+						const action = actions.shift();
+						this.focusItemByElement(action.container);
+
+						actions.push(action);
+						this.mnemonics.set(key, actions);
+					}
+				}
+			}));
+		}
+
 		$(this.domNode).on(EventType.MOUSE_OUT, (e) => {
 			let relatedTarget = (e as MouseEvent).relatedTarget as HTMLElement;
 			if (!isAncestor(relatedTarget, this.domNode)) {
@@ -90,9 +124,20 @@ export class Menu extends ActionBar {
 			parent: this
 		};
 
+		this.mnemonics = new Map<KeyCode, Array<MenuActionItem>>();
+
 		this.push(actions, { icon: true, label: true, isMenu: true });
 	}
 
+	private focusItemByElement(element: HTMLElement) {
+		const lastFocusedItem = this.focusedItem;
+		this.setFocusedItem(element);
+
+		if (lastFocusedItem !== this.focusedItem) {
+			this.updateFocus();
+		}
+	}
+
 	private setFocusedItem(element: HTMLElement): void {
 		for (let i = 0; i < this.actionsList.children.length; i++) {
 			let elem = this.actionsList.children[i];
@@ -107,9 +152,25 @@ export class Menu extends ActionBar {
 		if (action instanceof Separator) {
 			return new ActionItem(options.context, action, { icon: true });
 		} else if (action instanceof SubmenuAction) {
-			return new SubmenuActionItem(action, action.entries, parentData, options);
+			const menuActionItem = new SubmenuActionItem(action, action.entries, parentData, options);
+
+			if (options.enableMnemonics) {
+				const mnemonic = menuActionItem.getMnemonic();
+				if (mnemonic && menuActionItem.isEnabled()) {
+					let actionItems = [];
+					if (this.mnemonics.has(mnemonic)) {
+						actionItems = this.mnemonics.get(mnemonic);
+					}
+
+					actionItems.push(menuActionItem);
+
+					this.mnemonics.set(mnemonic, actionItems);
+				}
+			}
+
+			return menuActionItem;
 		} else {
-			const menuItemOptions: IActionItemOptions = {};
+			const menuItemOptions: IMenuItemOptions = { enableMnemonics: options.enableMnemonics };
 			if (options.getKeyBinding) {
 				const keybinding = options.getKeyBinding(action);
 				if (keybinding) {
@@ -117,7 +178,23 @@ export class Menu extends ActionBar {
 				}
 			}
 
-			return new MenuActionItem(options.context, action, menuItemOptions);
+			const menuActionItem = new MenuActionItem(options.context, action, menuItemOptions);
+
+			if (options.enableMnemonics) {
+				const mnemonic = menuActionItem.getMnemonic();
+				if (mnemonic && menuActionItem.isEnabled()) {
+					let actionItems = [];
+					if (this.mnemonics.has(mnemonic)) {
+						actionItems = this.mnemonics.get(mnemonic);
+					}
+
+					actionItems.push(menuActionItem);
+
+					this.mnemonics.set(mnemonic, actionItems);
+				}
+			}
+
+			return menuActionItem;
 		}
 	}
 
@@ -126,16 +203,23 @@ export class Menu extends ActionBar {
 	}
 }
 
+interface IMenuItemOptions extends IActionItemOptions {
+	enableMnemonics?: boolean;
+}
+
 class MenuActionItem extends BaseActionItem {
 	static MNEMONIC_REGEX: RegExp = /&&(.)/g;
+	static ESCAPED_MNEMONIC_REGEX: RegExp = /&amp;&amp;(.)/g;
 
+	public container: HTMLElement;
 	protected $e: Builder;
 	protected $label: Builder;
 	protected $check: Builder;
-	protected options: IActionItemOptions;
+	protected options: IMenuItemOptions;
+	protected mnemonic: KeyCode;
 	private cssClass: string;
 
-	constructor(ctx: any, action: IAction, options: IActionItemOptions = {}) {
+	constructor(ctx: any, action: IAction, options: IMenuItemOptions = {}) {
 		options.isMenu = true;
 		super(action, action, options);
 
@@ -143,11 +227,24 @@ class MenuActionItem extends BaseActionItem {
 		this.options.icon = options.icon !== undefined ? options.icon : false;
 		this.options.label = options.label !== undefined ? options.label : true;
 		this.cssClass = '';
+
+		// Set mnemonic
+		if (this.options.label && options.enableMnemonics) {
+			let label = this.getAction().label;
+			if (label) {
+				let matches = MenuActionItem.MNEMONIC_REGEX.exec(label);
+				if (matches && matches.length === 2) {
+					this.mnemonic = KeyCodeUtils.fromString(matches[1].toLocaleLowerCase());
+				}
+			}
+		}
 	}
 
-	public render(container: HTMLElement): void {
+	render(container: HTMLElement): void {
 		super.render(container);
 
+		this.container = container;
+
 		this.$e = $('a.action-menu-item').appendTo(this.builder);
 		if (this._action.id === Separator.ID) {
 			// A separator is a presentation item
@@ -170,12 +267,12 @@ class MenuActionItem extends BaseActionItem {
 		this._updateChecked();
 	}
 
-	public focus(): void {
+	focus(): void {
 		super.focus();
 		this.$e.domFocus();
 	}
 
-	public _updateLabel(): void {
+	_updateLabel(): void {
 		if (this.options.label) {
 			let label = this.getAction().label;
 			if (label) {
@@ -185,20 +282,25 @@ class MenuActionItem extends BaseActionItem {
 
 					let ariaLabel = label.replace(MenuActionItem.MNEMONIC_REGEX, mnemonic);
 
-					this.$e.getHTMLElement().accessKey = mnemonic.toLocaleLowerCase();
+					this.mnemonic = KeyCodeUtils.fromString(mnemonic.toLocaleLowerCase());
+
 					this.$label.attr('aria-label', ariaLabel);
 				} else {
 					this.$label.attr('aria-label', label);
 				}
 
-				label = label.replace(MenuActionItem.MNEMONIC_REGEX, '$1\u0332');
+				if (this.options.enableMnemonics) {
+					label = strings.escape(label).replace(MenuActionItem.ESCAPED_MNEMONIC_REGEX, '<u>$1</u>');
+				} else {
+					label = strings.escape(label).replace(MenuActionItem.ESCAPED_MNEMONIC_REGEX, '$1');
+				}
 			}
 
-			this.$label.text(label);
+			this.$label.innerHtml(label);
 		}
 	}
 
-	public _updateTooltip(): void {
+	_updateTooltip(): void {
 		let title: string = null;
 
 		if (this.getAction().tooltip) {
@@ -217,7 +319,7 @@ class MenuActionItem extends BaseActionItem {
 		}
 	}
 
-	public _updateClass(): void {
+	_updateClass(): void {
 		if (this.cssClass) {
 			this.$e.removeClass(this.cssClass);
 		}
@@ -233,7 +335,7 @@ class MenuActionItem extends BaseActionItem {
 		}
 	}
 
-	public _updateEnabled(): void {
+	_updateEnabled(): void {
 		if (this.getAction().enabled) {
 			this.builder.removeClass('disabled');
 			this.$e.removeClass('disabled');
@@ -245,13 +347,17 @@ class MenuActionItem extends BaseActionItem {
 		}
 	}
 
-	public _updateChecked(): void {
+	_updateChecked(): void {
 		if (this.getAction().checked) {
 			this.$e.addClass('checked');
 		} else {
 			this.$e.removeClass('checked');
 		}
 	}
+
+	getMnemonic(): KeyCode {
+		return this.mnemonic;
+	}
 }
 
 class SubmenuActionItem extends MenuActionItem {
@@ -267,7 +373,7 @@ class SubmenuActionItem extends MenuActionItem {
 		private parentData: ISubMenuData,
 		private submenuOptions?: IMenuOptions
 	) {
-		super(action, action, { label: true, isMenu: true });
+		super(action, action, submenuOptions);
 
 		this.showScheduler = new RunOnceScheduler(() => {
 			if (this.mouseOver) {
@@ -284,7 +390,7 @@ class SubmenuActionItem extends MenuActionItem {
 		}, 750);
 	}
 
-	public render(container: HTMLElement): void {
+	render(container: HTMLElement): void {
 		super.render(container);
 
 		this.$e.addClass('monaco-submenu-item');
@@ -326,10 +432,11 @@ class SubmenuActionItem extends MenuActionItem {
 		});
 	}
 
-	public onClick(e: EventLike) {
+	onClick(e: EventLike) {
 		// stop clicking from trying to run an action
 		EventHelper.stop(e, true);
 
+		this.cleanupExistingSubmenu(false);
 		this.createSubmenu(false);
 	}
 
@@ -376,6 +483,16 @@ class SubmenuActionItem extends MenuActionItem {
 
 
 			this.parentData.submenu = new Menu(this.submenuContainer.getHTMLElement(), this.submenuActions, this.submenuOptions);
+
+			this.parentData.submenu.onDidCancel(() => {
+				this.parentData.parent.focus();
+				this.parentData.submenu.dispose();
+				this.parentData.submenu = null;
+
+				this.submenuContainer.dispose();
+				this.submenuContainer = null;
+			});
+
 			this.parentData.submenu.focus(selectFirstItem);
 
 			this.mysubmenu = this.parentData.submenu;
@@ -384,7 +501,7 @@ class SubmenuActionItem extends MenuActionItem {
 		}
 	}
 
-	public dispose() {
+	dispose() {
 		super.dispose();
 
 		this.hideScheduler.dispose();
@@ -399,4 +516,4 @@ class SubmenuActionItem extends MenuActionItem {
 			this.submenuContainer = null;
 		}
 	}
-}
\ No newline at end of file
+}

src/vs/workbench/browser/parts/titlebar/menubarControl.ts

@@ -18,7 +18,7 @@ import * as DOM from 'vs/base/browser/dom';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 import { isMacintosh } from 'vs/base/common/platform';
 import { Menu, IMenuOptions, SubmenuAction } from 'vs/base/browser/ui/menu/menu';
-import { KeyCode } from 'vs/base/common/keyCodes';
+import { KeyCode, KeyCodeUtils } from 'vs/base/common/keyCodes';
 import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { IConfigurationService, IConfigurationChangeEvent } from 'vs/platform/configuration/common/configuration';
 import { Event, Emitter } from 'vs/base/common/event';
@@ -103,9 +103,10 @@ export class MenubarControl extends Disposable {
 	private container: HTMLElement;
 	private recentlyOpened: IRecentlyOpened;
 	private updatePending: boolean;
-	private _modifierKeyStatus: IModifierKeyStatus;
 	private _focusState: MenubarState;
+	private _mnemonicsInUse: boolean;
 	private openedViaKeyboard: boolean;
+	private mnemonics: Map<KeyCode, number>;
 
 	private _onVisibilityChange: Emitter<boolean>;
 
@@ -143,7 +144,7 @@ export class MenubarControl extends Disposable {
 			this.topLevelMenus['Preferences'] = this._register(this.menuService.createMenu(MenuId.MenubarPreferencesMenu, this.contextKeyService));
 		}
 
-		this.menuUpdater = this._register(new RunOnceScheduler(() => this.doSetupMenubar(), 100));
+		this.menuUpdater = this._register(new RunOnceScheduler(() => this.doSetupMenubar(), 200));
 
 		this.actionRunner = this._register(new ActionRunner());
 		this._register(this.actionRunner.onDidBeforeRun(() => {
@@ -306,6 +307,14 @@ export class MenubarControl extends Disposable {
 		this._focusState = value;
 	}
 
+	private get mnemonicsInUse(): boolean {
+		return this._mnemonicsInUse;
+	}
+
+	private set mnemonicsInUse(value: boolean) {
+		this._mnemonicsInUse = value;
+	}
+
 	private get isVisible(): boolean {
 		return this.focusState >= MenubarState.VISIBLE;
 	}
@@ -351,6 +360,9 @@ export class MenubarControl extends Disposable {
 		} else {
 			this.focusState = MenubarState.VISIBLE;
 		}
+
+		this.mnemonicsInUse = false;
+		this.updateMnemonicVisibility(false);
 	}
 
 	private hideMenubar(): void {
@@ -364,16 +376,15 @@ export class MenubarControl extends Disposable {
 	}
 
 	private onModifierKeyToggled(modifierKeyStatus: IModifierKeyStatus): void {
-		this._modifierKeyStatus = modifierKeyStatus;
 		const allModifiersReleased = !modifierKeyStatus.altKey && !modifierKeyStatus.ctrlKey && !modifierKeyStatus.shiftKey;
 
 		if (this.currentMenubarVisibility === 'hidden') {
 			return;
 		}
 
-
 		if (allModifiersReleased && modifierKeyStatus.lastKeyPressed === 'alt' && modifierKeyStatus.lastKeyReleased === 'alt') {
 			if (!this.isFocused) {
+				this.mnemonicsInUse = true;
 				this.focusedMenu = { index: 0 };
 				this.focusState = MenubarState.FOCUSED;
 			} else if (!this.isOpen) {
@@ -381,18 +392,22 @@ export class MenubarControl extends Disposable {
 			}
 		}
 
-		if (this.currentEnableMenuBarMnemonics && this.customMenus) {
-			this.customMenus.forEach(customMenu => {
-				if (customMenu.titleElement.children.length) {
-					let child = customMenu.titleElement.children.item(0) as HTMLElement;
-					if (child) {
-						child.style.textDecoration = modifierKeyStatus.altKey ? 'underline' : null;
-					}
-				}
-			});
+		if (this.currentEnableMenuBarMnemonics && this.customMenus && !this.isOpen) {
+			this.updateMnemonicVisibility(modifierKeyStatus.altKey || this.mnemonicsInUse);
 		}
 	}
 
+	private updateMnemonicVisibility(visible: boolean): void {
+		this.customMenus.forEach(customMenu => {
+			if (customMenu.titleElement.children.length) {
+				let child = customMenu.titleElement.children.item(0) as HTMLElement;
+				if (child) {
+					child.style.textDecoration = visible ? 'underline' : null;
+				}
+			}
+		});
+	}
+
 	private onRecentlyOpenedChange(): void {
 		this.windowService.getRecentlyOpened().then(recentlyOpened => {
 			this.recentlyOpened = recentlyOpened;
@@ -448,12 +463,8 @@ export class MenubarControl extends Disposable {
 		this.menuUpdater.schedule();
 	}
 
-	private clearMnemonic(topLevelElement: HTMLElement): void {
-		topLevelElement.accessKey = null;
-	}
-
-	private registerMnemonic(topLevelElement: HTMLElement, mnemonic: string): void {
-		topLevelElement.accessKey = mnemonic.toLocaleLowerCase();
+	private registerMnemonic(menuIndex: number, mnemonic: string): void {
+		this.mnemonics.set(KeyCodeUtils.fromString(mnemonic), menuIndex);
 	}
 
 	private setCheckedStatus(action: IAction | IMenubarMenuItemAction) {
@@ -624,6 +635,7 @@ export class MenubarControl extends Disposable {
 		const firstTimeSetup = this.customMenus === undefined;
 		if (firstTimeSetup) {
 			this.customMenus = [];
+			this.mnemonics = new Map<KeyCode, number>();
 		}
 
 		let idx = 0;
@@ -651,12 +663,11 @@ export class MenubarControl extends Disposable {
 			let displayTitle = this.topLevelTitles[menuTitle].replace(/&&(.)/g, this.currentEnableMenuBarMnemonics ? '<mnemonic>$1</mnemonic>' : '$1');
 			this.customMenus[menuIndex].titleElement.innerHTML = displayTitle;
 
-			// Clear and register mnemonics due to updated settings
-			this.clearMnemonic(this.customMenus[menuIndex].buttonElement);
-			if (this.currentEnableMenuBarMnemonics) {
+			// Register mnemonics
+			if (firstTimeSetup) {
 				let mnemonic = (/&&(.)/g).exec(this.topLevelTitles[menuTitle]);
 				if (mnemonic && mnemonic[1]) {
-					this.registerMnemonic(this.customMenus[menuIndex].buttonElement, mnemonic[1]);
+					this.registerMnemonic(menuIndex, mnemonic[1]);
 				}
 			}
 
@@ -714,27 +725,7 @@ export class MenubarControl extends Disposable {
 				}));
 
 				this._register(DOM.addDisposableListener(this.customMenus[menuIndex].buttonElement, DOM.EventType.CLICK, (e) => {
-					// This should only happen for mnemonics and we shouldn't trigger them
-					if (this.currentMenubarVisibility === 'hidden') {
-						return;
-					}
-
-					if (this._modifierKeyStatus && (this._modifierKeyStatus.shiftKey || this._modifierKeyStatus.ctrlKey)) {
-						return; // supress keyboard shortcuts that shouldn't conflict
-					}
-
-					if (this.isOpen) {
-						if (this.isCurrentMenu(menuIndex)) {
-							this.setUnfocusedState();
-						} else {
-							this.cleanupCustomMenu();
-							this.showCustomMenu(menuIndex, this.openedViaKeyboard);
-						}
-					} else {
-						this.focusedMenu = { index: menuIndex };
-						this.openedViaKeyboard = (e as MouseEvent).detail === 0; // Indicates mouse was not clicked
-						this.focusState = MenubarState.OPEN;
-					}
+					this.onMenuTriggered(menuIndex, true);
 
 					e.preventDefault();
 					e.stopPropagation();
@@ -757,6 +748,7 @@ export class MenubarControl extends Disposable {
 			this._register(DOM.addDisposableListener(this.container, DOM.EventType.KEY_DOWN, (e) => {
 				let event = new StandardKeyboardEvent(e as KeyboardEvent);
 				let eventHandled = true;
+				const key = !!e.key ? KeyCodeUtils.fromString(e.key) : KeyCode.Unknown;
 
 				if (event.equals(KeyCode.LeftArrow) || (event.shiftKey && event.keyCode === KeyCode.Tab)) {
 					this.focusPrevious();
@@ -764,6 +756,9 @@ export class MenubarControl extends Disposable {
 					this.focusNext();
 				} else if (event.equals(KeyCode.Escape) && this.isFocused && !this.isOpen) {
 					this.setUnfocusedState();
+				} else if (!event.ctrlKey && this.currentEnableMenuBarMnemonics && this.mnemonicsInUse && this.mnemonics.has(key)) {
+					const menuIndex = this.mnemonics.get(key);
+					this.onMenuTriggered(menuIndex, false);
 				} else {
 					eventHandled = false;
 				}
@@ -801,6 +796,44 @@ export class MenubarControl extends Disposable {
 					}
 				}
 			}));
+
+			this._register(DOM.addDisposableListener(window, DOM.EventType.KEY_DOWN, (e) => {
+				if (!this.currentEnableMenuBarMnemonics || !e.altKey || e.ctrlKey) {
+					return;
+				}
+
+				const key = KeyCodeUtils.fromString(e.key);
+				if (!this.mnemonics.has(key)) {
+					return;
+				}
+
+				// Prevent conflicts with keybindings
+				const standardKeyboardEvent = new StandardKeyboardEvent(e);
+				const resolvedResult = this.keybindingService.softDispatch(standardKeyboardEvent, standardKeyboardEvent.target);
+				if (resolvedResult) {
+					return;
+				}
+
+				this.mnemonicsInUse = true;
+
+				const menuIndex = this.mnemonics.get(key);
+				this.onMenuTriggered(menuIndex, false);
+			}));
+		}
+	}
+
+	private onMenuTriggered(menuIndex: number, clicked: boolean) {
+		if (this.isOpen) {
+			if (this.isCurrentMenu(menuIndex)) {
+				this.setUnfocusedState();
+			} else {
+				this.cleanupCustomMenu();
+				this.showCustomMenu(menuIndex, this.openedViaKeyboard);
+			}
+		} else {
+			this.focusedMenu = { index: menuIndex };
+			this.openedViaKeyboard = clicked;
+			this.focusState = MenubarState.OPEN;
 		}
 	}
 
@@ -946,6 +979,8 @@ export class MenubarControl extends Disposable {
 
 	private cleanupCustomMenu(): void {
 		if (this.focusedMenu) {
+			// Remove focus from the menus first
+			this.customMenus[this.focusedMenu.index].buttonElement.focus();
 
 			if (this.focusedMenu.holder) {
 				DOM.removeClass(this.focusedMenu.holder.parentElement, 'open');
@@ -973,6 +1008,7 @@ export class MenubarControl extends Disposable {
 		let menuOptions: IMenuOptions = {
 			getKeyBinding: (action) => this.keybindingService.lookupKeybinding(action.id),
 			actionRunner: this.actionRunner,
+			enableMnemonics: this.mnemonicsInUse
 		};
 
 		let menuWidget = this._register(new Menu(menuHolder, customMenu.actions, menuOptions));