From 6cdbbdc2d602253b33d0f93c38b85ccf627a6885 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Mon, 30 Jul 2018 14:39:02 +0800 Subject: [PATCH] put commenting ranges affordance into line decorations area. --- src/vs/editor/common/config/editorOptions.ts | 2 +- .../electron-browser/commentGlyphWidget.ts | 41 +++-- .../electron-browser/commentThreadWidget.ts | 70 +++++---- .../commentsEditorContribution.ts | 146 +++++++++--------- .../electron-browser/media/review.css | 23 ++- 5 files changed, 157 insertions(+), 125 deletions(-) diff --git a/src/vs/editor/common/config/editorOptions.ts b/src/vs/editor/common/config/editorOptions.ts index 75fd5559db8..35c16ff732d 100644 --- a/src/vs/editor/common/config/editorOptions.ts +++ b/src/vs/editor/common/config/editorOptions.ts @@ -1543,7 +1543,7 @@ function _stringSet(value: T, defaultValue: T, allowedValues: T[]): T { return value; } -function _clampedInt(value: any, defaultValue: number, minimum: number, maximum: number): number { +export function _clampedInt(value: any, defaultValue: number, minimum: number, maximum: number): number { let r: number; if (typeof value === 'undefined') { r = defaultValue; diff --git a/src/vs/workbench/parts/comments/electron-browser/commentGlyphWidget.ts b/src/vs/workbench/parts/comments/electron-browser/commentGlyphWidget.ts index 44d7a10810e..b05c049300e 100644 --- a/src/vs/workbench/parts/comments/electron-browser/commentGlyphWidget.ts +++ b/src/vs/workbench/parts/comments/electron-browser/commentGlyphWidget.ts @@ -4,37 +4,38 @@ *--------------------------------------------------------------------------------------------*/ 'use strict'; -import { ICodeEditor, IContentWidget, IContentWidgetPosition, ContentWidgetPositionPreference } from 'vs/editor/browser/editorBrowser'; +import { ICodeEditor, IContentWidgetPosition, ContentWidgetPositionPreference } from 'vs/editor/browser/editorBrowser'; +import { ModelDecorationOptions } from 'vs/editor/common/model/textModel'; -export class CommentGlyphWidget implements IContentWidget { - private _id: string; +export class CommentGlyphWidget { private _lineNumber: number; - private _domNode: HTMLDivElement; private _editor: ICodeEditor; + private commentsDecorations: string[] = []; + private _commentsOptions: ModelDecorationOptions; - constructor(id: string, editor: ICodeEditor, lineNumber: number, disabled: boolean, onClick: () => void) { - this._id = id; - this._domNode = document.createElement('div'); - this._domNode.className = disabled ? 'comment-hint commenting-disabled' : 'comment-hint'; - this._domNode.addEventListener('click', onClick); + constructor(editor: ICodeEditor, lineNumber: number, commentsOptions: ModelDecorationOptions, onClick: () => void) { + this._commentsOptions = commentsOptions; this._lineNumber = lineNumber; - this._editor = editor; - this._editor.addContentWidget(this); - + this.update(); } - getDomNode(): HTMLElement { - return this._domNode; - } + update() { + let commentsDecorations = [{ + range: { + startLineNumber: this._lineNumber, startColumn: 1, + endLineNumber: this._lineNumber, endColumn: 1 + }, + options: this._commentsOptions + }]; - getId(): string { - return this._id; + this.commentsDecorations = this._editor.getModel().deltaDecorations(this.commentsDecorations, commentsDecorations); } setLineNumber(lineNumber: number): void { this._lineNumber = lineNumber; + this.update(); } getPosition(): IContentWidgetPosition { @@ -46,4 +47,10 @@ export class CommentGlyphWidget implements IContentWidget { preference: [ContentWidgetPositionPreference.EXACT] }; } + + dispose() { + if (this.commentsDecorations) { + this._editor.deltaDecorations(this.commentsDecorations, []); + } + } } \ No newline at end of file diff --git a/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts b/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts index 41fd94bf268..763a07cd439 100644 --- a/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts +++ b/src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts @@ -34,6 +34,8 @@ import { Range, IRange } from 'vs/editor/common/core/range'; import { IPosition } from 'vs/editor/common/core/position'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { MarkdownRenderer } from 'vs/editor/contrib/markdown/markdownRenderer'; +import { IMarginData } from 'vs/editor/browser/controller/mouseTarget'; +import { ModelDecorationOptions } from 'vs/editor/common/model/textModel'; export const COMMENTEDITOR_DECORATION_KEY = 'commenteditordecoration'; const EXPAND_ACTION_CLASS = 'expand-review-action octicon octicon-chevron-down'; @@ -287,11 +289,10 @@ export class ReviewZoneWidget extends ZoneWidget { this._commentEditor.layout({ height: (this._commentEditor.hasWidgetFocus() ? 5 : 1) * 18, width: widthInPixel - 40 /* margin */ }); } - display(lineNumber: number) { - this._commentGlyph = new CommentGlyphWidget(`review_${lineNumber}`, this.editor, lineNumber, false, () => { + display(lineNumber: number, commentsOptions: ModelDecorationOptions) { + this._commentGlyph = new CommentGlyphWidget(this.editor, lineNumber, commentsOptions, () => { this.toggleExpand(); }); - this.editor.layoutContentWidget(this._commentGlyph); this._localToDispose.push(this.editor.onMouseDown(e => this.onEditorMouseDown(e))); this._localToDispose.push(this.editor.onMouseUp(e => this.onEditorMouseUp(e))); @@ -300,7 +301,6 @@ export class ReviewZoneWidget extends ZoneWidget { if (this.position) { if (this.position.lineNumber !== this._commentGlyph.getPosition().position.lineNumber) { this._commentGlyph.setLineNumber(this.position.lineNumber); - this.editor.layoutContentWidget(this._commentGlyph); } } else { // Otherwise manually calculate position change :( @@ -312,7 +312,6 @@ export class ReviewZoneWidget extends ZoneWidget { } }).reduce((prev, curr) => prev + curr, 0); this._commentGlyph.setLineNumber(this._commentGlyph.getPosition().position.lineNumber + positionChange); - this.editor.layoutContentWidget(this._commentGlyph); } })); var headHeight = Math.ceil(this.editor.getConfiguration().lineHeight * 1.2); @@ -501,42 +500,58 @@ export class ReviewZoneWidget extends ZoneWidget { } } - private mouseDownInfo: { lineNumber: number, iconClicked: boolean }; + private mouseDownInfo: { lineNumber: number }; private onEditorMouseDown(e: IEditorMouseEvent): void { + this.mouseDownInfo = null; + + const range = e.target.range; + + if (!range) { + return; + } + if (!e.event.leftButton) { return; } - let range = e.target.range; - if (!range) { + if (e.target.type !== MouseTargetType.GUTTER_LINE_DECORATIONS) { return; } - let iconClicked = false; - switch (e.target.type) { - case MouseTargetType.GUTTER_GLYPH_MARGIN: - iconClicked = true; - break; - default: - return; + const data = e.target.detail as IMarginData; + const gutterOffsetX = data.offsetX - data.glyphMarginWidth - data.lineNumbersWidth - data.glyphMarginLeft; + + // don't collide with folding and git decorations + if (gutterOffsetX < 10 && gutterOffsetX > 19) { + return; } - this.mouseDownInfo = { lineNumber: range.startLineNumber, iconClicked }; + this.mouseDownInfo = { lineNumber: range.startLineNumber }; } private onEditorMouseUp(e: IEditorMouseEvent): void { if (!this.mouseDownInfo) { return; } - let lineNumber = this.mouseDownInfo.lineNumber; - let iconClicked = this.mouseDownInfo.iconClicked; - let range = e.target.range; + const { lineNumber } = this.mouseDownInfo; + this.mouseDownInfo = null; + + const range = e.target.range; + if (!range || range.startLineNumber !== lineNumber) { return; } + if (e.target.type !== MouseTargetType.GUTTER_LINE_DECORATIONS) { + return; + } + + if (!e.target.element) { + return; + } + if (this.position && this.position.lineNumber !== lineNumber) { return; } @@ -545,17 +560,14 @@ export class ReviewZoneWidget extends ZoneWidget { return; } - if (iconClicked) { - if (e.target.type !== MouseTargetType.GUTTER_GLYPH_MARGIN) { - return; + if (e.target.element.className.indexOf('comment-thread') >= 0) { + if (this._isCollapsed) { + this.show({ lineNumber: lineNumber, column: 1 }, 2); + } else { + this.hide(); + this._onDidClose.fire(); } } - - if (this._isCollapsed) { - this.show({ lineNumber: lineNumber, column: 1 }, 2); - } else { - this.hide(); - } } private _applyTheme(theme: ITheme) { @@ -623,7 +635,7 @@ export class ReviewZoneWidget extends ZoneWidget { } if (this._commentGlyph) { - this.editor.removeContentWidget(this._commentGlyph); + this._commentGlyph.dispose(); this._commentGlyph = null; } diff --git a/src/vs/workbench/parts/comments/electron-browser/commentsEditorContribution.ts b/src/vs/workbench/parts/comments/electron-browser/commentsEditorContribution.ts index 7f0dea1edf0..8c6b8b55cf5 100644 --- a/src/vs/workbench/parts/comments/electron-browser/commentsEditorContribution.ts +++ b/src/vs/workbench/parts/comments/electron-browser/commentsEditorContribution.ts @@ -26,7 +26,6 @@ import { editorForeground, registerColor } from 'vs/platform/theme/common/colorR 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 { CommentGlyphWidget } from 'vs/workbench/parts/comments/electron-browser/commentGlyphWidget'; 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'; @@ -74,22 +73,24 @@ class CommentingRangeDecorator { } private commentingOptions: ModelDecorationOptions; - private decorations: string[] = []; + public commentsOptions: ModelDecorationOptions; + private commentingRangeDecorations: string[] = []; private disposables: IDisposable[] = []; constructor( ) { const options = { gutter: true, overview: false }; this.commentingOptions = CommentingRangeDecorator.createDecoration('comment-diff-added', overviewRulerCommentingRangeForeground, options); + this.commentsOptions = CommentingRangeDecorator.createDecoration('comment-thread', overviewRulerCommentingRangeForeground, options); } - update(editor: ICodeEditor, ranges: Range[]) { + update(editor: ICodeEditor, commentsRanges: Range[], commentingRanges: Range[]) { let model = editor.getModel(); if (!model) { return; } - let decorations = ranges.map((change) => { + let commentingRangesDecorations = commentingRanges.map((change) => { const startLineNumber = change.startLineNumber; const endLineNumber = change.endLineNumber; @@ -102,12 +103,27 @@ class CommentingRangeDecorator { }; }); - this.decorations = model.deltaDecorations(this.decorations, decorations); + this.commentingRangeDecorations = model.deltaDecorations(this.commentingRangeDecorations, commentingRangesDecorations); + } + + + getActiveRanges(editor: ICodeEditor): Range[] { + let model = editor.getModel(); + if (!model) { + return []; + } + + let ranges = []; + for (let i = 0; i < this.commentingRangeDecorations.length; i++) { + ranges.push(model.getDecorationRange(this.commentingRangeDecorations[i])); + } + + return ranges; } dispose(): void { this.disposables = dispose(this.disposables); - this.decorations = []; + this.commentingRangeDecorations = []; } } @@ -120,10 +136,10 @@ export class ReviewController implements IEditorContribution { private _reviewPanelVisible: IContextKey; private _commentInfos: modes.CommentInfo[]; private _reviewModel: ReviewModel; - private _newCommentGlyph: CommentGlyphWidget; // private _hasSetComments: boolean; private _commentingRangeDecorator: CommentingRangeDecorator; private mouseDownInfo: { lineNumber: number } | null = null; + private _commentingRangeSpaceReserved = false; constructor( @@ -144,7 +160,6 @@ export class ReviewController implements IEditorContribution { this._commentInfos = []; this._commentWidgets = []; this._newCommentWidget = null; - this._newCommentGlyph = null; // this._hasSetComments = false; this._reviewPanelVisible = ctxReviewPanelVisible.bindTo(contextKeyService); @@ -164,7 +179,7 @@ export class ReviewController implements IEditorContribution { 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); + zoneWidget.display(thread.range.startLineNumber, this._commentingRangeDecorator.commentsOptions); this._commentWidgets.push(zoneWidget); }); }); @@ -177,11 +192,6 @@ export class ReviewController implements IEditorContribution { this._newCommentWidget = null; } - if (this._newCommentGlyph) { - this.editor.removeContentWidget(this._newCommentGlyph); - this._newCommentGlyph = null; - } - this.getComments(); })); @@ -294,11 +304,6 @@ export class ReviewController implements IEditorContribution { this._newCommentWidget = null; } - if (this._newCommentGlyph) { - this.editor.removeContentWidget(this._newCommentGlyph); - this._newCommentGlyph = null; - } - this._commentWidgets.forEach(zone => { zone.dispose(); }); @@ -306,12 +311,7 @@ export class ReviewController implements IEditorContribution { this.localToDispose.push(this.editor.onMouseDown(e => this.onEditorMouseDown(e))); this.localToDispose.push(this.editor.onMouseUp(e => this.onEditorMouseUp(e))); - this.localToDispose.push(this.editor.onMouseLeave(() => this.onMouseLeave())); this.localToDispose.push(this.editor.onDidChangeModelContent(() => { - if (this._newCommentGlyph) { - this.editor.removeContentWidget(this._newCommentGlyph); - this._newCommentGlyph = null; - } })); this.localToDispose.push(this.commentService.onDidUpdateCommentThreads(e => { const editorURI = this.editor && this.editor.getModel() && this.editor.getModel().uri; @@ -340,7 +340,7 @@ export class ReviewController implements IEditorContribution { }); added.forEach(thread => { let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.editor, e.owner, thread, {}); - zoneWidget.display(thread.range.startLineNumber); + zoneWidget.display(thread.range.startLineNumber, this._commentingRangeDecorator.commentsOptions); this._commentWidgets.push(zoneWidget); this._commentInfos.filter(info => info.owner === e.owner)[0].threads.push(thread); }); @@ -371,41 +371,12 @@ export class ReviewController implements IEditorContribution { }, {}); this._newCommentWidget.onDidClose(e => { + this._newCommentWidget.dispose(); this._newCommentWidget = null; }); - this._newCommentWidget.display(lineNumber); + this._newCommentWidget.display(lineNumber, this._commentingRangeDecorator.commentsOptions); } - /* private onEditorMouseMove(e: IEditorMouseEvent): void { - if (!this._hasSetComments) { - return; - } - - if (!this.editor.hasTextFocus()) { - return; - } - - const hasCommentingRanges = this._commentInfos.length && this._commentInfos.some(info => !!info.commentingRanges.length); - if (hasCommentingRanges && e.target.position && e.target.position.lineNumber !== undefined) { - if (this._newCommentGlyph && e.target.element.className !== 'comment-hint') { - this.editor.removeContentWidget(this._newCommentGlyph); - } - - const lineNumber = e.target.position.lineNumber; - if (!this.isExistingCommentThreadAtLine(lineNumber)) { - this._newCommentGlyph = this.isLineInCommentingRange(lineNumber) - ? this._newCommentGlyph = new CommentGlyphWidget('comment-hint', this.editor, lineNumber, false, () => { - this.addComment(lineNumber); - }) - : this._newCommentGlyph = new CommentGlyphWidget('comment-hint', this.editor, lineNumber, true, () => { - this.notificationService.warn('Commenting is not supported outside of diff hunk areas.'); - }); - - this.editor.layoutContentWidget(this._newCommentGlyph); - } - } - } */ - private onEditorMouseDown(e: IEditorMouseEvent): void { this.mouseDownInfo = null; @@ -426,8 +397,8 @@ export class ReviewController implements IEditorContribution { const data = e.target.detail as IMarginData; const gutterOffsetX = data.offsetX - data.glyphMarginWidth - data.lineNumbersWidth - data.glyphMarginLeft; - // TODO@joao TODO@alex TODO@martin this is such that we don't collide with folding - if (gutterOffsetX > 10) { + // don't collide with folding and git decorations + if (gutterOffsetX < 10 && gutterOffsetX > 19) { return; } @@ -456,7 +427,7 @@ export class ReviewController implements IEditorContribution { return; } - if (e.target.element.className.indexOf('comment-dirty-diff-glyph') >= 0) { + if (e.target.element.className.indexOf('comment-diff-added') >= 0) { const lineNumber = e.target.position.lineNumber; if (!this.isExistingCommentThreadAtLine(lineNumber)) { if (this.isLineInCommentingRange(lineNumber)) { @@ -468,12 +439,6 @@ export class ReviewController implements IEditorContribution { } } - private onMouseLeave(): void { - if (this._newCommentGlyph) { - this.editor.removeContentWidget(this._newCommentGlyph); - } - } - private getNewCommentAction(line: number): { replyCommand: modes.Command, ownerId: number } { for (let i = 0; i < this._commentInfos.length; i++) { const commentInfo = this._commentInfos[i]; @@ -493,11 +458,10 @@ export class ReviewController implements IEditorContribution { } private isLineInCommentingRange(line: number): boolean { - return this._commentInfos.some(commentInfo => { - return commentInfo.commentingRanges.some(range => - range.startLineNumber <= line && line <= range.endLineNumber - ); - }); + let ranges = this._commentingRangeDecorator.getActiveRanges(this.editor); + return ranges.some(range => + range.startLineNumber <= line && line <= range.endLineNumber + ); } private isExistingCommentThreadAtLine(line: number): boolean { @@ -514,6 +478,29 @@ export class ReviewController implements IEditorContribution { setComments(commentInfos: modes.CommentInfo[]): void { this._commentInfos = commentInfos; + let lineDecorationsWidth: number = this.editor.getConfiguration().layoutInfo.decorationsWidth; + + if (this._commentInfos.some(info => Boolean(info.commentingRanges && info.commentingRanges.length))) { + if (!this._commentingRangeSpaceReserved) { + this._commentingRangeSpaceReserved = true; + let extraEditorClassName = []; + if (this.editor.getRawConfiguration().extraEditorClassName) { + extraEditorClassName = this.editor.getRawConfiguration().extraEditorClassName.split(' '); + } + + if (this.editor.getConfiguration().contribInfo.folding) { + lineDecorationsWidth -= 16; + } + lineDecorationsWidth += 9; + extraEditorClassName.push('inline-comment'); + this.editor.updateOptions({ + extraEditorClassName: extraEditorClassName.join(' '), + lineDecorationsWidth: lineDecorationsWidth + }); + this.editor.layout(); + } + } + // this._hasSetComments = true; // create viewzones @@ -524,17 +511,22 @@ export class ReviewController implements IEditorContribution { 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); + zoneWidget.display(thread.range.startLineNumber, this._commentingRangeDecorator.commentsOptions); this._commentWidgets.push(zoneWidget); }); }); const commentingRanges = []; - this._commentInfos.forEach(info => commentingRanges.push(...info.commentingRanges)); - this._commentingRangeDecorator.update(this.editor, commentingRanges); + const commentsRanges = []; + this._commentInfos.forEach(info => { + commentingRanges.push(...info.commentingRanges); + info.threads.forEach(thread => { + commentsRanges.push(thread.range); + }); + }); + this._commentingRangeDecorator.update(this.editor, commentsRanges, commentingRanges); } - public closeWidget(): void { this._reviewPanelVisible.reset(); @@ -661,6 +653,12 @@ registerThemingParticipant((theme, collector) => { .monaco-editor .comment-diff-added:before { background: ${commentingRangeForeground}; } + .monaco-editor .comment-thread { + border-left: 3px solid ${commentingRangeForeground}; + } + .monaco-editor .comment-thread:before { + background: ${commentingRangeForeground}; + } `); } }); diff --git a/src/vs/workbench/parts/comments/electron-browser/media/review.css b/src/vs/workbench/parts/comments/electron-browser/media/review.css index a01ef1ed9ed..78b1615a052 100644 --- a/src/vs/workbench/parts/comments/electron-browser/media/review.css +++ b/src/vs/workbench/parts/comments/electron-browser/media/review.css @@ -242,7 +242,7 @@ } .monaco-editor .comment-dirty-diff-glyph { - margin-left: 1px; + margin-left: 14px; cursor: pointer; } @@ -255,11 +255,26 @@ transition: width 80ms linear, left 80ms linear; } -.monaco-editor .margin-view-overlays>div:hover>.comment-dirty-diff-glyph:before { +.monaco-editor .margin-view-overlays>div:hover>.comment-dirty-diff-glyph.comment-diff-added:before { position: absolute; content: '+'; height: 100%; - width: 8px; - left: -4px; + width: 9px; + left: -6px; z-index: 10; + color: black; +} + +.monaco-editor .comment-dirty-diff-glyph.comment-thread:before { + position: absolute; + content: 'ยท'; + height: 100%; + width: 9px; + left: -6px; + z-index: 20; + color: black; +} + +.monaco-editor.inline-comment .margin-view-overlays .folding { + margin-left: 14px; } \ No newline at end of file -- GitLab