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 Mar 23, 2020
1 parent 868061f commit fedeb34
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 32 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
## v0.17.0

- [preferences] add a new preference to silence notifications [#7195](https://github.com/eclipse-theia/theia/pull/7195)
- [core] fixed keybindings for special Numpad keys in editors [#7315]
- [core] fixed keybindings for special Numpad keys in editors [#7315]

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

Expand Down
55 changes: 46 additions & 9 deletions packages/core/src/browser/common-frontend-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,21 +541,23 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
execute: (event?: Event) => {
const tabBar = this.findTabBar(event)!;
const currentTitle = this.findTitle(tabBar, event);
this.shell.closeTabs(tabBar, title => title !== currentTitle && title.closable);
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.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 currentIndex = this.targetTitleIndex(event);
this.shell.closeTabs(tabBar, (title, index) => index > currentIndex && title.closable);
}
});
Expand All @@ -577,7 +579,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');
Expand Down Expand Up @@ -611,9 +613,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 @@ -638,6 +640,37 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
});
}

/**
* 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.findTabBar(event)!;
const targetTitle = this.findTitle(tabBar, event);
let currentIndex: number;
if (targetTitle) {
currentIndex = tabBar.titles.indexOf(targetTitle);
} else {
currentIndex = tabBar.currentIndex;
}
return currentIndex;
}

private canToggleMaximized(event?: Event): boolean {
const targetTabBar = this.findTabBar(event);
if (targetTabBar) {
return this.shell.canToggleMaximized({ targetTabBar });
}
return false;
}

private toggleMaximized(event?: Event): void {
const targetTabBar = this.findTabBar(event)!;
this.shell.toggleMaximized({ targetTabBar });
}

private findTabBar(event?: Event): TabBar<Widget> | undefined {
if (event && event.target) {
const tabBar = this.shell.findWidgetForElement(event.target as HTMLElement);
Expand All @@ -663,7 +696,11 @@ export class CommonFrontendContribution implements FrontendApplicationContributi
tabNode = tabNode.parentElement;
}
if (tabNode && tabNode.title) {
const title = tabBar.titles.find(t => t.label === 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;
}
Expand Down
44 changes: 38 additions & 6 deletions packages/core/src/browser/shell/application-shell.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}

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 @@ -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);
}
};
Expand Down
64 changes: 60 additions & 4 deletions packages/navigator/src/browser/navigator-contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { injectable, inject, postConstruct } from 'inversify';
import { AbstractViewContribution } from '@theia/core/lib/browser/shell/view-contribution';
import {
Navigatable, SelectableTreeNode, Widget, KeybindingRegistry, CommonCommands,
OpenerService, FrontendApplicationContribution, FrontendApplication, CompositeTreeNode, PreferenceScope
OpenerService, FrontendApplicationContribution, FrontendApplication, CompositeTreeNode, PreferenceScope, TabBar, Title
} from '@theia/core/lib/browser';
import { FileDownloadCommands } from '@theia/filesystem/lib/browser/download/file-download-command-contribution';
import { CommandRegistry, MenuModelRegistry, MenuPath, isOSX, Command, DisposableCollection, Mutable } from '@theia/core/lib/common';
Expand Down Expand Up @@ -181,9 +181,20 @@ export class FileNavigatorContribution extends AbstractViewContribution<FileNavi
registerCommands(registry: CommandRegistry): void {
super.registerCommands(registry);
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.getRequiredWidget(event);
// eslint-disable-next-line no-unused-expressions
widget ? this.selectWidgetFileNode(widget) : this.selectWidgetFileNode(this.shell.currentWidget);
this.openView({ activate: true });
},
isEnabled: (event?: Event) => {
const widget = this.getRequiredWidget(event);
return widget ? Navigatable.is(widget) : Navigatable.is(this.shell.currentWidget);
},
isVisible: (event?: Event) => {
const widget = this.getRequiredWidget(event);
return widget ? Navigatable.is(widget) : Navigatable.is(this.shell.currentWidget);
}
});
registry.registerCommand(FileNavigatorCommands.TOGGLE_HIDDEN_FILES, {
execute: () => {
Expand Down Expand Up @@ -429,6 +440,51 @@ export class FileNavigatorContribution extends AbstractViewContribution<FileNavi
this.tabbarToolbarRegistry.registerItem(item);
};

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

/**
* 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 getRequiredWidget(event?: Event): Widget | undefined {
let title: Title<Widget> | undefined;
if (event && event.target) {
const tab = this.findTabBar(event);
title = this.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 fedeb34

Please sign in to comment.