Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only show comment thread range when the comment is expanded #152206

Merged
merged 1 commit into from Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vs/editor/common/languages.ts
Expand Up @@ -1549,7 +1549,7 @@ export interface CommentThread<T = IRange> {
onDidChangeInput: Event<CommentInput | undefined>;
onDidChangeRange: Event<T>;
onDidChangeLabel: Event<string | undefined>;
onDidChangeCollasibleState: Event<CommentThreadCollapsibleState | undefined>;
onDidChangeCollapsibleState: Event<CommentThreadCollapsibleState | undefined>;
onDidChangeState: Event<CommentThreadState | undefined>;
onDidChangeCanReply: Event<boolean>;
isDisposed: boolean;
Expand Down
8 changes: 4 additions & 4 deletions src/vs/workbench/api/browser/mainThreadComments.ts
Expand Up @@ -110,11 +110,11 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {

set collapsibleState(newState: languages.CommentThreadCollapsibleState | undefined) {
this._collapsibleState = newState;
this._onDidChangeCollasibleState.fire(this._collapsibleState);
this._onDidChangeCollapsibleState.fire(this._collapsibleState);
}

private readonly _onDidChangeCollasibleState = new Emitter<languages.CommentThreadCollapsibleState | undefined>();
public onDidChangeCollasibleState = this._onDidChangeCollasibleState.event;
private readonly _onDidChangeCollapsibleState = new Emitter<languages.CommentThreadCollapsibleState | undefined>();
public onDidChangeCollapsibleState = this._onDidChangeCollapsibleState.event;

private _isDisposed: boolean;

Expand Down Expand Up @@ -172,7 +172,7 @@ export class MainThreadCommentThread<T> implements languages.CommentThread<T> {

dispose() {
this._isDisposed = true;
this._onDidChangeCollasibleState.dispose();
this._onDidChangeCollapsibleState.dispose();
this._onDidChangeComments.dispose();
this._onDidChangeInput.dispose();
this._onDidChangeLabel.dispose();
Expand Down
Expand Up @@ -3,9 +3,10 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { Disposable } from 'vs/base/common/lifecycle';
import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
import { IRange } from 'vs/editor/common/core/range';
import { CommentThread, CommentThreadCollapsibleState } from 'vs/editor/common/languages';
import { IModelDecorationOptions, IModelDeltaDecoration } from 'vs/editor/common/model';
import { ModelDecorationOptions } from 'vs/editor/common/model/textModel';
import { ICommentInfo, ICommentService } from 'vs/workbench/contrib/comments/browser/commentService';
Expand Down Expand Up @@ -34,6 +35,8 @@ export class CommentThreadRangeDecorator extends Disposable {
private decorationIds: string[] = [];
private activeDecorationIds: string[] = [];
private editor: ICodeEditor | undefined;
private threadCollapseStateListeners: IDisposable[] = [];
private currentThreadCollapseStateListener: IDisposable | undefined;

constructor(commentService: ICommentService) {
super();
Expand All @@ -55,26 +58,42 @@ export class CommentThreadRangeDecorator extends Disposable {

this.activeDecorationOptions = ModelDecorationOptions.createDynamic(activeDecorationOptions);
this._register(commentService.onDidChangeCurrentCommentThread(thread => {
if (!this.editor) {
return;
}
const newDecoration: CommentThreadRangeDecoration[] = [];
if (thread) {
const range = thread.range;
if (!((range.startLineNumber === range.endLineNumber) && (range.startColumn === range.endColumn))) {
this.updateCurrent(thread);
}));
this._register(commentService.onDidUpdateCommentThreads(() => {
this.updateCurrent(undefined);
}));
}

private updateCurrent(thread: CommentThread<IRange> | undefined) {
if (!this.editor) {
return;
}
this.currentThreadCollapseStateListener?.dispose();
const newDecoration: CommentThreadRangeDecoration[] = [];
if (thread) {
const range = thread.range;
if (!((range.startLineNumber === range.endLineNumber) && (range.startColumn === range.endColumn))) {
if (thread.collapsibleState === CommentThreadCollapsibleState.Expanded) {
this.currentThreadCollapseStateListener = thread.onDidChangeCollapsibleState(state => {
if (state === CommentThreadCollapsibleState.Collapsed) {
this.updateCurrent(undefined);
}
});
newDecoration.push(new CommentThreadRangeDecoration(range, this.activeDecorationOptions));
}
}
this.activeDecorationIds = this.editor.deltaDecorations(this.activeDecorationIds, newDecoration);
newDecoration.forEach((decoration, index) => decoration.id = this.decorationIds[index]);
}));
}
this.activeDecorationIds = this.editor.deltaDecorations(this.activeDecorationIds, newDecoration);
newDecoration.forEach((decoration, index) => decoration.id = this.decorationIds[index]);
}

public update(editor: ICodeEditor, commentInfos: ICommentInfo[]) {
const model = editor.getModel();
if (!model) {
return;
}
this.threadCollapseStateListeners.forEach(disposable => disposable.dispose());
this.editor = editor;

const commentThreadRangeDecorations: CommentThreadRangeDecoration[] = [];
Expand All @@ -83,17 +102,33 @@ export class CommentThreadRangeDecorator extends Disposable {
if (thread.isDisposed) {
return;
}

const range = thread.range;
// We only want to show a range decoration when there's the range spans either multiple lines
// or, when is spans multiple characters on the sample line
if ((range.startLineNumber === range.endLineNumber) && (range.startColumn === range.endColumn)) {
return;
}

this.threadCollapseStateListeners.push(thread.onDidChangeCollapsibleState(() => {
this.update(editor, commentInfos);
}));

if (thread.collapsibleState === CommentThreadCollapsibleState.Collapsed) {
return;
}

commentThreadRangeDecorations.push(new CommentThreadRangeDecoration(range, this.decorationOptions));
});
}

this.decorationIds = editor.deltaDecorations(this.decorationIds, commentThreadRangeDecorations);
commentThreadRangeDecorations.forEach((decoration, index) => decoration.id = this.decorationIds[index]);
}

override dispose() {
this.threadCollapseStateListeners.forEach(disposable => disposable.dispose());
this.currentThreadCollapseStateListener?.dispose();
super.dispose();
}
}
Expand Up @@ -342,7 +342,7 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
}
}));

this._commentThreadDisposables.push(this._commentThread.onDidChangeCollasibleState(state => {
this._commentThreadDisposables.push(this._commentThread.onDidChangeCollapsibleState(state => {
if (state === languages.CommentThreadCollapsibleState.Expanded && !this._isExpanded) {
const lineNumber = this._commentThread.range.startLineNumber;

Expand Down