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 <[email protected]>
eclipse-theia · Aug 21, 2020 · f10da77 · f10da77
1 parent 9d2dcd0
commit f10da77
Show file tree

Hide file tree

Showing 5 changed files with 136 additions and 53 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
   - Note: the updated dependency may have a [performance impact](https://github.com/eclipse-theia/theia/pull/7715#issuecomment-667434288) on the deployment of plugins.
 <a name="1_5_0_electron_main_extension"></a>
 - [[electron]](#1_5_0_electron_main_extension) Electron applications can now be configured/extended through `inversify`. Added new `electronMain` extension points to provide inversify container modules. [#8076](https://github.com/eclipse-theia/theia/pull/8076)
+- [core] added functionality for handling context menu for `tab-bars` without activating the shell tab-bar [#6965](https://github.com/eclipse-theia/theia/pull/6965)
 
 <a name="breaking_changes_1.5.0">[Breaking Changes:](#breaking_changes_1.5.0)</a>
 
@@ -49,6 +50,11 @@
   },
   ```
   - Consequently, `ThemeService` and `IconThemeService` don't allow to change the default color or icon theme anymore.
+- [core] Context-menu for `tab-bars` requires an `Event` to be passed onto it to perform actions without activating the shell tab-bar [#6965](https://github.com/eclipse-theia/theia/pull/6965)
+  - Removed the logic from `handleContextMenuEvent()` that gives focus to the widget upon opening the context menu, instead functionality added to handle it without activating the widget
+  - This change triggers the context menu for a given shell tab-bar without the need to activate it
+  - While registering a command, `Event` should be passed down, if not passed, then the commands will not work correctly as they no longer rely on the activation of tab-bar
+- [core] Moved `findTitle()` and `findTabBar()` from `common-frontend-contribution.ts` to `application-shell.ts`  [#6965](https://github.com/eclipse-theia/theia/pull/6965)
 
 ## v1.4.0 - 30/07/2020
 

diff --git a/packages/core/src/browser/common-frontend-contribution.ts b/packages/core/src/browser/common-frontend-contribution.ts
@@ -603,55 +603,59 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
         });
         commandRegistry.registerCommand(CommonCommands.CLOSE_TAB, {
             isEnabled: (event?: Event) => {
-                const tabBar = this.findTabBar(event);
+                const tabBar = this.shell.findTabBar(event);
                 if (!tabBar) {
                     return false;
                 }
-                const currentTitle = this.findTitle(tabBar, event);
+                const currentTitle = this.shell.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.shell.findTabBar(event)!;
+                const currentTitle = this.shell.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.shell.findTabBar(event);
                 if (!tabBar) {
                     return false;
                 }
-                const currentTitle = this.findTitle(tabBar, event);
+                const currentTitle = this.shell.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);
+                const tabBar = this.shell.findTabBar(event)!;
+                const currentTitle = this.shell.findTitle(tabBar, event);
                 this.shell.closeTabs(tabBar, 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.shell.findTabBar(event);
+                if (!tabBar) {
+                    return false;
+                }
+                const currentIndex = this.findTitleIndex(tabBar, event);
+                return 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.shell.findTabBar(event)!;
+                const currentIndex = this.findTitleIndex(tabBar, 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.shell.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.shell.findTabBar(event)!, title => title.closable)
         });
         commandRegistry.registerCommand(CommonCommands.CLOSE_MAIN_TAB, {
             isEnabled: () => {
@@ -698,9 +702,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, {
@@ -725,38 +729,61 @@ 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;
-            }
-        }
-        return this.shell.currentTabBar;
-    }
-
     private findTabArea(event?: Event): ApplicationShell.Area | undefined {
-        const tabBar = this.findTabBar(event);
+        const tabBar = this.shell.findTabBar(event);
         if (tabBar) {
             return this.shell.getAreaFor(tabBar);
         }
         return this.shell.currentTabArea;
     }
 
-    private findTitle(tabBar: TabBar<Widget>, event?: Event): Title<Widget> | undefined {
+    /**
+     * Finds the index of the selected title from the tab-bar.
+     * @param tabBar: used for providing an array of titles.
+     * @returns the index of the selected title if it is available in the tab-bar, else returns the index of currently-selected title.
+     */
+    private findTitleIndex(tabBar: TabBar<Widget>, event?: Event): number {
+        if (event) {
+            const targetTitle = this.shell.findTitle(tabBar, event);
+            return targetTitle ? tabBar.titles.indexOf(targetTitle) : tabBar.currentIndex;
+        }
+        return tabBar.currentIndex;
+    }
+
+    private canToggleMaximized(event?: Event): boolean {
         if (event && event.target) {
-            let tabNode: HTMLElement | null = event.target as HTMLElement;
-            while (tabNode && !tabNode.classList.contains('p-TabBar-tab')) {
-                tabNode = tabNode.parentElement;
+            const widget = this.shell.findWidgetForElement(event.target as HTMLElement);
+            if (widget) {
+                return this.shell.mainPanel.contains(widget) || this.shell.bottomPanel.contains(widget);
             }
-            if (tabNode && tabNode.title) {
-                const title = tabBar.titles.find(t => t.label === tabNode!.title);
-                if (title) {
-                    return title;
+        }
+        return this.shell.canToggleMaximized();
+    }
+
+    /**
+     * Maximize the bottom or the main dockpanel based on the widget.
+     * @param event used to find the selected widget.
+     */
+    private toggleMaximized(event?: Event): void {
+        if (event && event.target) {
+            const widget = this.shell.findWidgetForElement(event.target as HTMLElement);
+            if (widget) {
+                if (this.shell.mainPanel.contains(widget)) {
+                    this.shell.mainPanel.toggleMaximized();
+                } else if (this.shell.bottomPanel.contains(widget)) {
+                    this.shell.bottomPanel.toggleMaximized();
+                }
+                if (widget instanceof TabBar) {
+                    // reveals the widget when maximized.
+                    const title = this.shell.findTitle(widget, event);
+                    if (title) {
+                        this.shell.revealWidget(title.owner.id);
+                    }
                 }
             }
+        } else {
+            this.shell.toggleMaximized();
         }
-        return tabBar.currentTitle || undefined;
     }
 
     private isElectron(): boolean {

diff --git a/packages/core/src/browser/shell/application-shell.ts b/packages/core/src/browser/shell/application-shell.ts
@@ -811,6 +811,45 @@ export class ApplicationShell extends Widget {
         return result;
     }
 
+    /**
+     * Finds the title widget from the tab-bar.
+     * @param tabBar used for providing an array of titles.
+     * @returns the selected title widget, else returns the currentTitle or undefined.
+     */
+    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) {
+                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.currentTitle || undefined;
+    }
+
+    /**
+     * Finds the tab-bar widget.
+     * @returns the selected tab-bar, else returns the currentTabBar.
+     */
+    findTabBar(event?: Event): TabBar<Widget> | undefined {
+        if (event && event.target) {
+            const tabBar = this.findWidgetForElement(event.target as HTMLElement);
+            if (tabBar instanceof TabBar) {
+                return tabBar;
+            }
+        }
+        return this.currentTabBar;
+    }
+
     /**
      * The current widget in the application shell. The current widget is the last widget that
      * was active and not yet closed. See the remarks to `activeWidget` on what _active_ means.

diff --git a/packages/core/src/browser/shell/tab-bars.ts b/packages/core/src/browser/shell/tab-bars.ts
@@ -435,18 +435,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);
         }
     };

diff --git a/packages/navigator/src/browser/navigator-contribution.ts b/packages/navigator/src/browser/navigator-contribution.ts
@@ -28,7 +28,8 @@ import {
     PreferenceService,
     SelectableTreeNode,
     SHELL_TABBAR_CONTEXT_MENU,
-    Widget
+    Widget,
+    Title
 } from '@theia/core/lib/browser';
 import { FileDownloadCommands } from '@theia/filesystem/lib/browser/download/file-download-command-contribution';
 import {
@@ -260,9 +261,18 @@ export class FileNavigatorContribution extends AbstractViewContribution<FileNavi
             execute: () => this.openView({ activate: true })
         });
         registry.registerCommand(FileNavigatorCommands.REVEAL_IN_NAVIGATOR, {
-            execute: () => this.openView({ activate: true }).then(() => this.selectWidgetFileNode(this.shell.currentWidget)),
-            isEnabled: () => Navigatable.is(this.shell.currentWidget),
-            isVisible: () => Navigatable.is(this.shell.currentWidget)
+            execute: (event?: Event) => {
+                const widget = this.findTargetedWidget(event);
+                this.openView({ activate: true }).then(() => this.selectWidgetFileNode(widget || this.shell.currentWidget));
+            },
+            isEnabled: (event?: Event) => {
+                const widget = this.findTargetedWidget(event);
+                return widget ? Navigatable.is(widget) : Navigatable.is(this.shell.currentWidget);
+            },
+            isVisible: (event?: Event) => {
+                const widget = this.findTargetedWidget(event);
+                return widget ? Navigatable.is(widget) : Navigatable.is(this.shell.currentWidget);
+            }
         });
         registry.registerCommand(FileNavigatorCommands.TOGGLE_HIDDEN_FILES, {
             execute: () => {
@@ -545,6 +555,19 @@ export class FileNavigatorContribution extends AbstractViewContribution<FileNavi
         this.tabbarToolbarRegistry.registerItem(item);
     };
 
+    /**
+     * Find the selected widget.
+     * @returns `widget` of the respective `title` if it exists, else returns undefined.
+     */
+    private findTargetedWidget(event?: Event): Widget | undefined {
+        let title: Title<Widget> | undefined;
+        if (event) {
+            const tab = this.shell.findTabBar(event);
+            title = tab && this.shell.findTitle(tab, event);
+        }
+        return title && title.owner;
+    }
+
     /**
      * Reveals and selects node in the file navigator to which given widget is related.
      * Does nothing if given widget undefined or doesn't have related resource.