development/vscode
development/vscode
1
0
Fork 0
mirror of https://github.com/Microsoft/vscode synced 2024-09-29 16:01:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #214589 from rehmsen/notebook_comment_lifecycle

Browse source 
Fix leaking comment thread when CellComment is reused.
This commit is contained in:
Peng Lyu 2024-06-24 14:45:23 -07:00 committed by GitHub
parent 36dc1957f1 52f062f709
commit a5fea0cab6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 48 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
src/vs/workbench/contrib/comments/browser/commentThreadHeader.ts
View file

@ -7,7 +7,7 @@ import * as dom from 'vs/base/browser/dom';
import { ActionBar } from 'vs/base/browser/ui/actionbar/actionbar';
import { Action, ActionRunner } from 'vs/base/common/actions';
import { Codicon } from 'vs/base/common/codicons';
import { Disposable } from 'vs/base/common/lifecycle';
import { Disposable, toDisposable } from 'vs/base/common/lifecycle';
import * as strings from 'vs/base/common/strings';
import * as languages from 'vs/editor/common/languages';
import { IRange } from 'vs/editor/common/core/range';
@ -46,6 +46,7 @@ export class CommentThreadHeader<T = IRange> extends Disposable {
super();
this._headElement = <HTMLDivElement>dom.$('.head');
container.appendChild(this._headElement);
this._register(toDisposable(() => this._headElement.remove()));
this._fillHead();
}

3
src/vs/workbench/contrib/comments/browser/commentThreadWidget.ts
View file

