Core: Do no expand the widgets on the side-bars for the context menu

Before, right clicking on different menus would focus the menu item and open it, however this should not be the case. There was a dangling code in the handleContextMenu which causes this effect, as it was checking for the id when right clicked and looked up the ID for the menu-item and set the current title to the menu-item.
Issue ID: 4367

Signed-off-by: Muhammad Anas Shahid <muhammad.shahid@ericsson.com>

CHANGELOG.md

@@ -10,6 +10,7 @@ Breaking changes:
 - [scm][git] the History view (GitHistoryWidget) has moved from the git package to a new   package, scm-extra, and
   renamed to ScmHistoryWidget.  GitNavigableListWidget has also moved.
   CSS classes have been moved renamed accordingly.  [6381](https://github.com/eclipse-theia/theia/pull/6381)
+- [core] removed the logic of giving focus to the widget upon opening the context menu from `handleContextMenu()` instead functionality added to handle it without focus. [#6965](https://github.com/eclipse-theia/theia/pull/6965)
 
 ## v0.16.0
 

packages/core/src/browser/common-frontend-contribution.ts

@@ -18,7 +18,6 @@
 
 import debounce = require('lodash.debounce');
 import { injectable, inject, postConstruct } from 'inversify';
-import { TabBar, Widget, Title } from '@phosphor/widgets';
 import { MAIN_MENU_BAR, MenuContribution, MenuModelRegistry } from '../common/menu';
 import { KeybindingContribution, KeybindingRegistry } from './keybinding';
 import { FrontendApplicationContribution } from './frontend-application';
@@ -47,6 +46,7 @@ import { ColorRegistry, Color } from './color-registry';
 import { CorePreferences } from './core-preferences';
 import { ThemeService } from './theming';
 import { PreferenceService, PreferenceScope } from './preferences';
+import { ContextMenuService } from './context-menu-service';
 
 export namespace CommonMenus {
 
@@ -284,6 +284,9 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
     @inject(PreferenceService)
     protected readonly preferenceService: PreferenceService;
 
+    @inject(ContextMenuService)
+    protected readonly contextMenuService: ContextMenuService;
+
     @postConstruct()
     protected init(): void {
         this.contextKeyService.createKey<boolean>('isLinux', OS.type() === OS.Type.Linux);
@@ -516,55 +519,57 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
         });
         commandRegistry.registerCommand(CommonCommands.CLOSE_TAB, {
             isEnabled: (event?: Event) => {
-                const tabBar = this.findTabBar(event);
+                const tabBar = this.contextMenuService.findTabBar(event);
                 if (!tabBar) {
                     return false;
                 }
-                const currentTitle = this.findTitle(tabBar, event);
+                const currentTitle = this.contextMenuService.findTitle(tabBar, event);
                 return currentTitle !== undefined && currentTitle.closable;
             },
             execute: (event?: Event) => {
-                const tabBar = this.findTabBar(event)!;
-                const currentTitle = this.findTitle(tabBar, event);
+                const tabBar = this.contextMenuService.findTabBar(event)!;
+                const currentTitle = this.contextMenuService.findTitle(tabBar, event);
                 this.shell.closeTabs(tabBar, title => title === currentTitle);
             }
         });
         commandRegistry.registerCommand(CommonCommands.CLOSE_OTHER_TABS, {
             isEnabled: (event?: Event) => {
-                const tabBar = this.findTabBar(event);
+                const tabBar = this.contextMenuService.findTabBar(event);
                 if (!tabBar) {
                     return false;
                 }
-                const currentTitle = this.findTitle(tabBar, event);
+                const currentTitle = this.contextMenuService.findTitle(tabBar, event);
                 return tabBar.titles.some(title => title !== currentTitle && title.closable);
             },
             execute: (event?: Event) => {
-                const tabBar = this.findTabBar(event)!;
-                const currentTitle = this.findTitle(tabBar, event);
-                this.shell.closeTabs(tabBar, title => title !== currentTitle && title.closable);
+                const tabBar = this.contextMenuService.findTabBar(event)!;
+                const currentTitle = this.contextMenuService.findTitle(tabBar, event);
+                const area = this.shell.getAreaFor(tabBar)!;
+                this.shell.closeTabs(area, title => title !== currentTitle && title.closable);
             }
         });
         commandRegistry.registerCommand(CommonCommands.CLOSE_RIGHT_TABS, {
             isEnabled: (event?: Event) => {
-                const tabBar = this.findTabBar(event);
-                return tabBar !== undefined && tabBar.titles.some((title, index) => index > tabBar.currentIndex && title.closable);
+                const tabBar = this.contextMenuService.findTabBar(event)!;
+                const currentIndex = this.targetTitleIndex(event);
+                return tabBar !== undefined && tabBar.titles.some((title, index) => index > currentIndex && title.closable);
             },
             isVisible: (event?: Event) => {
                 const area = this.findTabArea(event);
                 return area !== undefined && area !== 'left' && area !== 'right';
             },
             execute: (event?: Event) => {
-                const tabBar = this.findTabBar(event)!;
-                const currentIndex = tabBar.currentIndex;
+                const tabBar = this.contextMenuService.findTabBar(event)!;
+                const currentIndex = this.targetTitleIndex(event);
                 this.shell.closeTabs(tabBar, (title, index) => index > currentIndex && title.closable);
             }
         });
         commandRegistry.registerCommand(CommonCommands.CLOSE_ALL_TABS, {
             isEnabled: (event?: Event) => {
-                const tabBar = this.findTabBar(event);
+                const tabBar = this.contextMenuService.findTabBar(event);
                 return tabBar !== undefined && tabBar.titles.some(title => title.closable);
             },
-            execute: (event?: Event) => this.shell.closeTabs(this.findTabBar(event)!, title => title.closable)
+            execute: (event?: Event) => this.shell.closeTabs(this.contextMenuService.findTabBar(event)!, title => title.closable)
         });
         commandRegistry.registerCommand(CommonCommands.CLOSE_MAIN_TAB, {
             isEnabled: () => {
@@ -577,7 +582,7 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             isEnabled: () => {
                 const currentWidget = this.shell.getCurrentWidget('main');
                 return currentWidget !== undefined &&
-                       this.shell.mainAreaTabBars.some(tb => tb.titles.some(title => title.owner !== currentWidget && title.closable));
+                    this.shell.mainAreaTabBars.some(tb => tb.titles.some(title => title.owner !== currentWidget && title.closable));
             },
             execute: () => {
                 const currentWidget = this.shell.getCurrentWidget('main');
@@ -611,9 +616,9 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
             }
         });
         commandRegistry.registerCommand(CommonCommands.TOGGLE_MAXIMIZED, {
-            isEnabled: () => this.shell.canToggleMaximized(),
-            isVisible: () => this.shell.canToggleMaximized(),
-            execute: () => this.shell.toggleMaximized()
+            isEnabled: (event?: Event) => this.canToggleMaximized(event),
+            isVisible: (event?: Event) => this.canToggleMaximized(event),
+            execute: (event?: Event) => this.toggleMaximized(event)
         });
 
         commandRegistry.registerCommand(CommonCommands.SAVE, {
@@ -638,40 +643,45 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
         });
     }
 
-    private findTabBar(event?: Event): TabBar<Widget> | undefined {
-        if (event && event.target) {
-            const tabBar = this.shell.findWidgetForElement(event.target as HTMLElement);
-            if (tabBar instanceof TabBar) {
-                return tabBar;
-            }
+    /**
+     * Evaluates the currentIndex of the title in the array of titles.
+     * @param event: `event` to be used when searching for the title and the tab-bar.
+     *
+     * @returns `currentIndex` if the `targetTitle` is available in the array, else returns the index of currently-selected title.
+     */
+    private targetTitleIndex(event?: Event): number {
+        const tabBar = this.contextMenuService.findTabBar(event)!;
+        const targetTitle = this.contextMenuService.findTitle(tabBar, event);
+        let currentIndex: number;
+        if (targetTitle) {
+            currentIndex = tabBar.titles.indexOf(targetTitle);
+        } else {
+            currentIndex = tabBar.currentIndex;
         }
-        return this.shell.currentTabBar;
+        return currentIndex;
+    }
+
+    private canToggleMaximized(event?: Event): boolean {
+        const targetTabBar = this.contextMenuService.findTabBar(event);
+        if (targetTabBar) {
+            return this.shell.canToggleMaximized({ targetTabBar });
+        }
+        return false;
+    }
+
+    private toggleMaximized(event?: Event): void {
+        const targetTabBar = this.contextMenuService.findTabBar(event)!;
+        this.shell.toggleMaximized({ targetTabBar });
     }
 
     private findTabArea(event?: Event): ApplicationShell.Area | undefined {
-        const tabBar = this.findTabBar(event);
+        const tabBar = this.contextMenuService.findTabBar(event);
         if (tabBar) {
             return this.shell.getAreaFor(tabBar);
         }
         return this.shell.currentTabArea;
     }
 
-    private findTitle(tabBar: TabBar<Widget>, event?: Event): Title<Widget> | undefined {
-        if (event && event.target) {
-            let tabNode: HTMLElement | null = event.target as HTMLElement;
-            while (tabNode && !tabNode.classList.contains('p-TabBar-tab')) {
-                tabNode = tabNode.parentElement;
-            }
-            if (tabNode && tabNode.title) {
-                const title = tabBar.titles.find(t => t.label === tabNode!.title);
-                if (title) {
-                    return title;
-                }
-            }
-        }
-        return tabBar.currentTitle || undefined;
-    }
-
     private isElectron(): boolean {
         return environment.electron.is();
     }

packages/core/src/browser/context-menu-service.ts

@@ -0,0 +1,56 @@
+/********************************************************************************
+ * Copyright (C) 2019 TypeFox and others.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0.
+ *
+ * This Source Code may also be made available under the following Secondary
+ * Licenses when the conditions for such availability set forth in the Eclipse
+ * Public License v. 2.0 are satisfied: GNU General Public License, version 2
+ * with the GNU Classpath Exception which is available at
+ * https://www.gnu.org/software/classpath/license.html.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
+ ********************************************************************************/
+
+import { TabBar, Widget, Title } from '@phosphor/widgets';
+import { injectable, inject } from 'inversify';
+import { ApplicationShell } from './shell/application-shell';
+
+@injectable()
+export class ContextMenuService {
+
+    @inject(ApplicationShell) protected readonly shell: ApplicationShell;
+
+    findTitle(tabBar: TabBar<Widget> | undefined, event?: Event): Title<Widget> | undefined {
+        if (event && event.target) {
+            let tabNode: HTMLElement | null = event.target as HTMLElement;
+            while (tabNode && !tabNode.classList.contains('p-TabBar-tab')) {
+                tabNode = tabNode.parentElement;
+            }
+            if (tabBar && tabNode && tabNode.title) {
+                let title = tabBar.titles.find(t => t.caption === tabNode!.title);
+                if (title) {
+                    return title;
+                }
+                title = tabBar.titles.find(t => t.label === tabNode!.title);
+                if (title) {
+                    return title;
+                }
+            }
+        }
+        return tabBar ? tabBar.currentTitle || undefined : undefined;
+    }
+
+    findTabBar(event?: Event): TabBar<Widget> | undefined {
+        if (event && event.target) {
+            const tabBar = this.shell.findWidgetForElement(event.target as HTMLElement);
+            if (tabBar instanceof TabBar) {
+                return tabBar;
+            }
+        }
+        return this.shell.currentTabBar;
+    }
+
+}

packages/core/src/browser/frontend-application-module.ts

@@ -91,6 +91,7 @@ import { IconThemeApplicationContribution, IconThemeContribution, DefaultFileIco
 import { TreeLabelProvider } from './tree/tree-label-provider';
 import { ProgressBar } from './progress-bar';
 import { ProgressBarFactory, ProgressBarOptions } from './progress-bar-factory';
+import { ContextMenuService } from './context-menu-service';
 
 export { bindResourceProvider, bindMessageService, bindPreferenceService };
 
@@ -318,4 +319,5 @@ export const frontendApplicationModule = new ContainerModule((bind, unbind, isBo
     });
 
     bind(ContextMenuContext).toSelf().inSingletonScope();
+    bind(ContextMenuService).toSelf().inSingletonScope();
 });

packages/core/src/browser/shell/application-shell.ts

@@ -1618,15 +1618,47 @@ export class ApplicationShell extends Widget {
         return [...this.tracker.widgets];
     }
 
-    canToggleMaximized(): boolean {
-        const area = this.currentWidget && this.getAreaFor(this.currentWidget);
+    /**
+     * Determines if the target widget is located in an area which can be successfully maximized.
+     * - If `options` is provided, determine the area/location of the passed widget, else use the `currentWidget`.
+     * @param options: optional `targetTabBar` to be used when searching.
+     *
+     * @returns `true` if the widget is located in the `main` or `bottom` area, and `false` otherwise.
+     */
+    canToggleMaximized(options?: { targetTabBar: TabBar<Widget> }): boolean {
+        let area: ApplicationShell.Area | undefined;
+        if (options) {
+            const { targetTabBar } = options;
+            area = this.getAreaFor(targetTabBar);
+        } else {
+            area = this.currentWidget && this.getAreaFor(this.currentWidget);
+        }
         return area === 'main' || area === 'bottom';
     }
 
-    toggleMaximized(): void {
-        const area = this.currentWidget && this.getAreaPanelFor(this.currentWidget);
-        if (area instanceof TheiaDockPanel && (area === this.mainPanel || area === this.bottomPanel)) {
-            area.toggleMaximized();
+    /**
+     * Maximizes the target widget in the target area/location. Otherwise, throw a warning.
+     * - If `options` is provided, determine the area/location of the passed widget, else use the `currentWidget`.
+     * @param options: optional `targetTabBar` to be used when searching.
+     */
+    toggleMaximized(options?: { targetTabBar: TabBar<Widget> }): void {
+        let area: String | undefined;
+        if (options) {
+            const { targetTabBar } = options;
+            if (targetTabBar instanceof Widget && this.currentWidget) {
+                area = this.getAreaFor(targetTabBar ? targetTabBar : this.currentWidget);
+            }
+        }
+        if (!area && this.currentWidget) {
+            area = this.getAreaFor(this.currentWidget);
+        }
+
+        if (area === 'bottom') {
+            this.bottomPanel.toggleMaximized();
+        } else if (area === 'main') {
+            this.mainPanel.toggleMaximized();
+        } else {
+            console.warn('Could not find area for widget');
         }
     }
 

packages/core/src/browser/shell/tab-bars.ts

@@ -413,18 +413,6 @@ export class TabBarRenderer extends TabBar.Renderer {
         if (this.contextMenuRenderer && this.contextMenuPath && event.currentTarget instanceof HTMLElement) {
             event.stopPropagation();
             event.preventDefault();
-
-            if (this.tabBar) {
-                const id = event.currentTarget.id;
-                // eslint-disable-next-line no-null/no-null
-                const title = this.tabBar.titles.find(t => this.createTabId(t) === id) || null;
-                this.tabBar.currentTitle = title;
-                this.tabBar.activate();
-                if (title) {
-                    title.owner.activate();
-                }
-            }
-
             this.contextMenuRenderer.render(this.contextMenuPath, event);
         }
     };