From aa8850afc03095226005c53d369d6e6ef18bd77d Mon Sep 17 00:00:00 2001
From: EstherPerelman <esther.perelman@sap.com>
Date: Thu, 1 Oct 2020 18:45:04 +0300
Subject: [PATCH] Fix for #8572: Previous active tab flicker when navigating
 the top menu

Signed-off-by: Esther Perelman <esther.perelman@sap.com>
---
 .../src/browser/menu/browser-menu-plugin.ts   | 51 +++++++++----------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/packages/core/src/browser/menu/browser-menu-plugin.ts b/packages/core/src/browser/menu/browser-menu-plugin.ts
index 9187a01355430..b7b2b798bea40 100644
--- a/packages/core/src/browser/menu/browser-menu-plugin.ts
+++ b/packages/core/src/browser/menu/browser-menu-plugin.ts
@@ -17,11 +17,13 @@
 import { injectable, inject } from 'inversify';
 import { MenuBar, Menu as MenuWidget, Widget } from '@phosphor/widgets';
 import { CommandRegistry as PhosphorCommandRegistry } from '@phosphor/commands';
+import { ElementExt } from '@phosphor/domutils';
 import {
     CommandRegistry, ActionMenuNode, CompositeMenuNode,
     MenuModelRegistry, MAIN_MENU_BAR, MenuPath, DisposableCollection, Disposable, MenuNode
 } from '../../common';
 import { KeybindingRegistry } from '../keybinding';
+import { Key } from '../keyboard/keys';
 import { FrontendApplicationContribution, FrontendApplication } from '../frontend-application';
 import { ContextKeyService } from '../context-key-service';
 import { ContextMenuContext } from './context-menu-context';
@@ -31,6 +33,19 @@ import { ApplicationShell } from '../shell';
 export abstract class MenuBarWidget extends MenuBar {
     abstract activateMenu(label: string, ...labels: string[]): Promise<MenuWidget>;
     abstract triggerMenuItem(label: string, ...labels: string[]): Promise<MenuWidget.IItem>;
+
+    handleEvent(event: Event): void {
+        // on menubar click - if the menu is open -
+        // close active menu and restore the last focused element
+        if (event instanceof MouseEvent &&
+            event.type === 'mousedown' &&
+            ElementExt.hitTest(this.node, event.clientX, event.clientY) &&
+            this.activeMenu?.isVisible &&
+            this.activeMenu instanceof DynamicMenuWidget) {
+            this.activeMenu.restoreFocusedElement();
+        }
+        super.handleEvent(event);
+    }
 }
 
 @injectable()
@@ -237,19 +252,15 @@ export class DynamicMenuWidget extends MenuWidget {
     public aboutToShow({ previousFocusedElement }: { previousFocusedElement: HTMLElement | undefined }): void {
         this.preserveFocusedElement(previousFocusedElement);
         this.clearItems();
-        this.runWithPreservedFocusContext(() => {
-            this.options.commands.snapshot();
-            this.updateSubMenus(this, this.menu, this.options.commands);
-        });
+        this.options.commands.snapshot();
+        this.updateSubMenus(this, this.menu, this.options.commands);
     }
 
-    public open(x: number, y: number, options?: MenuWidget.IOpenOptions): void {
-        const cb = () => {
+    handleEvent(event: Event): void {
+        if (event instanceof KeyboardEvent && event.key === Key.ESCAPE.code) {
             this.restoreFocusedElement();
-            this.aboutToClose.disconnect(cb);
-        };
-        this.aboutToClose.connect(cb);
-        super.open(x, y, options);
+        }
+        super.handleEvent(event);
     }
 
     private updateSubMenus(parent: MenuWidget, menu: CompositeMenuNode, commands: MenuCommandRegistry): void {
@@ -308,14 +319,14 @@ export class DynamicMenuWidget extends MenuWidget {
     }
 
     protected preserveFocusedElement(previousFocusedElement: Element | null = document.activeElement): boolean {
-        if (!this.previousFocusedElement && previousFocusedElement instanceof HTMLElement) {
+        if (previousFocusedElement instanceof HTMLElement) {
             this.previousFocusedElement = previousFocusedElement;
             return true;
         }
         return false;
     }
 
-    protected restoreFocusedElement(): boolean {
+    public restoreFocusedElement(): boolean {
         if (this.previousFocusedElement) {
             this.previousFocusedElement.focus({ preventScroll: true });
             this.previousFocusedElement = undefined;
@@ -324,22 +335,6 @@ export class DynamicMenuWidget extends MenuWidget {
         return false;
     }
 
-    protected runWithPreservedFocusContext(what: () => void): void {
-        let focusToRestore: HTMLElement | undefined = undefined;
-        const { activeElement } = document;
-        if (this.previousFocusedElement && activeElement instanceof HTMLElement && this.previousFocusedElement !== activeElement) {
-            focusToRestore = activeElement;
-            this.previousFocusedElement.focus({ preventScroll: true });
-        }
-        try {
-            what();
-        } finally {
-            if (focusToRestore) {
-                focusToRestore.focus({ preventScroll: true });
-            }
-        }
-    }
-
 }
 
 @injectable()