From 57eb460b52d1b802ab4f7c08af0a821dd0e53e1a Mon Sep 17 00:00:00 2001 From: Anton Kosyakov Date: Thu, 30 Apr 2020 12:30:06 +0000 Subject: [PATCH 1/2] fix #7170: default to last visible editor if no last focused editor Signed-off-by: Anton Kosyakov --- .../src/browser/shell/application-shell.ts | 9 +++ packages/editor/src/browser/editor-manager.ts | 37 ++++++++++- .../marker-tree-label-provider.spec.ts | 11 +++- .../workspace-uri-contribution.spec.ts | 10 ++- .../workspace-variable-contribution.ts | 65 ++++++++++++++----- 5 files changed, 107 insertions(+), 25 deletions(-) diff --git a/packages/core/src/browser/shell/application-shell.ts b/packages/core/src/browser/shell/application-shell.ts index 83339b3ee8bc8..b35d70970de16 100644 --- a/packages/core/src/browser/shell/application-shell.ts +++ b/packages/core/src/browser/shell/application-shell.ts @@ -1620,6 +1620,15 @@ export class ApplicationShell extends Widget { return [...this.tracker.widgets]; } + getWidgetById(id: string): Widget | undefined { + for (const widget of this.tracker.widgets) { + if (widget.id === id) { + return widget; + } + } + return undefined; + } + canToggleMaximized(): boolean { const area = this.currentWidget && this.getAreaFor(this.currentWidget); return area === 'main' || area === 'bottom'; diff --git a/packages/editor/src/browser/editor-manager.ts b/packages/editor/src/browser/editor-manager.ts index 29b549d8f8dba..e7ffb81be6de1 100644 --- a/packages/editor/src/browser/editor-manager.ts +++ b/packages/editor/src/browser/editor-manager.ts @@ -52,7 +52,40 @@ export class EditorManager extends NavigatableWidgetOpenHandler { super.init(); this.shell.activeChanged.connect(() => this.updateActiveEditor()); this.shell.currentChanged.connect(() => this.updateCurrentEditor()); - this.onCreated(widget => widget.disposed.connect(() => this.updateCurrentEditor())); + this.onCreated(widget => { + widget.onDidChangeVisibility(() => { + if (widget.isVisible) { + this.addRecentlyVisible(widget); + this.updateCurrentEditor(); + } + }); + widget.disposed.connect(() => { + this.removeRecentlyVisible(widget); + this.updateCurrentEditor(); + }); + }); + for (const widget of this.all) { + if (widget.isVisible) { + this.addRecentlyVisible(widget); + } + } + this.updateCurrentEditor(); + } + + protected readonly recentlyVisibleIds: string[] = []; + protected get recentlyVisible(): EditorWidget | undefined { + const id = this.recentlyVisibleIds[0]; + return id && this.all.find(w => w.id === id) || undefined; + } + protected addRecentlyVisible(widget: EditorWidget): void { + this.removeRecentlyVisible(widget); + this.recentlyVisibleIds.unshift(widget.id); + } + protected removeRecentlyVisible(widget: EditorWidget): void { + const index = this.recentlyVisibleIds.indexOf(widget.id); + if (index !== -1) { + this.recentlyVisibleIds.splice(index, 1); + } } protected _activeEditor: EditorWidget | undefined; @@ -93,7 +126,7 @@ export class EditorManager extends NavigatableWidgetOpenHandler { if (widget instanceof EditorWidget) { this.setCurrentEditor(widget); } else if (!this._currentEditor || !this._currentEditor.isVisible) { - this.setCurrentEditor(undefined); + this.setCurrentEditor(this.recentlyVisible); } } diff --git a/packages/markers/src/browser/marker-tree-label-provider.spec.ts b/packages/markers/src/browser/marker-tree-label-provider.spec.ts index f3341ec6f821f..49b8b93171924 100644 --- a/packages/markers/src/browser/marker-tree-label-provider.spec.ts +++ b/packages/markers/src/browser/marker-tree-label-provider.spec.ts @@ -21,9 +21,9 @@ let disableJSDOM = enableJSDOM(); import URI from '@theia/core/lib/common/uri'; import { expect } from 'chai'; import { Container } from 'inversify'; -import { ContributionProvider } from '@theia/core/lib/common'; +import { ContributionProvider, Event } from '@theia/core/lib/common'; import { FileStat, FileSystem } from '@theia/filesystem/lib/common'; -import { LabelProvider, LabelProviderContribution, DefaultUriLabelProviderContribution, ApplicationShell } from '@theia/core/lib/browser'; +import { LabelProvider, LabelProviderContribution, DefaultUriLabelProviderContribution, ApplicationShell, WidgetManager } from '@theia/core/lib/browser'; import { MarkerInfoNode } from './marker-tree'; import { MarkerTreeLabelProvider } from './marker-tree-label-provider'; import { Signal } from '@phosphor/signaling'; @@ -46,7 +46,12 @@ before(() => { testContainer.bind(WorkspaceService).toConstantValue(workspaceService); testContainer.bind(WorkspaceVariableContribution).toSelf().inSingletonScope(); testContainer.bind(ApplicationShell).toConstantValue({ - currentChanged: new Signal({}) + currentChanged: new Signal({}), + widgets: () => [] + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + testContainer.bind(WidgetManager).toConstantValue({ + onDidCreateWidget: Event.None // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); testContainer.bind(FileSystem).to(MockFilesystem).inSingletonScope(); diff --git a/packages/workspace/src/browser/workspace-uri-contribution.spec.ts b/packages/workspace/src/browser/workspace-uri-contribution.spec.ts index d3977bbd49feb..ab1b6fb64cebf 100644 --- a/packages/workspace/src/browser/workspace-uri-contribution.spec.ts +++ b/packages/workspace/src/browser/workspace-uri-contribution.spec.ts @@ -21,7 +21,8 @@ import { expect } from 'chai'; import * as sinon from 'sinon'; import { Container } from 'inversify'; import { Signal } from '@phosphor/signaling'; -import { ApplicationShell } from '@theia/core/lib/browser'; +import { Event } from '@theia/core/lib/common/event'; +import { ApplicationShell, WidgetManager } from '@theia/core/lib/browser'; import { FileStat, FileSystem } from '@theia/filesystem/lib/common/filesystem'; import { MockFilesystem } from '@theia/filesystem/lib/common/test'; import { DefaultUriLabelProviderContribution } from '@theia/core/lib/browser/label-provider'; @@ -44,7 +45,12 @@ beforeEach(() => { container = new Container(); container.bind(ApplicationShell).toConstantValue({ - currentChanged: new Signal({}) + currentChanged: new Signal({}), + widgets: () => [] + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + container.bind(WidgetManager).toConstantValue({ + onDidCreateWidget: Event.None // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any); const workspaceService = new WorkspaceService(); diff --git a/packages/workspace/src/browser/workspace-variable-contribution.ts b/packages/workspace/src/browser/workspace-variable-contribution.ts index 65eb458a7f04a..7f1433bdb3109 100644 --- a/packages/workspace/src/browser/workspace-variable-contribution.ts +++ b/packages/workspace/src/browser/workspace-variable-contribution.ts @@ -18,8 +18,7 @@ import { injectable, inject, postConstruct } from 'inversify'; import URI from '@theia/core/lib/common/uri'; import { Path } from '@theia/core/lib/common/path'; import { FileSystem } from '@theia/filesystem/lib/common'; -import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable'; -import { ApplicationShell, NavigatableWidget } from '@theia/core/lib/browser'; +import { ApplicationShell, NavigatableWidget, WidgetManager } from '@theia/core/lib/browser'; import { VariableContribution, VariableRegistry, Variable } from '@theia/variable-resolver/lib/browser'; import { WorkspaceService } from './workspace-service'; @@ -32,31 +31,62 @@ export class WorkspaceVariableContribution implements VariableContribution { protected readonly shell: ApplicationShell; @inject(FileSystem) protected readonly fileSystem: FileSystem; + @inject(WidgetManager) + protected readonly widgetManager: WidgetManager; protected currentWidget: NavigatableWidget | undefined; @postConstruct() protected init(): void { - this.updateCurrentWidget(); this.shell.currentChanged.connect(() => this.updateCurrentWidget()); + this.widgetManager.onDidCreateWidget(({ widget }) => { + if (NavigatableWidget.is(widget)) { + widget.onDidChangeVisibility(() => { + if (widget.isVisible) { + this.addRecentlyVisible(widget); + this.updateCurrentWidget(); + } + }); + widget.onDidDispose(() => { + this.removeRecentlyVisible(widget); + this.updateCurrentWidget(); + }); + } + }); + for (const widget of this.shell.widgets) { + if (NavigatableWidget.is(widget) && widget.isVisible) { + this.addRecentlyVisible(widget); + } + } + this.updateCurrentWidget(); } - protected updateCurrentWidget(): void { - const { currentWidget } = this.shell; - if (NavigatableWidget.is(currentWidget)) { - this.setCurrentWidget(currentWidget); + + protected readonly recentlyVisibleIds: string[] = []; + protected get recentlyVisible(): NavigatableWidget | undefined { + const id = this.recentlyVisibleIds[0]; + const widget = id && this.shell.getWidgetById(id) || undefined; + if (NavigatableWidget.is(widget)) { + return widget; + } + return undefined; + } + protected addRecentlyVisible(widget: NavigatableWidget): void { + this.removeRecentlyVisible(widget); + this.recentlyVisibleIds.unshift(widget.id); + } + protected removeRecentlyVisible(widget: NavigatableWidget): void { + const index = this.recentlyVisibleIds.indexOf(widget.id); + if (index !== -1) { + this.recentlyVisibleIds.splice(index, 1); } } - protected readonly toDisposeOnUpdateCurrentWidget = new DisposableCollection(); - protected setCurrentWidget(currentWidget: NavigatableWidget | undefined): void { - this.toDisposeOnUpdateCurrentWidget.dispose(); - this.currentWidget = currentWidget; - if (currentWidget) { - const resetCurrentWidget = () => this.setCurrentWidget(undefined); - currentWidget.disposed.connect(resetCurrentWidget); - this.toDisposeOnUpdateCurrentWidget.push(Disposable.create(() => - currentWidget.disposed.disconnect(resetCurrentWidget) - )); + protected updateCurrentWidget(): void { + const { currentWidget } = this.shell; + if (NavigatableWidget.is(currentWidget)) { + this.currentWidget = currentWidget; + } else if (!this.currentWidget || !this.currentWidget.isVisible) { + this.currentWidget = this.recentlyVisible; } } @@ -178,7 +208,6 @@ export class WorkspaceVariableContribution implements VariableContribution { } getResourceUri(): URI | undefined { - // TODO replace with ResourceContextKey.get? return this.currentWidget && this.currentWidget.getResourceUri(); } From bb7462dc2121c66beaab3ebe896a887172c326ea Mon Sep 17 00:00:00 2001 From: Anton Kosyakov Date: Thu, 30 Apr 2020 13:32:10 +0000 Subject: [PATCH 2/2] fix #2234: activate another widget if an active widget gets closed support next/previous tab group commands support next/previous tab in group commands Signed-off-by: Anton Kosyakov --- .../browser/common-frontend-contribution.ts | 36 ++++++ .../src/browser/shell/application-shell.ts | 122 ++++++++++++++++-- 2 files changed, 144 insertions(+), 14 deletions(-) diff --git a/packages/core/src/browser/common-frontend-contribution.ts b/packages/core/src/browser/common-frontend-contribution.ts index 843541f66f4fe..dc2ca29cfbc13 100644 --- a/packages/core/src/browser/common-frontend-contribution.ts +++ b/packages/core/src/browser/common-frontend-contribution.ts @@ -133,6 +133,26 @@ export namespace CommonCommands { category: VIEW_CATEGORY, label: 'Switch to Previous Tab' }; + export const NEXT_TAB_IN_GROUP: Command = { + id: 'core.nextTabInGroup', + category: VIEW_CATEGORY, + label: 'Switch to Next Tab in Group' + }; + export const PREVIOUS_TAB_IN_GROUP: Command = { + id: 'core.previousTabInGroup', + category: VIEW_CATEGORY, + label: 'Switch to Previous Tab in Group' + }; + export const NEXT_TAB_GROUP: Command = { + id: 'core.nextTabGroup', + category: VIEW_CATEGORY, + label: 'Switch to Next Tab Group' + }; + export const PREVIOUS_TAB_GROUP: Command = { + id: 'core.previousTabBar', + category: VIEW_CATEGORY, + label: 'Switch to Previous Tab Group' + }; export const CLOSE_TAB: Command = { id: 'core.close.tab', category: VIEW_CATEGORY, @@ -529,6 +549,22 @@ export class CommonFrontendContribution implements FrontendApplicationContributi isEnabled: () => this.shell.currentTabBar !== undefined, execute: () => this.shell.activatePreviousTab() }); + commandRegistry.registerCommand(CommonCommands.NEXT_TAB_IN_GROUP, { + isEnabled: () => this.shell.nextTabIndexInTabBar() !== -1, + execute: () => this.shell.activateNextTabInTabBar() + }); + commandRegistry.registerCommand(CommonCommands.PREVIOUS_TAB_IN_GROUP, { + isEnabled: () => this.shell.previousTabIndexInTabBar() !== -1, + execute: () => this.shell.activatePreviousTabInTabBar() + }); + commandRegistry.registerCommand(CommonCommands.NEXT_TAB_GROUP, { + isEnabled: () => this.shell.nextTabBar() !== undefined, + execute: () => this.shell.activateNextTabBar() + }); + commandRegistry.registerCommand(CommonCommands.PREVIOUS_TAB_GROUP, { + isEnabled: () => this.shell.previousTabBar() !== undefined, + execute: () => this.shell.activatePreviousTabBar() + }); commandRegistry.registerCommand(CommonCommands.CLOSE_TAB, { isEnabled: (event?: Event) => { const tabBar = this.findTabBar(event); diff --git a/packages/core/src/browser/shell/application-shell.ts b/packages/core/src/browser/shell/application-shell.ts index b35d70970de16..719bde33ef7c2 100644 --- a/packages/core/src/browser/shell/application-shell.ts +++ b/packages/core/src/browser/shell/application-shell.ts @@ -877,6 +877,8 @@ export class ApplicationShell extends Widget { */ readonly activeChanged = new Signal>(this); + protected readonly toDisposeOnActiveChanged = new DisposableCollection(); + /** * Handle a change to the active widget. */ @@ -914,6 +916,28 @@ export class ApplicationShell extends Widget { } // Set the z-index so elements with `position: fixed` contained in the active widget are displayed correctly this.setZIndex(newValue.node, '1'); + + // activate another widget if an active widget gets closed + const onCloseRequest = newValue['onCloseRequest']; + newValue['onCloseRequest'] = msg => { + const currentTabBar = this.currentTabBar; + if (currentTabBar) { + const recentlyUsedInTabBar = currentTabBar['_previousTitle'] as TabBar['currentTitle']; + if (recentlyUsedInTabBar && recentlyUsedInTabBar.owner !== newValue) { + currentTabBar.currentIndex = ArrayExt.firstIndexOf(currentTabBar.titles, recentlyUsedInTabBar); + if (currentTabBar.currentTitle) { + currentTabBar.currentTitle.owner.activate(); + } + } else if (!this.activateNextTabInTabBar(currentTabBar)) { + if (!this.activatePreviousTabBar(currentTabBar)) { + this.activateNextTabBar(currentTabBar); + } + } + } + newValue['onCloseRequest'] = onCloseRequest; + newValue['onCloseRequest'](msg); + }; + this.toDisposeOnActiveChanged.push(Disposable.create(() => newValue['onCloseRequest'] = onCloseRequest)); } this.activeChanged.emit(args); this.onDidChangeActiveWidgetEmitter.fire(args); @@ -1502,7 +1526,33 @@ export class ApplicationShell extends Widget { /* * Activate the next tab in the current tab bar. */ - activateNextTab(): void { + activateNextTabInTabBar(current: TabBar | undefined = this.currentTabBar): boolean { + const index = this.nextTabIndexInTabBar(current); + if (!current || index === -1) { + return false; + } + current.currentIndex = index; + if (current.currentTitle) { + current.currentTitle.owner.activate(); + } + return true; + } + + nextTabIndexInTabBar(current: TabBar | undefined = this.currentTabBar): number { + if (!current || current.titles.length <= 1) { + return -1; + } + const index = current.currentIndex; + if (index === -1) { + return -1; + } + if (index < current.titles.length - 1) { + return index + 1; + } + return 0; + } + + activateNextTab(): boolean { const current = this.currentTabBar; if (current) { const ci = current.currentIndex; @@ -1512,21 +1562,31 @@ export class ApplicationShell extends Widget { if (current.currentTitle) { current.currentTitle.owner.activate(); } + return true; } else if (ci === current.titles.length - 1) { - const nextBar = this.nextTabBar(current); - nextBar.currentIndex = 0; - if (nextBar.currentTitle) { - nextBar.currentTitle.owner.activate(); - } + return this.activateNextTabBar(current); } } } + return false; + } + + activateNextTabBar(current: TabBar | undefined = this.currentTabBar): boolean { + const nextBar = this.nextTabBar(current); + if (nextBar) { + nextBar.currentIndex = 0; + if (nextBar.currentTitle) { + nextBar.currentTitle.owner.activate(); + } + return true; + } + return false; } /** * Return the tab bar next to the given tab bar; return the given tab bar if there is no adjacent one. */ - private nextTabBar(current: TabBar): TabBar { + nextTabBar(current: TabBar | undefined = this.currentTabBar): TabBar | undefined { let bars = toArray(this.bottomPanel.tabBars()); let len = bars.length; let ci = ArrayExt.firstIndexOf(bars, current); @@ -1547,6 +1607,32 @@ export class ApplicationShell extends Widget { /* * Activate the previous tab in the current tab bar. */ + activatePreviousTabInTabBar(current: TabBar | undefined = this.currentTabBar): boolean { + const index = this.previousTabIndexInTabBar(current); + if (!current || index === -1) { + return false; + } + current.currentIndex = index; + if (current.currentTitle) { + current.currentTitle.owner.activate(); + } + return true; + } + + previousTabIndexInTabBar(current: TabBar | undefined = this.currentTabBar): number { + if (!current || current.titles.length <= 1) { + return -1; + } + const index = current.currentIndex; + if (index === -1) { + return -1; + } + if (index > 0) { + return index - 1; + } + return current.titles.length - 1; + } + activatePreviousTab(): void { const current = this.currentTabBar; if (current) { @@ -1558,21 +1644,29 @@ export class ApplicationShell extends Widget { current.currentTitle.owner.activate(); } } else if (ci === 0) { - const prevBar = this.previousTabBar(current); - const len = prevBar.titles.length; - prevBar.currentIndex = len - 1; - if (prevBar.currentTitle) { - prevBar.currentTitle.owner.activate(); - } + this.activatePreviousTabBar(current); } } } } + activatePreviousTabBar(current: TabBar | undefined = this.currentTabBar): boolean { + const prevBar = this.previousTabBar(current); + if (!prevBar) { + return false; + } + const len = prevBar.titles.length; + prevBar.currentIndex = len - 1; + if (prevBar.currentTitle) { + prevBar.currentTitle.owner.activate(); + } + return true; + } + /** * Return the tab bar previous to the given tab bar; return the given tab bar if there is no adjacent one. */ - private previousTabBar(current: TabBar): TabBar { + previousTabBar(current: TabBar | undefined = this.currentTabBar): TabBar | undefined { const bars = toArray(this.mainPanel.tabBars()); const len = bars.length; const ci = ArrayExt.firstIndexOf(bars, current);