Skip to content

Commit d5f6599

Browse files
authored
Comments gutter improvements (microsoft#158501)
* Some transparency fixes for comment gutter Part of microsoft#158299 * Try dotted line for multine comments
1 parent ff8d0d6 commit d5f6599

File tree

3 files changed

+37
-55
lines changed

3 files changed

+37
-55
lines changed

src/vs/workbench/contrib/comments/browser/commentThreadZoneWidget.ts

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,6 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
318318
display(lineNumber: number) {
319319
this._commentGlyph = new CommentGlyphWidget(this.editor, lineNumber);
320320

321-
this._disposables.add(this.editor.onMouseDown(e => this.onEditorMouseDown(e)));
322-
this._disposables.add(this.editor.onMouseUp(e => this.onEditorMouseUp(e)));
323-
324321
this._commentThreadWidget.display(this.editor.getOption(EditorOption.lineHeight));
325322
this._disposables.add(this._commentThreadWidget.onDidResize(dimension => {
326323
this._refresh(dimension);
@@ -418,29 +415,6 @@ export class ReviewZoneWidget extends ZoneWidget implements ICommentThreadWidget
418415
}
419416
}
420417

421-
private mouseDownInfo: { lineNumber: number } | null = null;
422-
423-
private onEditorMouseDown(e: IEditorMouseEvent): void {
424-
this.mouseDownInfo = parseMouseDownInfoFromEvent(e);
425-
}
426-
427-
private onEditorMouseUp(e: IEditorMouseEvent): void {
428-
const matchedLineNumber = isMouseUpEventMatchMouseDown(this.mouseDownInfo, e);
429-
this.mouseDownInfo = null;
430-
431-
if (matchedLineNumber === null || !e.target.element) {
432-
return;
433-
}
434-
435-
if (this._commentGlyph && this._commentGlyph.getPosition().position!.lineNumber !== matchedLineNumber) {
436-
return;
437-
}
438-
439-
if (e.target.element.className.indexOf('comment-thread') >= 0) {
440-
this.toggleExpand(matchedLineNumber);
441-
}
442-
}
443-
444418
private _applyTheme(theme: IColorTheme) {
445419
const borderColor = getCommentThreadWidgetStateColor(this._commentThread.state, this.themeService.getColorTheme()) || Color.transparent;
446420
this.style({

src/vs/workbench/contrib/comments/browser/commentsEditorContribution.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,15 @@ class CommentingRangeDecorator {
147147
const hoverDecorationOptions: IModelDecorationOptions = {
148148
description: CommentingRangeDecorator.description,
149149
isWholeLine: true,
150-
linesDecorationsClassName: `comment-range-glyph comment-diff-added line-hover`
150+
linesDecorationsClassName: `comment-range-glyph line-hover`
151151
};
152152

153153
this.hoverDecorationOptions = ModelDecorationOptions.createDynamic(hoverDecorationOptions);
154154

155155
const multilineDecorationOptions: IModelDecorationOptions = {
156156
description: CommentingRangeDecorator.description,
157157
isWholeLine: true,
158-
linesDecorationsClassName: `comment-range-glyph comment-diff-added multiline-add`
158+
linesDecorationsClassName: `comment-range-glyph multiline-add`
159159
};
160160

161161
this.multilineDecorationOptions = ModelDecorationOptions.createDynamic(multilineDecorationOptions);
@@ -203,7 +203,7 @@ class CommentingRangeDecorator {
203203
// If there's only one selection line, then just drop into the else if and show an emphasis line.
204204
&& !((intersectingSelectionRange.startLineNumber === intersectingSelectionRange.endLineNumber)
205205
&& (emphasisLine === intersectingSelectionRange.startLineNumber))) {
206-
// The emphasisLine should be the within the commenting range, even if the selection range stretches
206+
// The emphasisLine should be within the commenting range, even if the selection range stretches
207207
// outside of the commenting range.
208208
// Clip the emphasis and selection ranges to the commenting range
209209
let intersectingEmphasisRange: Range;
@@ -230,11 +230,15 @@ class CommentingRangeDecorator {
230230
commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, info.extensionId, info.label, afterRange, this.decorationOptions, info.commentingRanges, true));
231231
}
232232
} else if ((rangeObject.startLineNumber <= emphasisLine) && (emphasisLine <= rangeObject.endLineNumber)) {
233-
const beforeRange = new Range(range.startLineNumber, 1, emphasisLine, 1);
234-
const afterRange = new Range(emphasisLine, 1, range.endLineNumber, 1);
235-
commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, info.extensionId, info.label, beforeRange, this.decorationOptions, info.commentingRanges, true));
233+
if (rangeObject.startLineNumber < emphasisLine) {
234+
const beforeRange = new Range(range.startLineNumber, 1, emphasisLine - 1, 1);
235+
commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, info.extensionId, info.label, beforeRange, this.decorationOptions, info.commentingRanges, true));
236+
}
236237
commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, info.extensionId, info.label, new Range(emphasisLine, 1, emphasisLine, 1), this.hoverDecorationOptions, info.commentingRanges, true));
237-
commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, info.extensionId, info.label, afterRange, this.decorationOptions, info.commentingRanges, true));
238+
if (emphasisLine < rangeObject.endLineNumber) {
239+
const afterRange = new Range(emphasisLine + 1, 1, range.endLineNumber, 1);
240+
commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, info.extensionId, info.label, afterRange, this.decorationOptions, info.commentingRanges, true));
241+
}
238242
} else {
239243
commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, info.extensionId, info.label, range, this.decorationOptions, info.commentingRanges));
240244
}
@@ -702,7 +706,7 @@ export class CommentController implements IEditorContribution {
702706
if (matchedLineNumber === null || !e.target.element) {
703707
return;
704708
}
705-
const mouseUpIsOnDecorator = (e.target.element.className.indexOf('comment-diff-added') >= 0);
709+
const mouseUpIsOnDecorator = (e.target.element.className.indexOf('comment-range-glyph') >= 0);
706710

707711
const lineNumber = e.target.position!.lineNumber;
708712
let range: Range | undefined;
@@ -1143,15 +1147,14 @@ registerThemingParticipant((theme, collector) => {
11431147
const commentingRangeForeground = theme.getColor(overviewRulerCommentingRangeForeground);
11441148
if (commentingRangeForeground) {
11451149
collector.addRule(`
1146-
.monaco-editor .comment-diff-added {
1147-
border-left: 3px solid ${commentingRangeForeground};
1150+
.monaco-editor .comment-diff-added,
1151+
.monaco-editor .comment-range-glyph.multiline-add {
1152+
border-left-color: ${commentingRangeForeground};
11481153
}
1149-
.monaco-editor .comment-diff-added:before {
1154+
.monaco-editor .comment-diff-added:before,
1155+
.monaco-editor .comment-range-glyph.line-hover:before {
11501156
background: ${commentingRangeForeground};
11511157
}
1152-
.monaco-editor .comment-thread {
1153-
border-left: 3px solid ${commentingRangeForeground};
1154-
}
11551158
.monaco-editor .comment-thread:before {
11561159
background: ${commentingRangeForeground};
11571160
}

src/vs/workbench/contrib/comments/browser/media/review.css

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -382,18 +382,18 @@ div.preview.inline .monaco-editor .comment-range-glyph {
382382
display: none !important;
383383
}
384384

385-
.monaco-editor .comment-range-glyph:before {
386-
position: absolute;
387-
content: "";
388-
height: 100%;
389-
width: 0;
390-
left: -2px;
391-
transition: width 80ms linear, left 80ms linear;
385+
.monaco-editor .comment-diff-added {
386+
border-left-width: 3px;
387+
border-left-style: solid;
388+
}
389+
390+
.monaco-editor .margin-view-overlays .comment-range-glyph.line-hover,
391+
.monaco-editor .margin-view-overlays .comment-range-glyph.comment-thread {
392+
margin-left: 13px;
392393
}
393394

394395
.monaco-editor .margin-view-overlays > div:hover > .comment-range-glyph.comment-diff-added:before,
395-
.monaco-editor .margin-view-overlays .comment-range-glyph.comment-diff-added.line-hover:before,
396-
.monaco-editor .margin-view-overlays .comment-range-glyph.comment-diff-added.multiline-add:before,
396+
.monaco-editor .margin-view-overlays .comment-range-glyph.line-hover:before,
397397
.monaco-editor .comment-range-glyph.comment-thread:before {
398398
position: absolute;
399399
height: 100%;
@@ -408,15 +408,20 @@ div.preview.inline .monaco-editor .comment-range-glyph {
408408
justify-content: center;
409409
}
410410

411-
.monaco-editor .margin-view-overlays .comment-range-glyph.comment-diff-added.multiline-add:before,
411+
.monaco-editor .margin-view-overlays .comment-range-glyph.multiline-add {
412+
border-left-width: 3px;
413+
border-left-style: dotted;
414+
height: 16px;
415+
margin-top: 2px;
416+
}
417+
412418
.monaco-editor .margin-view-overlays > div:hover > .comment-range-glyph.comment-diff-added:before,
413-
.monaco-editor .margin-view-overlays .comment-range-glyph.comment-diff-added.line-hover:before {
419+
.monaco-editor .margin-view-overlays .comment-range-glyph.line-hover:before {
414420
content: "\ea60";
415421
font-family: "codicon";
416422
border-radius: 3px;
417-
width: 19px !important;
423+
width: 18px !important;
418424
margin-left: -5px;
419-
padding-top: 1px;
420425
padding-left: 1px;
421426
}
422427

@@ -427,8 +432,8 @@ div.preview.inline .monaco-editor .comment-range-glyph {
427432
.monaco-editor .comment-range-glyph.comment-thread:before {
428433
content: "\ea6b";
429434
font-family: "codicon";
430-
font-size: 14px;
431-
width: 19px !important;
435+
font-size: 13px;
436+
width: 18px !important;
432437
line-height: 100%;
433438
border-radius: 3px;
434439
z-index: 20;

0 commit comments

Comments
 (0)