fix toolbar to play nice with keyboard focus

Benjamin Pasero · Benjamin Pasero · commit 3e288084bb56 · 2016-01-18T10:20:00.000+01:00
diff --git a/src/vs/base/browser/ui/actionbar/actionbar.ts b/src/vs/base/browser/ui/actionbar/actionbar.ts
@@ -662,9 +662,8 @@ export class ActionBar extends EventEmitter implements IActionRunner {
 	}
 
 	private doTrigger(event): void {
-		//nothing to focus
 		if (typeof this.focusedItem === 'undefined') {
-			return;
+			return; //nothing to focus
 		}
 
 		// trigger action
diff --git a/src/vs/base/browser/ui/toolbar/toolbar.ts b/src/vs/base/browser/ui/toolbar/toolbar.ts
@@ -7,6 +7,7 @@
 
 import 'vs/css!./toolbar';
 import nls = require('vs/nls');
+import {Promise} from 'vs/base/common/winjs.base';
 import {IDisposable} from 'vs/base/common/lifecycle';
 import {Builder, $} from 'vs/base/browser/builder';
 import types = require('vs/base/common/types');
@@ -15,7 +16,7 @@ import {ActionBar, ActionsOrientation, IActionItemProvider, BaseActionItem} from
 import {IContextMenuProvider, DropdownMenu, IActionProvider, ILabelRenderer, IDropdownMenuOptions} from 'vs/base/browser/ui/dropdown/dropdown';
 import {ListenerUnbind} from 'vs/base/common/eventEmitter';
 
-export var CONTEXT = 'context.toolbar';
+export const CONTEXT = 'context.toolbar';
 
 export interface IToolBarOptions {
 	orientation?: ActionsOrientation;
@@ -34,9 +35,9 @@ export class ToolBar {
 
 	constructor(container: HTMLElement, contextMenuProvider: IContextMenuProvider, options: IToolBarOptions = { orientation: ActionsOrientation.HORIZONTAL }) {
 		this.options = options;
-		this.toggleMenuAction = new ToggleMenuAction();
+		this.toggleMenuAction = new ToggleMenuAction(() => this.toggleMenuActionItem && this.toggleMenuActionItem.show());
 
-		var element = document.createElement('div');
+		let element = document.createElement('div');
 		element.className = 'monaco-toolbar';
 		container.appendChild(element);
 
@@ -58,7 +59,6 @@ export class ToolBar {
 						(<ToggleMenuAction>action).menuActions,
 						contextMenuProvider,
 						this.options.actionItemProvider,
-						this.options.orientation === ActionsOrientation.HORIZONTAL,
 						this.actionRunner,
 						'toolbar-toggle-more'
 					);
@@ -85,7 +85,7 @@ export class ToolBar {
 
 	public setActions(primaryActions: IAction[], secondaryActions?: IAction[]): () => void {
 		return () => {
-			var primaryActionsToSet = primaryActions ? primaryActions.slice(0) : [];
+			let primaryActionsToSet = primaryActions ? primaryActions.slice(0) : [];
 
 			// Inject additional action to open secondary actions if present
 			this.hasSecondaryActions = secondaryActions && secondaryActions.length > 0;
@@ -104,7 +104,7 @@ export class ToolBar {
 
 			// Add after the "..." action if we have secondary actions
 			if (this.hasSecondaryActions) {
-				var itemCount = this.actionBar.length();
+				let itemCount = this.actionBar.length();
 				this.actionBar.push(primaryActions, { icon: true, label: false, index: itemCount });
 			}
 
@@ -130,9 +130,18 @@ class ToggleMenuAction extends Action {
 	public static ID = 'toolbar.toggle.more';
 
 	private _menuActions: IAction[];
+	private toggleDropdownMenu: () => void;
 
-	constructor() {
+	constructor(toggleDropdownMenu: () => void) {
 		super(ToggleMenuAction.ID, nls.localize('more', "More"), null, true);
+
+		this.toggleDropdownMenu = toggleDropdownMenu;
+	}
+
+	public run(): Promise {
+		this.toggleDropdownMenu();
+
+		return Promise.as(true);
 	}
 
 	public get menuActions() {
@@ -145,59 +154,40 @@ class ToggleMenuAction extends Action {
 }
 
 export class DropdownMenuActionItem extends BaseActionItem {
-
 	private menuActionsOrProvider: any;
-	private animateClick: boolean;
 	private dropdownMenu: DropdownMenu;
 	private toUnbind: ListenerUnbind;
 	private contextMenuProvider: IContextMenuProvider;
 	private actionItemProvider: IActionItemProvider;
 	private clazz: string;
 
-	constructor(action: IAction, menuActions: IAction[], contextMenuProvider: IContextMenuProvider, actionItemProvider: IActionItemProvider, animateClick: boolean, actionRunner: IActionRunner, clazz: string);
-	constructor(action: IAction, actionProvider: IActionProvider, contextMenuProvider: IContextMenuProvider, actionItemProvider: IActionItemProvider, animateClick: boolean, actionRunner: IActionRunner, clazz: string);
-	constructor(action: IAction, menuActionsOrProvider: any, contextMenuProvider: IContextMenuProvider, actionItemProvider: IActionItemProvider, animateClick: boolean, actionRunner: IActionRunner, clazz: string) {
+	constructor(action: IAction, menuActions: IAction[], contextMenuProvider: IContextMenuProvider, actionItemProvider: IActionItemProvider, actionRunner: IActionRunner, clazz: string);
+	constructor(action: IAction, actionProvider: IActionProvider, contextMenuProvider: IContextMenuProvider, actionItemProvider: IActionItemProvider, actionRunner: IActionRunner, clazz: string);
+	constructor(action: IAction, menuActionsOrProvider: any, contextMenuProvider: IContextMenuProvider, actionItemProvider: IActionItemProvider, actionRunner: IActionRunner, clazz: string) {
 		super(null, action);
 
 		this.menuActionsOrProvider = menuActionsOrProvider;
 		this.contextMenuProvider = contextMenuProvider;
 		this.actionItemProvider = actionItemProvider;
-		this.animateClick = animateClick;
 		this.actionRunner = actionRunner;
 		this.clazz = clazz;
 	}
 
 	public render(container: HTMLElement): void {
-		super.render(container);
-
-		var labelRenderer: ILabelRenderer = (el: HTMLElement): IDisposable => {
-			var e = $('a.action-label').attr({
+		let labelRenderer: ILabelRenderer = (el: HTMLElement): IDisposable => {
+			this.builder = $('a.action-label').attr({
 				tabIndex: '0',
 				role: 'menuitem',
 				title: this._action.label || '',
 				class: this.clazz
-			}).appendTo(el);
+			});
 
-			$('span.label').text(this.getAction().label).appendTo(e);
-
-			if (this.animateClick) {
-				$(el).on('mousedown', (e: MouseEvent) => {
-					if (e.button === 0) {
-						$(el).addClass('active');
-					}
-				});
-
-				$(el).on(['mouseup', 'mouseout'], (e: MouseEvent) => {
-					if (e.button === 0) {
-						$(el).removeClass('active');
-					}
-				});
-			}
+			this.builder.appendTo(el);
 
 			return null;
 		};
 
-		var options: IDropdownMenuOptions = {
+		let options: IDropdownMenuOptions = {
 			contextMenuProvider: this.contextMenuProvider,
 			labelRenderer: labelRenderer
 		};
@@ -220,6 +210,10 @@ export class DropdownMenuActionItem extends BaseActionItem {
 		this.toUnbind = this.addEmitter(this.dropdownMenu);
 	}
 
+	public show(): void {
+		this.dropdownMenu && this.dropdownMenu.show();
+	}
+
 	public dispose(): void {
 		this.toUnbind();
 		this.dropdownMenu.dispose();

Original file line number	Diff line number	Diff line change
`@@ -662,9 +662,8 @@ export class ActionBar extends EventEmitter implements IActionRunner {`
`662`	`662`	`}`
`663`	`663`
`664`	`664`	`private doTrigger(event): void {`
`665`		`- //nothing to focus`
`666`	`665`	`if (typeof this.focusedItem === 'undefined') {`
`667`		`- return;`
	`666`	`+ return; //nothing to focus`
`668`	`667`	`}`
`669`	`668`
`670`	`669`	`// trigger action`