Skip to content

Commit

Permalink
[plugin] ensure that views are prepared before activating them
Browse files Browse the repository at this point in the history 
Signed-off-by: Anton Kosyakov <anton.kosyakov@typefox.io>
  • Loading branch information
@akosyakov
akosyakov committed Jun 4, 2020
1 parent 2973eee commit 5525057
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 36 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
# Change Log

## v1.3.0

Breaking Changes:

- [task] Widened the scope of some methods in TaskManager and TaskConfigurations from string to TaskConfigurationScope. This is only breaking for extenders, not callers. [#7928](https://github.com/eclipse-theia/theia/pull/7928)
- [shell] `ApplicationShell.TrackableWidgetProvider.getTrackableWidgets` is sync to register child widgets in the same tick, use `ApplicationShell.TrackableWidgetProvider.onDidChangeTrackableWidgets` if child widgets are added async

## v1.2.0

Expand Down
28 changes: 14 additions & 14 deletions packages/core/src/browser/shell/application-shell.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
} from '@phosphor/widgets';
import { Message } from '@phosphor/messaging';
import { IDragEvent } from '@phosphor/dragdrop';
import { RecursivePartial, MaybePromise, Event as CommonEvent, DisposableCollection, Disposable } from '../../common';
import { RecursivePartial, Event as CommonEvent, DisposableCollection, Disposable } from '../../common';
import { animationFrame } from '../browser';
import { Saveable, SaveableWidget } from '../saveable';
import { StatusBarImpl, StatusBarEntry, StatusBarAlignment } from '../status-bar/status-bar';
Expand Down Expand Up @@ -610,18 +610,18 @@ export class ApplicationShell extends Widget {
const { mainPanel, bottomPanel, leftPanel, rightPanel, activeWidgetId } = layoutData;
if (leftPanel) {
this.leftPanelHandler.setLayoutData(leftPanel);
await this.registerWithFocusTracker(leftPanel);
this.registerWithFocusTracker(leftPanel);
}
if (rightPanel) {
this.rightPanelHandler.setLayoutData(rightPanel);
await this.registerWithFocusTracker(rightPanel);
this.registerWithFocusTracker(rightPanel);
}
// Proceed with the bottom panel once the side panels are set up
await Promise.all([this.leftPanelHandler.state.pendingUpdate, this.rightPanelHandler.state.pendingUpdate]);
if (bottomPanel) {
if (bottomPanel.config) {
this.bottomPanel.restoreLayout(bottomPanel.config);
await this.registerWithFocusTracker(bottomPanel.config.main);
this.registerWithFocusTracker(bottomPanel.config.main);
}
if (bottomPanel.size) {
this.bottomPanelState.lastPanelSize = bottomPanel.size;
Expand All @@ -637,7 +637,7 @@ export class ApplicationShell extends Widget {
await this.bottomPanelState.pendingUpdate;
if (mainPanel) {
this.mainPanel.restoreLayout(mainPanel);
await this.registerWithFocusTracker(mainPanel.main);
this.registerWithFocusTracker(mainPanel.main);
}
if (activeWidgetId) {
this.activateWidget(activeWidgetId);
Expand Down Expand Up @@ -679,22 +679,22 @@ export class ApplicationShell extends Widget {
/**
* Track all widgets that are referenced by the given layout data.
*/
protected async registerWithFocusTracker(data: DockLayout.ITabAreaConfig | DockLayout.ISplitAreaConfig | SidePanel.LayoutData | null): Promise<void> {
protected registerWithFocusTracker(data: DockLayout.ITabAreaConfig | DockLayout.ISplitAreaConfig | SidePanel.LayoutData | null): void {
if (data) {
if (data.type === 'tab-area') {
for (const widget of data.widgets) {
if (widget) {
await this.track(widget);
this.track(widget);
}
}
} else if (data.type === 'split-area') {
for (const child of data.children) {
await this.registerWithFocusTracker(child);
this.registerWithFocusTracker(child);
}
} else if (data.type === 'sidepanel' && data.items) {
for (const item of data.items) {
if (item.widget) {
await this.track(item.widget);
this.track(item.widget);
}
}
}
Expand Down Expand Up @@ -759,7 +759,7 @@ export class ApplicationShell extends Widget {
throw new Error('Unexpected area: ' + options.area);
}
if (area !== 'top') {
await this.track(widget);
this.track(widget);
}
}

Expand Down Expand Up @@ -958,16 +958,16 @@ export class ApplicationShell extends Widget {
/**
* Track the given widget so it is considered in the `current` and `active` state of the shell.
*/
protected async track(widget: Widget): Promise<void> {
protected track(widget: Widget): void {
if (this.tracker.widgets.indexOf(widget) !== -1) {
return;
}
this.tracker.add(widget);
this.checkActivation(widget);
Saveable.apply(widget);
if (ApplicationShell.TrackableWidgetProvider.is(widget)) {
for (const toTrack of await widget.getTrackableWidgets()) {
await this.track(toTrack);
for (const toTrack of widget.getTrackableWidgets()) {
this.track(toTrack);
}
if (widget.onDidChangeTrackableWidgets) {
widget.onDidChangeTrackableWidgets(widgets => widgets.forEach(w => this.track(w)));
Expand Down Expand Up @@ -1860,7 +1860,7 @@ export namespace ApplicationShell {
* Exposes widgets which activation state should be tracked by shell.
*/
export interface TrackableWidgetProvider {
getTrackableWidgets(): MaybePromise<Widget[]>
getTrackableWidgets(): Widget[]
readonly onDidChangeTrackableWidgets?: CommonEvent<Widget[]>
/**
* Make visible and focus a trackable widget for the given id.
Expand Down
28 changes: 24 additions & 4 deletions packages/core/src/browser/widget-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { inject, named, injectable } from 'inversify';
import { Widget } from '@phosphor/widgets';
import { ILogger, Emitter, Event, ContributionProvider, MaybePromise } from '../common';
import { ILogger, Emitter, Event, ContributionProvider, MaybePromise, WaitUntilEvent } from '../common';

/* eslint-disable @typescript-eslint/no-explicit-any */
export const WidgetFactory = Symbol('WidgetFactory');
Expand Down Expand Up @@ -53,6 +53,20 @@ export interface WidgetConstructionOptions {
options?: any
}

/**
* Representation of a `willCreateWidgetEvent`.
*/
export interface WillCreateWidgetEvent extends WaitUntilEvent {
/**
* The widget which will be created.
*/
readonly widget: Widget;
/**
* The widget factory id.
*/
readonly factoryId: string;
}

/**
* Representation of a `didCreateWidgetEvent`.
*/
Expand Down Expand Up @@ -84,6 +98,13 @@ export class WidgetManager {
@inject(ILogger)
protected readonly logger: ILogger;

protected readonly onWillCreateWidgetEmitter = new Emitter<WillCreateWidgetEvent>();
/**
* An event can be used to participate in the widget creation.
* Listeners may not dispose the given widget.
*/
readonly onWillCreateWidget: Event<WillCreateWidgetEvent> = this.onWillCreateWidgetEmitter.event;

protected readonly onDidCreateWidgetEmitter = new Emitter<DidCreateWidgetEvent>();
readonly onDidCreateWidget: Event<DidCreateWidgetEvent> = this.onDidCreateWidgetEmitter.event;

Expand Down Expand Up @@ -154,15 +175,14 @@ export class WidgetManager {
const widgetPromise = factory.createWidget(options);
this.pendingWidgetPromises.set(key, widgetPromise);
const widget = await widgetPromise;
await WaitUntilEvent.fire(this.onWillCreateWidgetEmitter, { factoryId, widget });
this.widgetPromises.set(key, widgetPromise);
this.widgets.set(key, widget);
widget.disposed.connect(() => {
this.widgets.delete(key);
this.widgetPromises.delete(key);
});
this.onDidCreateWidgetEmitter.fire({
factoryId, widget
});
this.onDidCreateWidgetEmitter.fire({ factoryId, widget });
return widget as T;
} finally {
this.pendingWidgetPromises.delete(key);
Expand Down
5 changes: 2 additions & 3 deletions packages/editor-preview/src/browser/editor-preview-widget.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,8 @@ export class EditorPreviewWidget extends BaseWidget implements ApplicationShell.
}
}

getTrackableWidgets(): Promise<Widget[]> {
return new Promise(
resolve => resolve(this.editorWidget_ ? [this.editorWidget_] : []));
getTrackableWidgets(): Widget[] {
return this.editorWidget_ ? [this.editorWidget_] : [];
}

storeState(): PreviewViewState {
Expand Down
35 changes: 20 additions & 15 deletions packages/plugin-ext/src/main/browser/view/plugin-view-registry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,22 @@ export class PluginViewRegistry implements FrontendApplicationContribution {
this.updateFocusedView();
this.shell.onDidChangeActiveWidget(() => this.updateFocusedView());

this.widgetManager.onDidCreateWidget(({ factoryId, widget }) => {
this.widgetManager.onWillCreateWidget(({ factoryId, widget, waitUntil }) => {
if (factoryId === EXPLORER_VIEW_CONTAINER_ID && widget instanceof ViewContainerWidget) {
this.prepareViewContainer('explorer', widget);
waitUntil(this.prepareViewContainer('explorer', widget));
}
if (factoryId === SCM_VIEW_CONTAINER_ID && widget instanceof ViewContainerWidget) {
this.prepareViewContainer('scm', widget);
waitUntil(this.prepareViewContainer('scm', widget));
}
if (factoryId === DebugWidget.ID && widget instanceof DebugWidget) {
const viewContainer = widget['sessionWidget']['viewContainer'];
this.prepareViewContainer('debug', viewContainer);
waitUntil(this.prepareViewContainer('debug', viewContainer));
}
if (factoryId === PLUGIN_VIEW_CONTAINER_FACTORY_ID && widget instanceof ViewContainerWidget) {
this.prepareViewContainer(this.toViewContainerId(widget.options), widget);
waitUntil(this.prepareViewContainer(this.toViewContainerId(widget.options), widget));
}
if (factoryId === PLUGIN_VIEW_FACTORY_ID && widget instanceof PluginViewWidget) {
this.prepareView(widget);
waitUntil(this.prepareView(widget));
}
});
this.doRegisterViewContainer('test', 'left', {
Expand Down Expand Up @@ -212,18 +212,18 @@ export class PluginViewRegistry implements FrontendApplicationContribution {
id: toggleCommandId,
label: 'Toggle ' + options.label + ' View'
}, {
execute: async () => {
let widget = await this.getPluginViewContainer(id);
execute: async () => {
let widget = await this.getPluginViewContainer(id);
if (widget) {
widget.dispose();
} else {
widget = await this.openViewContainer(id);
if (widget) {
widget.dispose();
} else {
widget = await this.openViewContainer(id);
if (widget) {
this.shell.activateWidget(widget.id);
}
this.shell.activateWidget(widget.id);
}
}
}));
}
}));
toDispose.push(this.menus.registerMenuAction(CommonMenus.VIEW_VIEWS, {
commandId: toggleCommandId,
label: options.label
Expand Down Expand Up @@ -309,6 +309,11 @@ export class PluginViewRegistry implements FrontendApplicationContribution {
widget.title.label = view.name;
const currentDataWidget = widget.widgets[0];
const viewDataWidget = await this.createViewDataWidget(view.id);
if (widget.disposed) {
// eslint-disable-next-line no-unused-expressions
viewDataWidget?.dispose();
return;
}
if (currentDataWidget !== viewDataWidget) {
if (currentDataWidget) {
currentDataWidget.dispose();
Expand Down

0 comments on commit 5525057

Please sign in to comment.