@ -6,7 +6,7 @@
import 'vs/css!./media/review';
import * as dom from 'vs/base/browser/dom';
import { Emitter } from 'vs/base/common/event';
import { Disposable, dispose, IDisposable } from 'vs/base/common/lifecycle';
import { Disposable, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { URI } from 'vs/base/common/uri';
import * as languages from 'vs/editor/common/languages';
import { IMarkdownRendererOptions } from 'vs/editor/browser/widget/markdownRenderer/browser/markdownRenderer';
@ -104,6 +104,7 @@ export class CommentThreadWidget<T extends IRange | ICellRange = IRange> extends
const bodyElement = <HTMLDivElement>dom.$('.body');
container.appendChild(bodyElement);
this._register(toDisposable(() => bodyElement.remove()));
const tracker = this._register(dom.trackFocus(bodyElement));
this._register(registerNavigableContainer({

90
src/vs/workbench/contrib/notebook/browser/view/cellParts/cellComments.ts
View file

@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import { coalesce } from 'vs/base/common/arrays';
import { DisposableStore } from 'vs/base/common/lifecycle';
import { DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle';
import { EDITOR_FONT_DEFAULTS, IEditorOptions } from 'vs/editor/common/config/editorOptions';
import * as languages from 'vs/editor/common/languages';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
@ -20,10 +20,9 @@ import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
export class CellComments extends CellContentPart {
private _initialized: boolean = false;
private _commentThreadWidget: CommentThreadWidget<ICellRange> | null = null;
private readonly _commentThreadWidget = new MutableDisposable<CommentThreadWidget<ICellRange>>;
private currentElement: CodeCellViewModel | undefined;
private readonly commentTheadDisposables = this._register(new DisposableStore());
private readonly _commentThreadDisposables = this._register(new DisposableStore());
constructor(
private readonly notebookEditor: INotebookEditorDelegate,
@ -45,22 +44,17 @@ export class CellComments extends CellContentPart {
}
private async initialize(element: ICellViewModel) {
if (this._initialized) {
if (this.currentElement === element) {
return;
}
this._initialized = true;
const info = await this._getCommentThreadForCell(element);
if (info) {
await this._createCommentTheadWidget(info.owner, info.thread);
}
this.currentElement = element as CodeCellViewModel;
await this._updateThread();
}
private async _createCommentTheadWidget(owner: string, commentThread: languages.CommentThread<ICellRange>) {
this._commentThreadWidget?.dispose();
this.commentTheadDisposables.clear();
this._commentThreadWidget = this.instantiationService.createInstance(
this._commentThreadDisposables.clear();
this._commentThreadWidget.value = this.instantiationService.createInstance(
CommentThreadWidget,
this.container,
this.notebookEditor,
@ -84,44 +78,48 @@ export class CellComments extends CellContentPart {
const layoutInfo = this.notebookEditor.getLayoutInfo();
await this._commentThreadWidget.display(layoutInfo.fontInfo.lineHeight, true);
await this._commentThreadWidget.value.display(layoutInfo.fontInfo.lineHeight, true);
this._applyTheme();
this.commentTheadDisposables.add(this._commentThreadWidget.onDidResize(() => {
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget) {
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
this._commentThreadDisposables.add(this._commentThreadWidget.value.onDidResize(() => {
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget.value) {
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.value.getDimensions().height);
}
}));
}
private _bindListeners() {
this.cellDisposables.add(this.commentService.onDidUpdateCommentThreads(async () => {
if (this.currentElement) {
const info = await this._getCommentThreadForCell(this.currentElement);
if (!this._commentThreadWidget && info) {
await this._createCommentTheadWidget(info.owner, info.thread);
const layoutInfo = (this.currentElement as CodeCellViewModel).layoutInfo;
this.container.style.top = `${layoutInfo.outputContainerOffset + layoutInfo.outputTotalHeight}px`;
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget!.getDimensions().height);
return;
}
this.cellDisposables.add(this.commentService.onDidUpdateCommentThreads(async () => this._updateThread()));
}
if (this._commentThreadWidget) {
if (!info) {
this._commentThreadWidget.dispose();
this.currentElement.commentHeight = 0;
return;
}
if (this._commentThreadWidget.commentThread === info.thread) {
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
return;
}
private async _updateThread() {
if (!this.currentElement) {
return;
}
const info = await this._getCommentThreadForCell(this.currentElement);
if (!this._commentThreadWidget.value && info) {
await this._createCommentTheadWidget(info.owner, info.thread);
const layoutInfo = (this.currentElement as CodeCellViewModel).layoutInfo;
this.container.style.top = `${layoutInfo.outputContainerOffset + layoutInfo.outputTotalHeight}px`;
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.value!.getDimensions().height);
return;
}
await this._commentThreadWidget.updateCommentThread(info.thread);
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
}
if (this._commentThreadWidget.value) {
if (!info) {
this._commentThreadDisposables.clear();
this._commentThreadWidget.dispose();
this.currentElement.commentHeight = 0;
return;
}
}));
if (this._commentThreadWidget.value.commentThread === info.thread) {
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.value.getDimensions().height);
return;
}
await this._commentThreadWidget.value.updateCommentThread(info.thread);
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.value.getDimensions().height);
}
}
private _calculateCommentThreadHeight(bodyHeight: number) {
@ -151,7 +149,7 @@ export class CellComments extends CellContentPart {
private _applyTheme() {
const theme = this.themeService.getColorTheme();
const fontInfo = this.notebookEditor.getLayoutInfo().fontInfo;
this._commentThreadWidget?.applyTheme(theme, fontInfo);
this._commentThreadWidget.value?.applyTheme(theme, fontInfo);
}
override didRenderCell(element: ICellViewModel): void {
@ -164,13 +162,13 @@ export class CellComments extends CellContentPart {
}
override prepareLayout(): void {
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget) {
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.getDimensions().height);
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget.value) {
this.currentElement.commentHeight = this._calculateCommentThreadHeight(this._commentThreadWidget.value.getDimensions().height);
}
}
override updateInternalLayoutNow(element: ICellViewModel): void {
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget) {
if (this.currentElement?.cellKind === CellKind.Code && this._commentThreadWidget.value) {
const layoutInfo = (element as CodeCellViewModel).layoutInfo;
this.container.style.top = `${layoutInfo.outputContainerOffset + layoutInfo.outputTotalHeight}px`;
}