Skip to content

Commit

Permalink
Core: Do no expand the widgets on the side-bars for the context menu
Browse files Browse the repository at this point in the history 
Fixes: #4367

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.

Signed-off-by: Muhammad Anas Shahid <muhammad.shahid@ericsson.com>
  • Loading branch information
Muhammad Anas Shahid committed Aug 21, 2020
1 parent b10fec5 commit 976a752
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 55 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -53,6 +54,11 @@
- [[repo]](#1_5_0_drop_node_10_support) support for `Node 10` is dropped. [#8290](https://github.com/eclipse-theia/theia/pull/8290)
- From now on, Node.js `12.x` is required when building.\
The recommended minimum version is aligned with `electron` (Node.js `12.14.1`).
- [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

Expand Down
105 changes: 66 additions & 39 deletions packages/core/src/browser/common-frontend-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import debounce = require('lodash.debounce');
import { injectable, inject } from 'inversify';
import { TabBar, Widget, Title } from '@phosphor/widgets';
import { TabBar, Widget } from '@phosphor/widgets';
import { MAIN_MENU_BAR, MenuContribution, MenuModelRegistry } from '../common/menu';
import { KeybindingContribution, KeybindingRegistry } from './keybinding';
import { FrontendApplicationContribution } from './frontend-application';
Expand Down Expand Up @@ -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: () => {
Expand Down Expand Up @@ -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, {
Expand All @@ -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 {
if (event && event.target) {
let tabNode: HTMLElement | null = event.target as HTMLElement;
while (tabNode && !tabNode.classList.contains('p-TabBar-tab')) {
tabNode = tabNode.parentElement;
/**
* 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?.target instanceof HTMLElement) {
const widget = this.shell.findWidgetForElement(event.target);
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?.target instanceof HTMLElement) {
const widget = this.shell.findWidgetForElement(event.target);
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 {
Expand Down
39 changes: 39 additions & 0 deletions packages/core/src/browser/shell/application-shell.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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?.target instanceof HTMLElement) {
let tabNode: HTMLElement | null = event.target;
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?.target instanceof HTMLElement) {
const tabBar = this.findWidgetForElement(event.target);
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.
Expand Down
12 changes: 0 additions & 12 deletions packages/core/src/browser/shell/tab-bars.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};
Expand Down
31 changes: 27 additions & 4 deletions packages/navigator/src/browser/navigator-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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: () => {
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 976a752

Please sign in to comment.