diff --git a/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts b/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts index e06e3c0ea214d8f74024a43ec4d09286b9bf1c4f..1ee06237e2ea963ad2c50c9bf9c7889d0c17b7c5 100644 --- a/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts +++ b/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts @@ -108,7 +108,6 @@ export class ReviewZoneWidget extends ZoneWidget { private _commentThread: modes.CommentThread; private _commentGlyph: CommentGlyphWidget; private _owner: number; - private _decorationIDs: string[]; private _localToDispose: IDisposable[]; public get owner(): number { @@ -134,7 +133,6 @@ export class ReviewZoneWidget extends ZoneWidget { this._owner = owner; this._commentThread = commentThread; this._isCollapsed = commentThread.collapsibleState !== modes.CommentThreadCollapsibleState.Expanded; - this._decorationIDs = []; this._localToDispose = []; this.create(); this.themeService.onThemeChange(this._applyTheme, this); @@ -150,12 +148,7 @@ export class ReviewZoneWidget extends ZoneWidget { public reveal(commentId?: string) { if (this._isCollapsed) { - if (this._decorationIDs && this._decorationIDs.length) { - let range = this.editor.getModel().getDecorationRange(this._decorationIDs[0]); - this.show(range, 2); - } else { - this.show({ lineNumber: this._commentThread.range.startLineNumber, column: 1 }, 2); - } + this.show({ lineNumber: this._commentThread.range.startLineNumber, column: 1 }, 2); } this._bodyElement.focus(); @@ -195,13 +188,9 @@ export class ReviewZoneWidget extends ZoneWidget { this._secondaryHeading = $('span.dirname').appendTo(titleElement).getHTMLElement(); this._metaHeading = $('span.meta').appendTo(titleElement).getHTMLElement(); - let primaryHeading = 'Participants:'; - $(this._primaryHeading).safeInnerHtml(primaryHeading); - this._primaryHeading.setAttribute('aria-label', primaryHeading); - - let secondaryHeading = this._commentThread.comments.filter(arrays.uniqueFilter(comment => comment.userName)).map(comment => `@${comment.userName}`).join(', '); - $(this._secondaryHeading).safeInnerHtml(secondaryHeading); - this._secondaryHeading.setAttribute('aria-label', secondaryHeading); + if (this._commentThread.comments.length) { + this.createParticipantsLabel(); + } const actionsContainer = $('.review-actions').appendTo(this._headElement); this._actionbarWidget = new ActionBar(actionsContainer.getHTMLElement(), {}); @@ -253,16 +242,6 @@ export class ReviewZoneWidget extends ZoneWidget { this._commentsElement.removeChild(commentElementsToDel[i].domNode); } - if (this._commentElements.length === 0) { - this._commentThread = commentThread; - commentThread.comments.forEach(comment => { - let newElement = new CommentNode(comment); - this._commentElements.push(newElement); - this._commentsElement.appendChild(newElement.domNode); - }); - return; - } - let lastCommentElement: HTMLElement = null; let newCommentNodeList: CommentNode[] = []; for (let i = newCommentsLen - 1; i >= 0; i--) { @@ -349,7 +328,15 @@ export class ReviewZoneWidget extends ZoneWidget { this.setCommentEditorDecorations(); // Only add the additional step of clicking a reply button to expand the textarea when there are existing comments - this.createReplyButton(); + if (hasExistingComments) { + this.createReplyButton(); + } else { + if (!dom.hasClass(this._commentForm, 'expand')) { + dom.addClass(this._commentForm, 'expand'); + this._commentEditor.focus(); + } + } + this._localToDispose.push(this._commentEditor.onKeyDown((ev: IKeyboardEvent) => { const hasExistingComments = this._commentThread.comments.length > 0; @@ -385,6 +372,7 @@ export class ReviewZoneWidget extends ZoneWidget { ); this.createReplyButton(); + this.createParticipantsLabel(); } this._commentEditor.setValue(''); @@ -416,31 +404,33 @@ export class ReviewZoneWidget extends ZoneWidget { } } - createReplyButton() { - const hasExistingComments = this._commentThread.comments.length > 0; - if (hasExistingComments) { - this._reviewThreadReplyButton = $('button.review-thread-reply-button').appendTo(this._commentForm).getHTMLElement(); - this._reviewThreadReplyButton.title = 'Reply...'; - this._reviewThreadReplyButton.textContent = 'Reply...'; - // bind click/escape actions for reviewThreadReplyButton and textArea - this._reviewThreadReplyButton.onclick = () => { - if (!dom.hasClass(this._commentForm, 'expand')) { - dom.addClass(this._commentForm, 'expand'); - this._commentEditor.focus(); - } - }; + createParticipantsLabel() { + const primaryHeading = 'Participants:'; + $(this._primaryHeading).safeInnerHtml(primaryHeading); + this._primaryHeading.setAttribute('aria-label', primaryHeading); - this._commentEditor.onDidBlurEditorWidget(() => { - if (this._commentEditor.getModel().getValueLength() === 0 && dom.hasClass(this._commentForm, 'expand')) { - dom.removeClass(this._commentForm, 'expand'); - } - }); - } else { + const secondaryHeading = this._commentThread.comments.filter(arrays.uniqueFilter(comment => comment.userName)).map(comment => `@${comment.userName}`).join(', '); + $(this._secondaryHeading).safeInnerHtml(secondaryHeading); + this._secondaryHeading.setAttribute('aria-label', secondaryHeading); + } + + createReplyButton() { + this._reviewThreadReplyButton = $('button.review-thread-reply-button').appendTo(this._commentForm).getHTMLElement(); + this._reviewThreadReplyButton.title = 'Reply...'; + this._reviewThreadReplyButton.textContent = 'Reply...'; + // bind click/escape actions for reviewThreadReplyButton and textArea + this._reviewThreadReplyButton.onclick = () => { if (!dom.hasClass(this._commentForm, 'expand')) { dom.addClass(this._commentForm, 'expand'); this._commentEditor.focus(); } - } + }; + + this._commentEditor.onDidBlurEditorWidget(() => { + if (this._commentEditor.getModel().getValueLength() === 0 && dom.hasClass(this._commentForm, 'expand')) { + dom.removeClass(this._commentForm, 'expand'); + } + }); } _refresh() { @@ -561,13 +551,12 @@ export class ReviewZoneWidget extends ZoneWidget { this._resizeObserver.disconnect(); this._resizeObserver = null; } - this.editor.changeDecorations(accessor => { - accessor.deltaDecorations(this._decorationIDs, []); - }); + if (this._commentGlyph) { this.editor.removeContentWidget(this._commentGlyph); this._commentGlyph = null; } + this._localToDispose.forEach(local => local.dispose()); this._onDidClose.fire(); }