Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIx missing progress events #7314

Merged
merged 1 commit into from
Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
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
AlexTugarev marked this conversation as resolved.
Show resolved Hide resolved
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