Skip to content

Commit 101beaa

Browse files
Eric Amodioeamodio
Eric Amodio
authored and
eamodio
committed
Removes code duplication
Consolidates ViewProgressIndicator into CompositeProgressIndicator
1 parent 690007d commit 101beaa

6 files changed

Lines changed: 49 additions & 216 deletions

File tree

‎src/vs/workbench/browser/parts/compositePart.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ export abstract class CompositePart<T extends Composite> extends Part {
173173
// Instantiate composite from registry otherwise
174174
const compositeDescriptor = this.registry.getComposite(id);
175175
if (compositeDescriptor) {
176-
const compositeProgressIndicator = this.instantiationService.createInstance(CompositeProgressIndicator, assertIsDefined(this.progressBar), compositeDescriptor.id, !!isActive);
176+
const compositeProgressIndicator = this.instantiationService.createInstance(CompositeProgressIndicator, assertIsDefined(this.progressBar), compositeDescriptor.id, !!isActive, undefined);
177177
const compositeInstantiationService = this.instantiationService.createChild(new ServiceCollection(
178178
[IEditorProgressService, compositeProgressIndicator] // provide the editor progress service for any editors instantiated within the composite
179179
));

‎src/vs/workbench/browser/parts/views/viewPaneContainer.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import { Link } from 'vs/platform/opener/browser/link';
4545
import { LocalSelectionTransfer } from 'vs/workbench/browser/dnd';
4646
import { Orientation } from 'vs/base/browser/ui/sash/sash';
4747
import { ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar';
48-
import { ViewProgressIndicator } from 'vs/workbench/services/progress/browser/progressIndicator';
48+
import { CompositeProgressIndicator } from 'vs/workbench/services/progress/browser/progressIndicator';
4949
import { IProgressIndicator } from 'vs/platform/progress/common/progress';
5050

5151
export interface IPaneColors extends IColorMapping {
@@ -345,7 +345,7 @@ export abstract class ViewPane extends Pane implements IView {
345345
}
346346

347347
if (this.progressIndicator === undefined) {
348-
this.progressIndicator = this.instantiationService.createInstance(ViewProgressIndicator, this, assertIsDefined(this.progressBar));
348+
this.progressIndicator = this.instantiationService.createInstance(CompositeProgressIndicator, assertIsDefined(this.progressBar), this.id, this.isVisible(), { hideWhenInactive: true });
349349
}
350350
return this.progressIndicator;
351351
}

‎src/vs/workbench/services/progress/browser/progressIndicator.ts‎

Lines changed: 14 additions & 162 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet';
1010
import { IPanelService } from 'vs/workbench/services/panel/common/panelService';
1111
import { IProgressRunner, IProgressIndicator, emptyProgressRunner } from 'vs/platform/progress/common/progress';
1212
import { IEditorGroupView } from 'vs/workbench/browser/parts/editor/editor';
13-
import { ViewPane } from 'vs/workbench/browser/parts/views/viewPaneContainer';
13+
import { IViewsService } from 'vs/workbench/common/views';
1414

1515
export class ProgressBarIndicator extends Disposable implements IProgressIndicator {
1616

@@ -154,6 +154,7 @@ export abstract class CompositeScope extends Disposable {
154154
constructor(
155155
private viewletService: IViewletService,
156156
private panelService: IPanelService,
157+
private viewsService: IViewsService,
157158
private scopeId: string
158159
) {
159160
super();
@@ -162,6 +163,8 @@ export abstract class CompositeScope extends Disposable {
162163
}
163164

164165
registerListeners(): void {
166+
this._register(this.viewsService.onDidChangeViewVisibility(e => e.visible ? this.onScopeOpened(e.id) : this.onScopeClosed(e.id)));
167+
165168
this._register(this.viewletService.onDidViewletOpen(viewlet => this.onScopeOpened(viewlet.getId())));
166169
this._register(this.panelService.onDidPanelOpen(({ panel }) => this.onScopeOpened(panel.getId())));
167170

@@ -195,17 +198,23 @@ export class CompositeProgressIndicator extends CompositeScope implements IProgr
195198
progressbar: ProgressBar,
196199
scopeId: string,
197200
isActive: boolean,
201+
private readonly options: { hideWhenInactive?: boolean } | undefined,
198202
@IViewletService viewletService: IViewletService,
199-
@IPanelService panelService: IPanelService
203+
@IPanelService panelService: IPanelService,
204+
@IViewsService viewsService: IViewsService
200205
) {
201-
super(viewletService, panelService, scopeId);
206+
super(viewletService, panelService, viewsService, scopeId);
202207

203208
this.progressbar = progressbar;
204209
this.isActive = isActive || isUndefinedOrNull(scopeId); // If service is unscoped, enable by default
205210
}
206211

207212
onScopeDeactivated(): void {
208213
this.isActive = false;
214+
215+
if (this.options?.hideWhenInactive) {
216+
this.progressbar.stop().hide();
217+
}
209218
}
210219

211220
onScopeActivated(): void {
@@ -305,7 +314,7 @@ export class CompositeProgressIndicator extends CompositeScope implements IProgr
305314
done: () => {
306315
this.progressState = ProgressIndicatorState.Done;
307316

308-
if (this.isActive) {
317+
if (this.isActive || this.options?.hideWhenInactive) {
309318
this.progressbar.stop().hide();
310319
}
311320
}
@@ -336,7 +345,7 @@ export class CompositeProgressIndicator extends CompositeScope implements IProgr
336345
// The while promise is either null or equal the promise we last hooked on
337346
this.progressState = ProgressIndicatorState.None;
338347

339-
if (this.isActive) {
348+
if (this.isActive || this.options?.hideWhenInactive) {
340349
this.progressbar.stop().hide();
341350
}
342351
}
@@ -351,160 +360,3 @@ export class CompositeProgressIndicator extends CompositeScope implements IProgr
351360
}
352361
}
353362
}
354-
355-
export class ViewProgressIndicator extends Disposable implements IProgressIndicator {
356-
private progressState: ProgressIndicatorState.State = ProgressIndicatorState.None;
357-
358-
private isVisible: boolean;
359-
360-
constructor(
361-
view: ViewPane,
362-
private progressbar: ProgressBar,
363-
) {
364-
super();
365-
366-
this.progressbar = progressbar;
367-
this._register(view.onDidChangeBodyVisibility(this.onVisible, this));
368-
this.isVisible = view.isVisible();
369-
}
370-
371-
onVisible(visible: boolean) {
372-
this.isVisible = visible;
373-
374-
// Return early if progress state indicates that progress is done
375-
if (!visible || this.progressState.type === ProgressIndicatorState.Done.type) {
376-
this.progressbar.stop().hide();
377-
return;
378-
}
379-
380-
// Replay Infinite Progress from Promise
381-
if (this.progressState.type === ProgressIndicatorState.Type.While) {
382-
let delay: number | undefined;
383-
if (this.progressState.whileDelay > 0) {
384-
const remainingDelay = this.progressState.whileDelay - (Date.now() - this.progressState.whileStart);
385-
if (remainingDelay > 0) {
386-
delay = remainingDelay;
387-
}
388-
}
389-
390-
this.doShowWhile(delay);
391-
}
392-
393-
// Replay Infinite Progress
394-
else if (this.progressState.type === ProgressIndicatorState.Type.Infinite) {
395-
this.progressbar.infinite().show();
396-
}
397-
398-
// Replay Finite Progress (Total & Worked)
399-
else if (this.progressState.type === ProgressIndicatorState.Type.Work) {
400-
if (this.progressState.total) {
401-
this.progressbar.total(this.progressState.total).show();
402-
}
403-
404-
if (this.progressState.worked) {
405-
this.progressbar.worked(this.progressState.worked).show();
406-
}
407-
}
408-
}
409-
410-
show(infinite: true, delay?: number): IProgressRunner;
411-
show(total: number, delay?: number): IProgressRunner;
412-
show(infiniteOrTotal: true | number, delay?: number): IProgressRunner {
413-
414-
// Sort out Arguments
415-
if (typeof infiniteOrTotal === 'boolean') {
416-
this.progressState = ProgressIndicatorState.Infinite;
417-
} else {
418-
this.progressState = new ProgressIndicatorState.Work(infiniteOrTotal, undefined);
419-
}
420-
421-
// Active: Show Progress
422-
if (this.isVisible) {
423-
424-
// Infinite: Start Progressbar and Show after Delay
425-
if (this.progressState.type === ProgressIndicatorState.Type.Infinite) {
426-
this.progressbar.infinite().show(delay);
427-
}
428-
429-
// Finite: Start Progressbar and Show after Delay
430-
else if (this.progressState.type === ProgressIndicatorState.Type.Work && typeof this.progressState.total === 'number') {
431-
this.progressbar.total(this.progressState.total).show(delay);
432-
}
433-
}
434-
435-
return {
436-
total: (total: number) => {
437-
this.progressState = new ProgressIndicatorState.Work(
438-
total,
439-
this.progressState.type === ProgressIndicatorState.Type.Work ? this.progressState.worked : undefined);
440-
441-
if (this.isVisible) {
442-
this.progressbar.total(total);
443-
}
444-
},
445-
446-
worked: (worked: number) => {
447-
448-
// Verify first that we are either not active or the progressbar has a total set
449-
if (!this.isVisible || this.progressbar.hasTotal()) {
450-
this.progressState = new ProgressIndicatorState.Work(
451-
this.progressState.type === ProgressIndicatorState.Type.Work ? this.progressState.total : undefined,
452-
this.progressState.type === ProgressIndicatorState.Type.Work && typeof this.progressState.worked === 'number' ? this.progressState.worked + worked : worked);
453-
454-
if (this.isVisible) {
455-
this.progressbar.worked(worked);
456-
}
457-
}
458-
459-
// Otherwise the progress bar does not support worked(), we fallback to infinite() progress
460-
else {
461-
this.progressState = ProgressIndicatorState.Infinite;
462-
this.progressbar.infinite().show();
463-
}
464-
},
465-
466-
done: () => {
467-
this.progressState = ProgressIndicatorState.Done;
468-
469-
this.progressbar.stop().hide();
470-
}
471-
};
472-
}
473-
474-
async showWhile(promise: Promise<unknown>, delay?: number): Promise<void> {
475-
476-
// Join with existing running promise to ensure progress is accurate
477-
if (this.progressState.type === ProgressIndicatorState.Type.While) {
478-
promise = Promise.all([promise, this.progressState.whilePromise]);
479-
}
480-
481-
// Keep Promise in State
482-
this.progressState = new ProgressIndicatorState.While(promise, delay || 0, Date.now());
483-
484-
try {
485-
this.doShowWhile(delay);
486-
487-
await promise;
488-
} catch (error) {
489-
// ignore
490-
} finally {
491-
492-
// If this is not the last promise in the list of joined promises, skip this
493-
if (this.progressState.type !== ProgressIndicatorState.Type.While || this.progressState.whilePromise === promise) {
494-
495-
// The while promise is either null or equal the promise we last hooked on
496-
this.progressState = ProgressIndicatorState.None;
497-
498-
this.progressbar.stop().hide();
499-
}
500-
}
501-
}
502-
503-
private doShowWhile(delay?: number): void {
504-
505-
// Show Progress when active
506-
if (this.isVisible) {
507-
this.progressbar.infinite().show(delay);
508-
}
509-
}
510-
}

‎src/vs/workbench/services/progress/browser/progressService.ts‎

Lines changed: 12 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -383,38 +383,8 @@ export class ProgressService extends Disposable implements IProgressService {
383383
// show in viewlet
384384
const promise = this.withCompositeProgress(this.viewletService.getProgressIndicator(viewletId), task, options);
385385

386-
// show activity bar
387-
let activityProgress: IDisposable;
388-
let delayHandle: any = setTimeout(() => {
389-
delayHandle = undefined;
390-
391-
const handle = this.activityService.showActivity(
392-
viewletId,
393-
new ProgressBadge(() => ''),
394-
'progress-badge',
395-
100
396-
);
397-
398-
const startTimeVisible = Date.now();
399-
const minTimeVisible = 300;
400-
activityProgress = {
401-
dispose() {
402-
const d = Date.now() - startTimeVisible;
403-
if (d < minTimeVisible) {
404-
// should at least show for Nms
405-
setTimeout(() => handle.dispose(), minTimeVisible - d);
406-
} else {
407-
// shown long enough
408-
handle.dispose();
409-
}
410-
}
411-
};
412-
}, options.delay || 300);
413-
414-
promise.finally(() => {
415-
clearTimeout(delayHandle);
416-
dispose(activityProgress);
417-
});
386+
// show on activity bar
387+
this.showOnActivityBar<P, R>(viewletId, options, promise);
418388

419389
return promise;
420390
}
@@ -434,18 +404,17 @@ export class ProgressService extends Disposable implements IProgressService {
434404
return promise;
435405
}
436406

437-
// show activity bar
407+
// show on activity bar
408+
this.showOnActivityBar(viewletId, options, promise);
409+
410+
return promise;
411+
}
412+
413+
private showOnActivityBar<P extends Promise<R>, R = unknown>(viewletId: string, options: IProgressCompositeOptions, promise: P) {
438414
let activityProgress: IDisposable;
439415
let delayHandle: any = setTimeout(() => {
440416
delayHandle = undefined;
441-
442-
const handle = this.activityService.showActivity(
443-
viewletId,
444-
new ProgressBadge(() => ''),
445-
'progress-badge',
446-
100
447-
);
448-
417+
const handle = this.activityService.showActivity(viewletId, new ProgressBadge(() => ''), 'progress-badge', 100);
449418
const startTimeVisible = Date.now();
450419
const minTimeVisible = 300;
451420
activityProgress = {
@@ -454,20 +423,18 @@ export class ProgressService extends Disposable implements IProgressService {
454423
if (d < minTimeVisible) {
455424
// should at least show for Nms
456425
setTimeout(() => handle.dispose(), minTimeVisible - d);
457-
} else {
426+
}
427+
else {
458428
// shown long enough
459429
handle.dispose();
460430
}
461431
}
462432
};
463433
}, options.delay || 300);
464-
465434
promise.finally(() => {
466435
clearTimeout(delayHandle);
467436
dispose(activityProgress);
468437
});
469-
470-
return promise;
471438
}
472439

473440
private withPanelProgress<P extends Promise<R>, R = unknown>(panelid: string, task: (progress: IProgress<IProgressStep>) => P, options: IProgressCompositeOptions): P {

‎src/vs/workbench/services/progress/test/browser/progressIndicator.test.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import { CompositeScope, CompositeProgressIndicator } from 'vs/workbench/service
1010
import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet';
1111
import { IPanelService } from 'vs/workbench/services/panel/common/panelService';
1212
import { IViewlet } from 'vs/workbench/common/viewlet';
13-
import { TestViewletService, TestPanelService } from 'vs/workbench/test/browser/workbenchTestServices';
13+
import { TestViewletService, TestPanelService, TestViewsService } from 'vs/workbench/test/browser/workbenchTestServices';
1414
import { Event } from 'vs/base/common/event';
15-
import { IView, IViewPaneContainer } from 'vs/workbench/common/views';
15+
import { IView, IViewPaneContainer, IViewsService } from 'vs/workbench/common/views';
1616

1717
class TestViewlet implements IViewlet {
1818

@@ -38,8 +38,8 @@ class TestViewlet implements IViewlet {
3838
class TestCompositeScope extends CompositeScope {
3939
isActive: boolean = false;
4040

41-
constructor(viewletService: IViewletService, panelService: IPanelService, scopeId: string) {
42-
super(viewletService, panelService, scopeId);
41+
constructor(viewletService: IViewletService, panelService: IPanelService, viewsService: IViewsService, scopeId: string) {
42+
super(viewletService, panelService, viewsService, scopeId);
4343
}
4444

4545
onScopeActivated() { this.isActive = true; }
@@ -106,7 +106,8 @@ suite('Progress Indicator', () => {
106106
test('CompositeScope', () => {
107107
let viewletService = new TestViewletService();
108108
let panelService = new TestPanelService();
109-
let service = new TestCompositeScope(viewletService, panelService, 'test.scopeId');
109+
let viewsService = new TestViewsService();
110+
let service = new TestCompositeScope(viewletService, panelService, viewsService, 'test.scopeId');
110111
const testViewlet = new TestViewlet('test.scopeId');
111112

112113
assert(!service.isActive);
@@ -122,7 +123,8 @@ suite('Progress Indicator', () => {
122123
let testProgressBar = new TestProgressBar();
123124
let viewletService = new TestViewletService();
124125
let panelService = new TestPanelService();
125-
let service = new CompositeProgressIndicator((<any>testProgressBar), 'test.scopeId', true, viewletService, panelService);
126+
let viewsService = new TestViewsService();
127+
let service = new CompositeProgressIndicator((<any>testProgressBar), 'test.scopeId', true, undefined, viewletService, panelService, viewsService);
126128

127129
// Active: Show (Infinite)
128130
let fn = service.show(true);

0 commit comments

Comments
 (0)