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 
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>
  • Loading branch information
Muhammad Anas Shahid committed Jul 6, 2020
1 parent 7a88d28 commit 9d33d71
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 52 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- [core] fixed handling of environment variables on Windows [#7973](https://github.com/eclipse-theia/theia/pull/7973)
- [plugin-ext] fix file-icon incorrectly displays name icon with a dot in name [#7680](https://github.com/eclipse-theia/theia/pull/7680)
- [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)

Breaking Changes:

Expand All @@ -17,6 +18,11 @@ Breaking Changes:
<a name="1_4_0_deprecate_languages"></a>
- [[languages]](#1_4_0_deprecate_languages) `@theia/languages` extension is deprecated, use VS Code extensions to provide language smartness:
https://code.visualstudio.com/api/language-extensions/language-server-extension-guide [#8112](https://github.com/eclipse-theia/theia/pull/8112)
- [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.3.0

Expand Down
97 changes: 61 additions & 36 deletions packages/core/src/browser/common-frontend-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,55 +591,56 @@ 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)!;
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.shell.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.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 @@ -686,9 +687,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 @@ -713,38 +714,62 @@ 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 {
/**
* 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.shell.findTabBar(event)!;
const targetTitle = this.shell.findTitle(tabBar, event);
let currentIndex: number;
if (targetTitle) {
currentIndex = tabBar.titles.indexOf(targetTitle);
} else {
currentIndex = tabBar.currentIndex;
}
return 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);
}
return this.shell.canToggleMaximized();
}

private toggleMaximized(event?: Event): void {
if (event && event.target) {
let title: Title<Widget> | undefined;
const widget = this.shell.findWidgetForElement(event.target as HTMLElement);
if (widget) {
if (widget instanceof TabBar) {
title = this.shell.findTitle(widget, event);
}
if (this.shell.mainPanel.contains(widget)) {
this.shell.mainPanel.toggleMaximized();
} else if (this.shell.bottomPanel.contains(widget)) {
this.shell.bottomPanel.toggleMaximized();
}
if (title) {
return title;
this.shell.revealWidget(title.owner.id);
}
}
} else {
this.shell.toggleMaximized();
}
return tabBar.currentTitle || undefined;
}

private isElectron(): boolean {
Expand Down
30 changes: 30 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,36 @@ export class ApplicationShell extends Widget {
return result;
}

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;
}

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.
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 @@ -424,18 +424,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
34 changes: 30 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 @@ -255,9 +256,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.getTargetedWidget(event);
this.openView({ activate: true }).then(() => this.selectWidgetFileNode(widget || this.shell.currentWidget));
},
isEnabled: (event?: Event) => {
const widget = this.getTargetedWidget(event);
return widget ? Navigatable.is(widget) : Navigatable.is(this.shell.currentWidget);
},
isVisible: (event?: Event) => {
const widget = this.getTargetedWidget(event);
return widget ? Navigatable.is(widget) : Navigatable.is(this.shell.currentWidget);
}
});
registry.registerCommand(FileNavigatorCommands.TOGGLE_HIDDEN_FILES, {
execute: () => {
Expand Down Expand Up @@ -532,6 +542,22 @@ export class FileNavigatorContribution extends AbstractViewContribution<FileNavi
this.tabbarToolbarRegistry.registerItem(item);
};

/**
* Evaluates the widget that is triggered by the right click.
* @param event: `event` to be used when searching for the title and the tab-bar.
*
* @returns `widget` of the respective `title` if it exists, else returns undefined.
*/
private getTargetedWidget(event?: Event): Widget | undefined {
let title: Title<Widget> | undefined;
if (event) {
const tab = this.shell.findTabBar(event);
title = tab && this.shell.findTitle(tab, event);
}
const widget = title && title.owner;
return widget;
}

/**
* 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 9d33d71

Please sign in to comment.