Fix two bugs in #214589 fixing #213535. (#218357)

* Fix two bugs in #214589.

1. We must not `dispose()` the `MutableDisposable` `this._commentThreadWidget` - as that disposes the MutableDisposable itself, and it cannot then later be reassigned to a new value. Instead, we need to assign `value=undefined`, which disposes the previous `value`, but keeps the `MutableDisposable` available to be reused later.
2. `initialize()` is a no-op if `this.currentElement` is already identical to the passed `element`, so we must not do that assignment before calling initialize - instead `initialize()` does the assignment after checking.

* Fix blank line.

* Register the _commentThreadWidget MutableDisposable so that it gets disposed when CellComments is disposed.
This commit is contained in:
Ole 2024-06-26 18:10:56 +02:00 committed by GitHub
parent 720c89311e
commit a11a0d7edc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -20,14 +20,13 @@ import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
export class CellComments extends CellContentPart {
private readonly _commentThreadWidget = new MutableDisposable<CommentThreadWidget<ICellRange>>;
private readonly _commentThreadWidget: MutableDisposable<CommentThreadWidget<ICellRange>>;
private currentElement: CodeCellViewModel | undefined;
private readonly _commentThreadDisposables = this._register(new DisposableStore());
constructor(
private readonly notebookEditor: INotebookEditorDelegate,
private readonly container: HTMLElement,
@IContextKeyService private readonly contextKeyService: IContextKeyService,
@IThemeService private readonly themeService: IThemeService,
@ICommentService private readonly commentService: ICommentService,
@ -37,6 +36,8 @@ export class CellComments extends CellContentPart {
super();
this.container.classList.add('review-widget');
this._register(this._commentThreadWidget = new MutableDisposable<CommentThreadWidget<ICellRange>>());
this._register(this.themeService.onDidColorThemeChange(this._applyTheme, this));
// TODO @rebornix onDidChangeLayout (font change)
// this._register(this.notebookEditor.onDidchangeLa)
@ -108,7 +109,7 @@ export class CellComments extends CellContentPart {
if (this._commentThreadWidget.value) {
if (!info) {
this._commentThreadDisposables.clear();
this._commentThreadWidget.dispose();
this._commentThreadWidget.value = undefined;
this.currentElement.commentHeight = 0;
return;
}
@ -154,7 +155,6 @@ export class CellComments extends CellContentPart {
override didRenderCell(element: ICellViewModel): void {
if (element.cellKind === CellKind.Code) {
this.currentElement = element as CodeCellViewModel;
this.initialize(element);
this._bindListeners();
}