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 <alex.tugarev@typefox.io>
  • Loading branch information
AlexTugarev committed Mar 11, 2020
1 parent 924e6a2 commit 7e403e5
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 14 deletions.
6 changes: 3 additions & 3 deletions packages/core/src/browser/progress-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { ProgressLocationEvent } from './progress-location-service';
import { LocationProgress } from './progress-location-service';
import { DisposableCollection, Disposable } from '../common';
import { Event } from '../common/event';

Expand All @@ -27,7 +27,7 @@ export class ProgressBar implements Disposable {

protected progressBar: HTMLDivElement;

constructor(protected options: ProgressBar.Options, onProgress: Event<ProgressLocationEvent>) {
constructor(protected options: ProgressBar.Options, onProgress: Event<LocationProgress>) {
this.progressBar = document.createElement('div');
this.progressBar.className = 'theia-progress-bar';
this.progressBar.style.display = 'none';
Expand All @@ -46,7 +46,7 @@ export class ProgressBar implements Disposable {
]);
}

protected onProgress(event: ProgressLocationEvent): void {
protected onProgress(event: LocationProgress): void {
if (this.toDispose.disposed) {
return;
}
Expand Down
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.only('progress-location-service', () => {

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

it.only('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

0 comments on commit 7e403e5

Please sign in to comment.