Properly dispose of newCommentWidget, fixes...

Properly dispose of newCommentWidget, fixes https://github.com/Microsoft/vscode-pull-request-github/issues/235

src/vs/workbench/parts/comments/common/reviewModel.ts

-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Microsoft Corporation. All rights reserved.
- *  Licensed under the MIT License. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-'use strict';
-
-import { Emitter, Event } from 'vs/base/common/event';
-
-export enum ReviewStyle {
-	Complete,
-	Inline,
-	Gutter
-}
-
-export class ReviewModel {
-	private _style: ReviewStyle;
-	public get style(): ReviewStyle { return this._style; }
-	private _onDidChangeStyle = new Emitter<ReviewStyle>();
-	public get onDidChangeStyle(): Event<ReviewStyle> { return this._onDidChangeStyle.event; }
-
-	constructor() {
-		this._style = ReviewStyle.Inline;
-	}
-
-	setStyle(style: ReviewStyle) {
-		this._style = style;
-		this._onDidChangeStyle.fire(this._style);
-	}
-}
\ No newline at end of file

src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts

@@ -97,6 +97,7 @@ export class CommentNode {
 }
 
 let INMEM_MODEL_ID = 0;
+
 export class ReviewZoneWidget extends ZoneWidget {
 	private _headElement: HTMLElement;
 	protected _headingLabel: HTMLElement;
@@ -109,6 +110,7 @@ export class ReviewZoneWidget extends ZoneWidget {
 	private _reviewThreadReplyButton: HTMLElement;
 	private _resizeObserver: any;
 	private _onDidClose = new Emitter<ReviewZoneWidget>();
+	private _onDidCreateThread = new Emitter<ReviewZoneWidget>();
 	private _isCollapsed;
 	private _toggleAction: Action;
 	private _commentThread: modes.CommentThread;
@@ -164,6 +166,10 @@ export class ReviewZoneWidget extends ZoneWidget {
 		return this._onDidClose.event;
 	}
 
+	public get onDidCreateThread(): Event<ReviewZoneWidget> {
+		return this._onDidCreateThread.event;
+	}
+
 	protected revealLine(lineNumber: number) {
 		// we don't do anything here as we always do the reveal ourselves.
 	}
@@ -395,9 +401,9 @@ export class ReviewZoneWidget extends ZoneWidget {
 	private async createComment(lineNumber: number): Promise<void> {
 		try {
 			let newCommentThread;
+			const isReply = this._commentThread.threadId !== null;
 
-			if (this._commentThread.threadId) {
-				// reply
+			if (isReply) {
 				newCommentThread = await this.commentService.replyToCommentThread(
 					this._owner,
 					this.editor.getModel().uri,
@@ -427,6 +433,10 @@ export class ReviewZoneWidget extends ZoneWidget {
 				this._error.textContent = '';
 				dom.addClass(this._error, 'hidden');
 				this.update(newCommentThread);
+
+				if (!isReply) {
+					this._onDidCreateThread.fire(this);
+				}
 			}
 		} catch (e) {
 			this._error.textContent = e.message

src/vs/workbench/parts/comments/electron-browser/commentsEditorContribution.ts

@@ -24,7 +24,6 @@ import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/co
 import { editorForeground, registerColor } from 'vs/platform/theme/common/colorRegistry';
 import { IThemeService, registerThemingParticipant } from 'vs/platform/theme/common/themeService';
 import { CommentThreadCollapsibleState } from 'vs/workbench/api/node/extHostTypes';
-import { ReviewModel } from 'vs/workbench/parts/comments/common/reviewModel';
 import { ReviewZoneWidget, COMMENTEDITOR_DECORATION_KEY } from 'vs/workbench/parts/comments/electron-browser/commentThreadWidget';
 import { ICommentService } from 'vs/workbench/parts/comments/electron-browser/commentService';
 import { IModelService } from 'vs/editor/common/services/modelService';
@@ -169,7 +168,6 @@ export class ReviewController implements IEditorContribution {
 	private _commentWidgets: ReviewZoneWidget[];
 	private _reviewPanelVisible: IContextKey<boolean>;
 	private _commentInfos: modes.CommentInfo[];
-	private _reviewModel: ReviewModel;
 	// private _hasSetComments: boolean;
 	private _commentingRangeDecorator: CommentingRangeDecorator;
 	private mouseDownInfo: { lineNumber: number } | null = null;
@@ -197,28 +195,8 @@ export class ReviewController implements IEditorContribution {
 		// this._hasSetComments = false;
 
 		this._reviewPanelVisible = ctxReviewPanelVisible.bindTo(contextKeyService);
-		this._reviewModel = new ReviewModel();
 		this._commentingRangeDecorator = new CommentingRangeDecorator();
 
-		this._reviewModel.onDidChangeStyle(style => {
-			if (this._newCommentWidget) {
-				this._newCommentWidget.dispose();
-				this._newCommentWidget = null;
-			}
-
-			this._commentWidgets.forEach(zone => {
-				zone.dispose();
-			});
-
-			this._commentInfos.forEach(info => {
-				info.threads.forEach(thread => {
-					let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.editor, info.owner, thread, {});
-					zoneWidget.display(thread.range.startLineNumber, this._commentingRangeDecorator.commentsOptions);
-					this._commentWidgets.push(zoneWidget);
-				});
-			});
-		});
-
 		this.globalToDispose.push(this.commentService.onDidDeleteDataProvider(e => {
 			// Remove new comment widget and glyph, refresh comments
 			if (this._newCommentWidget) {
@@ -352,6 +330,11 @@ export class ReviewController implements IEditorContribution {
 			if (!editorURI) {
 				return;
 			}
+
+			if (!this._commentInfos.some(info => info.owner === e.owner)) {
+				return;
+			}
+
 			let added = e.added.filter(thread => thread.resource.toString() === editorURI.toString());
 			let removed = e.removed.filter(thread => thread.resource.toString() === editorURI.toString());
 			let changed = e.changed.filter(thread => thread.resource.toString() === editorURI.toString());
@@ -409,9 +392,17 @@ export class ReviewController implements IEditorContribution {
 			collapsibleState: CommentThreadCollapsibleState.Expanded,
 		}, {});
 
-		this._newCommentWidget.onDidClose(e => {
+		this.localToDispose.push(this._newCommentWidget.onDidClose(e => {
 			this._newCommentWidget = null;
-		});
+		}));
+
+		this.localToDispose.push(this._newCommentWidget.onDidCreateThread(commentWidget => {
+			const thread = commentWidget.commentThread;
+			this._commentWidgets.push(commentWidget);
+			this._commentInfos.filter(info => info.owner === commentWidget.owner)[0].threads.push(thread);
+			this._newCommentWidget = null;
+		}));
+
 		this._newCommentWidget.display(lineNumber, this._commentingRangeDecorator.commentsOptions);
 	}