Skip to content

Commit 087985b

Browse files
author
Eric Amodio
committed
Fixes microsoft#91434 - stops timeline update when hidden
1 parent 0562301 commit 087985b

1 file changed

Lines changed: 93 additions & 30 deletions

File tree

‎src/vs/workbench/contrib/timeline/browser/timelinePane.ts‎

Lines changed: 93 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,12 @@ interface TimelineActionContext {
8080

8181
class TimelineAggregate {
8282
readonly items: TimelineItem[];
83+
readonly source: string;
8384

8485
lastRenderedIndex: number;
8586

8687
constructor(timeline: Timeline) {
88+
this.source = timeline.source;
8789
this.items = timeline.items;
8890
this._cursor = timeline.paging?.cursor;
8991
this._more = timeline.paging?.more ?? false;
@@ -162,6 +164,21 @@ class TimelineAggregate {
162164

163165
return updated;
164166
}
167+
168+
private _stale = false;
169+
get stale() {
170+
return this._stale;
171+
}
172+
173+
private _requiresReset = false;
174+
get requiresReset(): boolean {
175+
return this._requiresReset;
176+
}
177+
178+
invalidate(requiresReset: boolean) {
179+
this._stale = true;
180+
this._requiresReset = requiresReset;
181+
}
165182
}
166183

167184
export const TimelineFollowActiveEditorContext = new RawContextKey<boolean>('timelineFollowActiveEditor', true);
@@ -283,15 +300,32 @@ export class TimelinePane extends ViewPane {
283300
if ((uri?.toString(true) === this.uri?.toString(true) && uri !== undefined) ||
284301
// Fallback to match on fsPath if we are dealing with files or git schemes
285302
(uri?.fsPath === this.uri?.fsPath && (uri?.scheme === 'file' || uri?.scheme === 'git') && (this.uri?.scheme === 'file' || this.uri?.scheme === 'git'))) {
303+
304+
// If the uri hasn't changed, make sure we have valid caches
305+
for (const source of this.timelineService.getSources()) {
306+
if (this.excludedSources.has(source.id)) {
307+
continue;
308+
}
309+
310+
const timeline = this.timelinesBySource.get(source.id);
311+
if (timeline !== undefined && !timeline.stale) {
312+
continue;
313+
}
314+
315+
if (timeline !== undefined) {
316+
this.updateTimeline(timeline, timeline.requiresReset);
317+
} else {
318+
this.loadTimelineForSource(source.id, uri, true);
319+
}
320+
}
321+
286322
return;
287323
}
288324

289325
this.setUriCore(uri, false);
290326
}
291327

292328
private onProvidersChanged(e: TimelineProvidersChangeEvent) {
293-
// TODO@eamodio only do work if we are visible
294-
295329
if (e.removed) {
296330
for (const source of e.removed) {
297331
this.timelinesBySource.delete(source);
@@ -306,23 +340,16 @@ export class TimelinePane extends ViewPane {
306340
}
307341

308342
private onTimelineChanged(e: TimelineChangeEvent) {
309-
// TODO@eamodio only do work if we are visible
310-
311343
if (e?.uri === undefined || e.uri.toString(true) !== this.uri?.toString(true)) {
312344
const timeline = this.timelinesBySource.get(e.id);
313345
if (timeline === undefined) {
314346
return;
315347
}
316348

317-
if (e.reset) {
318-
this.timelinesBySource.delete(e.id);
319-
// Override the limit, to re-query for all our existing cached (possibly visible) items to keep visual continuity
320-
const { oldest } = timeline;
321-
this.loadTimelineForSource(e.id, this.uri!, true, oldest !== undefined ? { limit: { timestamp: oldest.timestamp, id: oldest.id } } : undefined);
349+
if (this.isBodyVisible()) {
350+
this.updateTimeline(timeline, e.reset ?? false);
322351
} else {
323-
// Override the limit, to query for any newer items
324-
const { newest } = timeline;
325-
this.loadTimelineForSource(e.id, this.uri!, false, newest !== undefined ? { limit: { timestamp: newest.timestamp, id: newest.id } } : { limit: PageSize });
352+
timeline.invalidate(e.reset ?? false);
326353
}
327354
}
328355
}
@@ -379,29 +406,37 @@ export class TimelinePane extends ViewPane {
379406
return this._visibleItemCount > 0;
380407
}
381408

409+
private clear(cancelPending: boolean) {
410+
this._visibleItemCount = 0;
411+
this._maxItemCount = PageSize;
412+
this.timelinesBySource.clear();
413+
414+
if (cancelPending) {
415+
for (const { tokenSource } of this.pendingRequests.values()) {
416+
tokenSource.dispose(true);
417+
}
418+
419+
this.pendingRequests.clear();
420+
421+
if (!this.isBodyVisible()) {
422+
this.tree.setChildren(null, undefined);
423+
this._isEmpty = true;
424+
}
425+
}
426+
}
427+
382428
private async loadTimeline(reset: boolean, sources?: string[]) {
383429
// If we have no source, we are reseting all sources, so cancel everything in flight and reset caches
384430
if (sources === undefined) {
385431
if (reset) {
386-
this._visibleItemCount = 0;
387-
this._maxItemCount = PageSize;
388-
this.timelinesBySource.clear();
389-
390-
for (const { tokenSource } of this.pendingRequests.values()) {
391-
tokenSource.dispose(true);
392-
}
393-
394-
this.pendingRequests.clear();
432+
this.clear(true);
395433
}
396434

397435
// TODO@eamodio: Are these the right the list of schemes to exclude? Is there a better way?
398436
if (this.uri?.scheme === 'vscode-settings' || this.uri?.scheme === 'webview-panel' || this.uri?.scheme === 'walkThrough') {
399437
this.uri = undefined;
400438

401-
this._visibleItemCount = 0;
402-
this._maxItemCount = PageSize;
403-
this.timelinesBySource.clear();
404-
439+
this.clear(false);
405440
this.refresh();
406441

407442
return;
@@ -413,15 +448,16 @@ export class TimelinePane extends ViewPane {
413448
}
414449

415450
if (this.uri === undefined) {
416-
this._visibleItemCount = 0;
417-
this._maxItemCount = PageSize;
418-
this.timelinesBySource.clear();
419-
451+
this.clear(false);
420452
this.refresh();
421453

422454
return;
423455
}
424456

457+
if (!this.isBodyVisible()) {
458+
return;
459+
}
460+
425461
let hasPendingRequests = false;
426462

427463
for (const source of sources ?? this.timelineService.getSources().map(s => s.id)) {
@@ -490,6 +526,19 @@ export class TimelinePane extends ViewPane {
490526
return true;
491527
}
492528

529+
private updateTimeline(timeline: TimelineAggregate, reset: boolean) {
530+
if (reset) {
531+
this.timelinesBySource.delete(timeline.source);
532+
// Override the limit, to re-query for all our existing cached (possibly visible) items to keep visual continuity
533+
const { oldest } = timeline;
534+
this.loadTimelineForSource(timeline.source, this.uri!, true, oldest !== undefined ? { limit: { timestamp: oldest.timestamp, id: oldest.id } } : undefined);
535+
} else {
536+
// Override the limit, to query for any newer items
537+
const { newest } = timeline;
538+
this.loadTimelineForSource(timeline.source, this.uri!, false, newest !== undefined ? { limit: { timestamp: newest.timestamp, id: newest.id } } : { limit: PageSize });
539+
}
540+
}
541+
493542
private _pendingRefresh = false;
494543

495544
private async handleRequest(request: TimelineRequest) {
@@ -590,7 +639,7 @@ export class TimelinePane extends ViewPane {
590639
for (const [source, timeline] of this.timelinesBySource) {
591640
timeline.lastRenderedIndex = -1;
592641

593-
if (this.excludedSources.has(source)) {
642+
if (this.excludedSources.has(source) || timeline.stale) {
594643
continue;
595644
}
596645

@@ -651,6 +700,10 @@ export class TimelinePane extends ViewPane {
651700
}
652701

653702
private refresh() {
703+
if (!this.isBodyVisible()) {
704+
return;
705+
}
706+
654707
this.tree.setChildren(null, this.getItems() as any);
655708
this._isEmpty = !this.hasVisibleItems;
656709

@@ -682,6 +735,16 @@ export class TimelinePane extends ViewPane {
682735
this.tree.domFocus();
683736
}
684737

738+
setExpanded(expanded: boolean): boolean {
739+
const changed = super.setExpanded(expanded);
740+
741+
if (changed && this.isBodyVisible()) {
742+
this.onActiveEditorChanged();
743+
}
744+
745+
return changed;
746+
}
747+
685748
setVisible(visible: boolean): void {
686749
if (visible) {
687750
this.visibilityDisposables = new DisposableStore();

0 commit comments

Comments
 (0)