improve focus handling within actionbar

Benjamin Pasero · Benjamin Pasero · commit 569865d4e7f6 · 2016-01-18T10:36:05.000+01:00
diff --git a/src/vs/base/browser/ui/actionbar/actionbar.ts b/src/vs/base/browser/ui/actionbar/actionbar.ts
@@ -440,9 +440,9 @@ export class ActionBar extends EventEmitter implements IActionRunner {
 			let event = new StandardKeyboardEvent(e);
 			let eventHandled = true;
 
-			if (event.equals(isVertical ? CommonKeybindings.UP_ARROW : CommonKeybindings.LEFT_ARROW)) {
+			if (event.equals(CommonKeybindings.UP_ARROW) || event.equals(CommonKeybindings.LEFT_ARROW)) {
 				this.focusPrevious();
-			} else if (event.equals(isVertical ? CommonKeybindings.DOWN_ARROW : CommonKeybindings.RIGHT_ARROW)) {
+			} else if (event.equals(CommonKeybindings.DOWN_ARROW) || event.equals(CommonKeybindings.RIGHT_ARROW)) {
 				this.focusNext();
 			} else if (event.equals(CommonKeybindings.ESCAPE)) {
 				this.cancel();
@@ -467,11 +467,17 @@ export class ActionBar extends EventEmitter implements IActionRunner {
 		$(this.domNode).on(DOM.EventType.KEY_UP, (e: KeyboardEvent) => {
 			let event = new StandardKeyboardEvent(e);
 
+			// Run action on Enter/Space
 			if (event.equals(CommonKeybindings.ENTER) || event.equals(CommonKeybindings.SPACE)) {
 				this.doTrigger(event);
 				event.preventDefault();
 				event.stopPropagation();
 			}
+
+			// Recompute focused item
+			else if (event.equals(CommonKeybindings.TAB)) {
+				this.updateFocusedItem();
+			}
 		});
 
 		this.focusTracker = DOM.trackFocus(this.domNode);
@@ -482,15 +488,7 @@ export class ActionBar extends EventEmitter implements IActionRunner {
 			}
 		});
 
-		this.focusTracker.addFocusListener((e: Event) => {
-			for (let i = 0; i < this.actionsList.children.length; i++) {
-				let elem = this.actionsList.children[i];
-				if (DOM.isAncestor(document.activeElement, elem)) {
-					this.focusedItem = i;
-					break;
-				}
-			}
-		});
+		this.focusTracker.addFocusListener(() => this.updateFocusedItem());
 
 		this.actionsList = document.createElement('ul');
 		this.actionsList.className = 'actions-container';
@@ -502,6 +500,16 @@ export class ActionBar extends EventEmitter implements IActionRunner {
 		container.appendChild(this.domNode);
 	}
 
+	private updateFocusedItem(): void {
+		for (let i = 0; i < this.actionsList.children.length; i++) {
+			let elem = this.actionsList.children[i];
+			if (DOM.isAncestor(document.activeElement, elem)) {
+				this.focusedItem = i;
+				break;
+			}
+		}
+	}
+
 	public get context(): any {
 		return this._context;
 	}
