Skip to content

Commit

Permalink
Fix missing progress events
Browse files Browse the repository at this point in the history
Fixes #7311

Signed-off-by: Alex Tugarev <[email protected]>
  • Loading branch information
AlexTugarev committed Mar 11, 2020
1 parent 924e6a2 commit 211e161
Show file tree
Hide file tree
Showing 9 changed files with 154 additions and 55 deletions.
8 changes: 8 additions & 0 deletions packages/core/src/browser/frontend-application-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ import { ExternalUriService } from './external-uri-service';
import { IconThemeService, NoneIconTheme } from './icon-theme-service';
import { IconThemeApplicationContribution, IconThemeContribution, DefaultFileIconThemeContribution } from './icon-theme-contribution';
import { TreeLabelProvider } from './tree/tree-label-provider';
import { ProgressBar } from './progress-bar';
import { ProgressBarFactory, ProgressBarOptions } from './progress-bar-factory';

export { bindResourceProvider, bindMessageService, bindPreferenceService };

Expand Down Expand Up @@ -308,6 +310,12 @@ export const frontendApplicationModule = new ContainerModule((bind, unbind, isBo
bind(ProgressStatusBarItem).toSelf().inSingletonScope();
bind(ProgressClient).toService(DispatchingProgressClient);
bind(ProgressService).toSelf().inSingletonScope();
bind(ProgressBarFactory).toFactory(context => (options: ProgressBarOptions) => {
const childContainer = context.container.createChild();
childContainer.bind(ProgressBarOptions).toConstantValue(options);
childContainer.bind(ProgressBar).toSelf().inSingletonScope();
return childContainer.get(ProgressBar);
});

bind(ContextMenuContext).toSelf().inSingletonScope();
});
29 changes: 29 additions & 0 deletions packages/core/src/browser/progress-bar-factory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/********************************************************************************
* Copyright (C) 2019 TypeFox and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { ProgressBar } from './progress-bar';

export const ProgressBarFactory = Symbol('ProgressBarFactory');
export interface ProgressBarFactory {
(options: ProgressBarOptions): ProgressBar;
}

export const ProgressBarOptions = Symbol('ProgressBarOptions');
export interface ProgressBarOptions {

This comment has been minimized.

Copy link
@akosyakov

akosyakov Mar 11, 2020

Member

I think most important that declarations come before inject, so you should be able to put them at the top of progress-bar file without moving to another file.

This comment has been minimized.

Copy link
@AlexTugarev

AlexTugarev Mar 11, 2020

Author Contributor

I tried, it failed to start :-(
Should I retry, or is this ok as well?

locationId: string;
container: HTMLElement;
insertMode: 'append' | 'prepend';
}
50 changes: 30 additions & 20 deletions packages/core/src/browser/progress-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,39 +14,55 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { ProgressLocationEvent } from './progress-location-service';
import { LocationProgress, ProgressLocationService } from './progress-location-service';
import { DisposableCollection, Disposable } from '../common';
import { Event } from '../common/event';
import { injectable, inject, postConstruct } from 'inversify';
import { ProgressBarOptions } from './progress-bar-factory';

@injectable()
export class ProgressBar implements Disposable {

@inject(ProgressLocationService)
protected readonly progressLocationService: ProgressLocationService;

@inject(ProgressBarOptions)
protected readonly options: ProgressBarOptions;

protected readonly toDispose = new DisposableCollection();
dispose(): void {
this.toDispose.dispose();
}

protected progressBar: HTMLDivElement;
protected progressBarContainer: HTMLDivElement;

constructor(protected options: ProgressBar.Options, onProgress: Event<ProgressLocationEvent>) {
constructor() {
this.progressBar = document.createElement('div');
this.progressBar.className = 'theia-progress-bar';
this.progressBar.style.display = 'none';
const progressBarContainer = document.createElement('div');
progressBarContainer.className = 'theia-progress-bar-container';
progressBarContainer.append(this.progressBar);
const { container, insertMode } = this.options;
this.progressBarContainer = document.createElement('div');
this.progressBarContainer.className = 'theia-progress-bar-container';
this.progressBarContainer.append(this.progressBar);
}

@postConstruct()
protected init(): void {
const { container, insertMode, locationId } = this.options;
if (insertMode === 'prepend') {
container.prepend(progressBarContainer);
container.prepend(this.progressBarContainer);
} else {
container.append(progressBarContainer);
container.append(this.progressBarContainer);
}
this.toDispose.push(Disposable.create(() => this.progressBarContainer.remove()));
const onProgress = this.progressLocationService.onProgress(locationId);
this.toDispose.push(onProgress(event => this.onProgress(event)));
const current = this.progressLocationService.getProgress(locationId);
if (current) {
this.onProgress(current);
}
this.toDispose.pushAll([
Disposable.create(() => progressBarContainer.remove()),
onProgress(event => this.onProgress(event))
]);
}

protected onProgress(event: ProgressLocationEvent): void {
protected onProgress(event: LocationProgress): void {
if (this.toDispose.disposed) {
return;
}
Expand All @@ -58,9 +74,3 @@ export class ProgressBar implements Disposable {
}

}
export namespace ProgressBar {
export interface Options {
container: HTMLElement;
insertMode: 'append' | 'prepend';
}
}
50 changes: 50 additions & 0 deletions packages/core/src/browser/progress-location-service.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/********************************************************************************
* Copyright (C) 2020 TypeFox and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { expect } from 'chai';
import { ProgressLocationService } from './progress-location-service';
import { CancellationTokenSource } from '../common';

describe('progress-location-service', () => {

const EXP = 'exp';
const SCM = 'scm';
const FOO = 'foo';

it('done event should be fired for multiple progress locations – bug #7311', async () => {
const eventLog = new Array<string>();
const logEvent = (location: string, show: boolean) => eventLog.push(`${location} show: ${show}`);

const service = new ProgressLocationService();

[EXP, SCM, FOO].map(location => {
service.onProgress(location)(e => logEvent(location, e.show));
const progressToken = new CancellationTokenSource();
service.showProgress(`progress-${location}-${Date.now()}`, { text: '', options: { location } }, progressToken.token);
return progressToken;
}).forEach(t => t.cancel());

expect(eventLog.join('\n')).eq(`
exp show: true
scm show: true
foo show: true
exp show: false
scm show: false
foo show: false
`.trim());
});

});
32 changes: 21 additions & 11 deletions packages/core/src/browser/progress-location-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,26 @@ import { ProgressClient } from '../common/progress-service-protocol';
import { ProgressMessage, ProgressUpdate } from '../common/message-service-protocol';
import { Deferred } from '../common/promise-util';
import { Event, Emitter } from '../common/event';
import throttle = require('lodash.throttle');

export interface ProgressLocationEvent {
message?: string;
export interface LocationProgress {
show: boolean;
}

@injectable()
export class ProgressLocationService implements ProgressClient {

protected emitters = new Map<string, Emitter<ProgressLocationEvent>[]>();
protected emitters = new Map<string, Emitter<LocationProgress>[]>();
protected lastEvents = new Map<string, LocationProgress>();

onProgress(locationId: string): Event<ProgressLocationEvent> {
getProgress(locationId: string): LocationProgress | undefined {
return this.lastEvents.get(locationId);
}
onProgress(locationId: string): Event<LocationProgress> {
const emitter = this.addEmitter(locationId);
return emitter.event;
}
protected addEmitter(locationId: string): Emitter<ProgressLocationEvent> {
const emitter = new Emitter<ProgressLocationEvent>();
protected addEmitter(locationId: string): Emitter<LocationProgress> {
const emitter = new Emitter<LocationProgress>();
const list = this.emitters.get(locationId) || [];
list.push(emitter);
this.emitters.set(locationId, list);
Expand Down Expand Up @@ -67,13 +69,21 @@ export class ProgressLocationService implements ProgressClient {
const show = !!progressSet.size;
this.fireEvent(locationId, show);
}
protected readonly fireEvent = throttle((locationId: string, show: boolean) => {
protected fireEvent(locationId: string, show: boolean): void {
const lastEvent = this.lastEvents.get(locationId);
const shouldFire = !lastEvent || lastEvent.show !== show;
if (shouldFire) {
this.lastEvents.set(locationId, { show });
this.getOrCreateEmitters(locationId).forEach(e => e.fire({ show }));
}
}
protected getOrCreateEmitters(locationId: string): Emitter<LocationProgress>[] {
let emitters = this.emitters.get(locationId);
if (!emitters) {
emitters = [ this.addEmitter(locationId) ];
emitters = [this.addEmitter(locationId)];
}
emitters.forEach(e => e.fire({ show }));
}, 250);
return emitters;
}

protected getLocationId(message: ProgressMessage): string {
return message.options && message.options.location || 'unknownLocation';
Expand Down
10 changes: 4 additions & 6 deletions packages/core/src/browser/view-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ import { parseCssMagnitude } from './browser';
import { WidgetManager } from './widget-manager';
import { TabBarToolbarRegistry, TabBarToolbarFactory, TabBarToolbar } from './shell/tab-bar-toolbar';
import { Key } from './keys';
import { ProgressLocationService } from './progress-location-service';
import { ProgressBar } from './progress-bar';
import { ProgressBarFactory } from './progress-bar-factory';

export interface ViewContainerTitleOptions {
label: string;
Expand Down Expand Up @@ -92,8 +91,8 @@ export class ViewContainer extends BaseWidget implements StatefulWidget, Applica
protected readonly onDidChangeTrackableWidgetsEmitter = new Emitter<Widget[]>();
readonly onDidChangeTrackableWidgets = this.onDidChangeTrackableWidgetsEmitter.event;

@inject(ProgressLocationService)
protected readonly progressLocationService: ProgressLocationService;
@inject(ProgressBarFactory)
protected readonly progressBarFactory: ProgressBarFactory;

@postConstruct()
protected init(): void {
Expand Down Expand Up @@ -145,8 +144,7 @@ export class ViewContainer extends BaseWidget implements StatefulWidget, Applica
this.onDidChangeTrackableWidgetsEmitter
]);
if (this.options.progressLocationId) {
const onProgress = this.progressLocationService.onProgress(this.options.progressLocationId);
this.toDispose.push(new ProgressBar({ container: this.node, insertMode: 'prepend' }, onProgress));
this.toDispose.push(this.progressBarFactory({ container: this.node, insertMode: 'prepend', locationId: this.options.progressLocationId }));
}
}

Expand Down
10 changes: 4 additions & 6 deletions packages/debug/src/browser/view/debug-widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ import { DebugSessionWidget } from './debug-session-widget';
import { DebugConfigurationWidget } from './debug-configuration-widget';
import { DebugViewModel } from './debug-view-model';
import { DebugSessionManager } from '../debug-session-manager';
import { ProgressLocationService } from '@theia/core/lib/browser/progress-location-service';
import { ProgressBar } from '@theia/core/lib/browser/progress-bar';
import { ProgressBarFactory } from '@theia/core/lib/browser/progress-bar-factory';

@injectable()
export class DebugWidget extends BaseWidget implements StatefulWidget, ApplicationShell.TrackableWidgetProvider {
Expand Down Expand Up @@ -53,8 +52,8 @@ export class DebugWidget extends BaseWidget implements StatefulWidget, Applicati
@inject(DebugSessionWidget)
protected readonly sessionWidget: DebugSessionWidget;

@inject(ProgressLocationService)
protected readonly progressLocationService: ProgressLocationService;
@inject(ProgressBarFactory)
protected readonly progressBarFactory: ProgressBarFactory;

@postConstruct()
protected init(): void {
Expand All @@ -78,8 +77,7 @@ export class DebugWidget extends BaseWidget implements StatefulWidget, Applicati
layout.addWidget(this.toolbar);
layout.addWidget(this.sessionWidget);

const onProgress = this.progressLocationService.onProgress('debug');
this.toDispose.push(new ProgressBar({ container: this.node, insertMode: 'prepend' }, onProgress));
this.toDispose.push(this.progressBarFactory({ container: this.node, insertMode: 'prepend', locationId: 'debug' }));
}

protected onActivateRequest(msg: Message): void {
Expand Down
10 changes: 4 additions & 6 deletions packages/plugin-ext/src/main/browser/plugin-ext-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ import { PluginMetadata } from '../../common/plugin-protocol';
import { ReactWidget } from '@theia/core/lib/browser/widgets/react-widget';
import { AlertMessage } from '@theia/core/lib/browser/widgets/alert-message';
import { HostedPluginSupport, PluginProgressLocation } from '../../hosted/browser/hosted-plugin';
import { ProgressLocationService } from '@theia/core/lib/browser/progress-location-service';
import { ProgressBar } from '@theia/core/lib/browser/progress-bar';
import { ProgressBarFactory } from '@theia/core/lib/browser/progress-bar-factory';
import { DisposableCollection } from '@theia/core/lib/common/disposable';

@injectable()
Expand All @@ -31,8 +30,8 @@ export class PluginWidget extends ReactWidget {
@inject(HostedPluginSupport)
protected readonly pluginService: HostedPluginSupport;

@inject(ProgressLocationService)
protected readonly progressLocationService: ProgressLocationService;
@inject(ProgressBarFactory)
protected readonly progressBarFactory: ProgressBarFactory;

constructor() {
super();
Expand Down Expand Up @@ -64,8 +63,7 @@ export class PluginWidget extends ReactWidget {
this.toDisposeProgress.dispose();
this.toDispose.push(this.toDisposeProgress);
if (ref) {
const onProgress = this.progressLocationService.onProgress(PluginProgressLocation);
this.toDisposeProgress.push(new ProgressBar({ container: ref, insertMode: 'prepend' }, onProgress));
this.toDispose.push(this.progressBarFactory({ container: this.node, insertMode: 'prepend', locationId: PluginProgressLocation }));
}
}}>{this.doRender()}</div>;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ import * as ReactDOM from 'react-dom';
import { Event, Emitter, Disposable } from '@theia/core/lib/common';
import { WorkspaceService } from '@theia/workspace/lib/browser';
import { SearchInWorkspaceContextKeyService } from './search-in-workspace-context-key-service';
import { ProgressLocationService } from '@theia/core/lib/browser/progress-location-service';
import { ProgressBar } from '@theia/core/lib/browser/progress-bar';
import { CancellationTokenSource } from '@theia/core';
import { ProgressBarFactory } from '@theia/core/lib/browser/progress-bar-factory';

export interface SearchFieldState {
className: string;
Expand Down Expand Up @@ -84,8 +83,8 @@ export class SearchInWorkspaceWidget extends BaseWidget implements StatefulWidge
@inject(SearchInWorkspaceContextKeyService)
protected readonly contextKeyService: SearchInWorkspaceContextKeyService;

@inject(ProgressLocationService)
protected readonly progressLocationService: ProgressLocationService;
@inject(ProgressBarFactory)
protected readonly progressBarFactory: ProgressBarFactory;

@postConstruct()
protected init(): void {
Expand Down Expand Up @@ -146,8 +145,7 @@ export class SearchInWorkspaceWidget extends BaseWidget implements StatefulWidge

this.toDispose.push(this.resultTreeWidget);

const onProgress = this.progressLocationService.onProgress('search');
this.toDispose.push(new ProgressBar({ container: this.node, insertMode: 'prepend' }, onProgress));
this.toDispose.push(this.progressBarFactory({ container: this.node, insertMode: 'prepend', locationId: 'search' }));
}

storeState(): object {
Expand Down

0 comments on commit 211e161

Please sign in to comment.