Skip to content

Commit 44fceab

Browse files
alexdimaalexdima
committed
Add a close button and a way to jump to a line to the diff review pane
1 parent 4d0173b commit 44fceab

5 files changed

Lines changed: 91 additions & 9 deletions

File tree

‎src/vs/editor/browser/widget/diffEditorWidget.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
287287
this._reviewPane = new DiffReview(this);
288288
this._containerDomElement.appendChild(this._reviewPane.domNode.domNode);
289289
this._containerDomElement.appendChild(this._reviewPane.shadow.domNode);
290+
this._containerDomElement.appendChild(this._reviewPane.actionBarContainer.domNode);
290291

291292
if (options.automaticLayout) {
292293
this._measureDomElementToken = window.setInterval(() => this._measureDomElement(false), 100);
@@ -771,7 +772,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
771772

772773
this._width = dimensions.width;
773774
this._height = dimensions.height;
774-
this._reviewHeight = this._reviewPane.isVisible() ? Math.floor(0.5 * this._height) : 0;
775+
this._reviewHeight = this._reviewPane.isVisible() ? this._height : 0;
775776

776777
this._doLayout();
777778
}

‎src/vs/editor/browser/widget/diffReview.ts‎

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { DiffEditorWidget } from "vs/editor/browser/widget/diffEditorWidget";
2222
import { DomScrollableElement } from "vs/base/browser/ui/scrollbar/scrollableElement";
2323
import { editorLineNumbers } from "vs/editor/common/view/editorColorRegistry";
2424
import { KeyCode, KeyMod } from "vs/base/common/keyCodes";
25+
import { ActionBar } from "vs/base/browser/ui/actionbar/actionbar";
26+
import { Action } from "vs/base/common/actions";
2527

2628
const DIFF_LINES_PADDING = 3;
2729

@@ -68,6 +70,8 @@ export class DiffReview extends Disposable {
6870
private readonly _diffEditor: DiffEditorWidget;
6971
private _isVisible: boolean;
7072
public readonly shadow: FastDomNode<HTMLElement>;
73+
private readonly _actionBar: ActionBar;
74+
public readonly actionBarContainer: FastDomNode<HTMLElement>;
7175
public readonly domNode: FastDomNode<HTMLElement>;
7276
private readonly _content: FastDomNode<HTMLElement>;
7377
private readonly scrollbar: DomScrollableElement;
@@ -82,10 +86,22 @@ export class DiffReview extends Disposable {
8286
this.shadow = createFastDomNode(document.createElement('div'));
8387
this.shadow.setClassName('diff-review-shadow');
8488

89+
this.actionBarContainer = createFastDomNode(document.createElement('div'));
90+
this.actionBarContainer.setClassName('diff-review-actions');
91+
this._actionBar = this._register(new ActionBar(
92+
this.actionBarContainer.domNode
93+
));
94+
95+
this._actionBar.push(new Action('diffreview.close', nls.localize('label.close', "Close"), 'close-diff-review', true, () => {
96+
this.hide();
97+
return null;
98+
}), { label: false, icon: true });
99+
85100
this.domNode = createFastDomNode(document.createElement('div'));
86101
this.domNode.setClassName('diff-review monaco-editor-background');
87102

88103
this._content = createFastDomNode(document.createElement('div'));
104+
this._content.setClassName('diff-review-content');
89105
this.scrollbar = this._register(new DomScrollableElement(this._content.domNode, {}));
90106
this.domNode.domNode.appendChild(this.scrollbar.getDomNode());
91107

@@ -137,11 +153,30 @@ export class DiffReview extends Disposable {
137153
e.preventDefault();
138154
this.hide();
139155
}
156+
157+
if (
158+
e.equals(KeyCode.Space)
159+
|| e.equals(KeyCode.Enter)
160+
) {
161+
e.preventDefault();
162+
this.accept();
163+
}
140164
}));
141165
this._diffs = [];
142166
this._currentDiff = null;
143167
}
144168

169+
private accept(): void {
170+
let current = this._getCurrentFocusedRow();
171+
if (current) {
172+
let lineNumber = parseInt(current.getAttribute('data-line'), 10);
173+
if (!isNaN(lineNumber)) {
174+
this._diffEditor.setPosition(new Position(lineNumber, 1));
175+
}
176+
}
177+
this.hide();
178+
}
179+
145180
private hide(): void {
146181
this._isVisible = false;
147182
this._diffEditor.focus();
@@ -187,7 +222,7 @@ export class DiffReview extends Disposable {
187222
let prev = this._getCurrentFocusedRow();
188223
row.tabIndex = 0;
189224
row.focus();
190-
if (prev) {
225+
if (prev && prev !== row) {
191226
prev.tabIndex = -1;
192227
}
193228
this.scrollbar.scanDomNode();
@@ -209,6 +244,14 @@ export class DiffReview extends Disposable {
209244
this.domNode.setHeight(height);
210245
this._content.setHeight(height);
211246
this._content.setWidth(width);
247+
248+
if (this._isVisible) {
249+
this.actionBarContainer.setAttribute('aria-hidden', 'false');
250+
this.actionBarContainer.setDisplay('block');
251+
} else {
252+
this.actionBarContainer.setAttribute('aria-hidden', 'true');
253+
this.actionBarContainer.setDisplay('none');
254+
}
212255
}
213256

214257
private _compute(): Diff[] {
@@ -451,18 +494,23 @@ export class DiffReview extends Disposable {
451494
header.className = 'diff-review-row';
452495

453496
let cell = document.createElement('div');
454-
cell.className = 'diff-review-cell';
497+
cell.className = 'diff-review-cell diff-review-summary';
455498
cell.appendChild(document.createTextNode(`${diffIndex + 1}/${this._diffs.length}: @@ -${minOriginalLine},${maxOriginalLine - minOriginalLine + 1} +${minModifiedLine},${maxModifiedLine - minModifiedLine + 1} @@`));
499+
header.setAttribute('data-line', String(minModifiedLine));
456500
header.setAttribute('aria-label', nls.localize('header', "Difference {0} of {1}: original {2}, {3} lines, modified {4}, {5} lines", (diffIndex + 1), this._diffs.length, minOriginalLine, maxOriginalLine - minOriginalLine + 1, minModifiedLine, maxModifiedLine - minModifiedLine + 1));
457501
header.appendChild(cell);
458502

459503
// @@ -504,7 +517,7 @@
460504
header.setAttribute('role', 'listitem');
461505
container.appendChild(header);
462506

507+
let modLine = minModifiedLine;
463508
for (let i = 0, len = diffs.length; i < len; i++) {
464509
const diffEntry = diffs[i];
465-
DiffReview._renderSection(container, diffEntry, this._width, originalOpts, originalModel, originalModelOpts, modifiedOpts, modifiedModel, modifiedModelOpts);
510+
DiffReview._renderSection(container, diffEntry, modLine, this._width, originalOpts, originalModel, originalModelOpts, modifiedOpts, modifiedModel, modifiedModelOpts);
511+
if (diffEntry.modifiedLineStart !== 0) {
512+
modLine = diffEntry.modifiedLineEnd;
513+
}
466514
}
467515

468516
dom.clearNode(this._content.domNode);
@@ -471,7 +519,7 @@ export class DiffReview extends Disposable {
471519
}
472520

473521
private static _renderSection(
474-
dest: HTMLElement, diffEntry: DiffEntry, width: number,
522+
dest: HTMLElement, diffEntry: DiffEntry, modLine: number, width: number,
475523
originalOpts: editorOptions.InternalEditorOptions, originalModel: editorCommon.IModel, originalModelOpts: editorCommon.TextModelResolvedOptions,
476524
modifiedOpts: editorOptions.InternalEditorOptions, modifiedModel: editorCommon.IModel, modifiedModelOpts: editorCommon.TextModelResolvedOptions
477525
): void {
@@ -504,6 +552,9 @@ export class DiffReview extends Disposable {
504552
originalLineEnd - originalLineStart
505553
);
506554

555+
const originalLineNumbersWidth = originalOpts.layoutInfo.glyphMarginWidth + originalOpts.layoutInfo.lineNumbersWidth;
556+
const modifiedLineNumbersWidth = 10 + modifiedOpts.layoutInfo.glyphMarginWidth + modifiedOpts.layoutInfo.lineNumbersWidth;
557+
507558
for (let i = 0; i <= cnt; i++) {
508559
const originalLine = (originalLineStart === 0 ? 0 : originalLineStart + i);
509560
const modifiedLine = (modifiedLineStart === 0 ? 0 : modifiedLineStart + i);
@@ -512,14 +563,18 @@ export class DiffReview extends Disposable {
512563
row.style.minWidth = width + 'px';
513564
row.className = rowClassName;
514565
row.setAttribute('role', 'listitem');
566+
if (modifiedLine !== 0) {
567+
modLine = modifiedLine;
568+
}
569+
row.setAttribute('data-line', String(modLine));
515570

516571
let cell = document.createElement('div');
517572
cell.className = 'diff-review-cell';
518573
row.appendChild(cell);
519574

520575
const originalLineNumber = document.createElement('span');
521-
originalLineNumber.style.width = (originalOpts.layoutInfo.lineNumbersWidth + 'px');
522-
originalLineNumber.style.minWidth = (originalOpts.layoutInfo.lineNumbersWidth + 'px');
576+
originalLineNumber.style.width = (originalLineNumbersWidth + 'px');
577+
originalLineNumber.style.minWidth = (originalLineNumbersWidth + 'px');
523578
originalLineNumber.className = 'diff-review-line-number' + lineNumbersExtraClassName;
524579
if (originalLine !== 0) {
525580
originalLineNumber.appendChild(document.createTextNode(String(originalLine)));
@@ -529,8 +584,8 @@ export class DiffReview extends Disposable {
529584
cell.appendChild(originalLineNumber);
530585

531586
const modifiedLineNumber = document.createElement('span');
532-
modifiedLineNumber.style.width = (10 + modifiedOpts.layoutInfo.lineNumbersWidth + 'px');
533-
modifiedLineNumber.style.minWidth = (10 + modifiedOpts.layoutInfo.lineNumbersWidth + 'px');
587+
modifiedLineNumber.style.width = (modifiedLineNumbersWidth + 'px');
588+
modifiedLineNumber.style.minWidth = (modifiedLineNumbersWidth + 'px');
534589
modifiedLineNumber.style.paddingRight = '10px';
535590
modifiedLineNumber.className = 'diff-review-line-number' + lineNumbersExtraClassName;
536591
if (modifiedLine !== 0) {

‎src/vs/editor/browser/widget/media/close-inverse.svg‎

Lines changed: 1 addition & 0 deletions
Loading

‎src/vs/editor/browser/widget/media/close.svg‎

Lines changed: 1 addition & 0 deletions
Loading

‎src/vs/editor/browser/widget/media/diffReview.css‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
user-select: none;
1919
}
2020

21+
.monaco-diff-editor .diff-review-summary {
22+
padding-left: 10px;
23+
}
24+
2125
.monaco-diff-editor .diff-review-shadow {
2226
position: absolute;
2327
}
@@ -44,3 +48,23 @@
4448
display: inline-block;
4549
width: 10px;
4650
}
51+
52+
.monaco-diff-editor .diff-review-actions {
53+
display: inline-block;
54+
position: absolute;
55+
right: 2px;
56+
top: 2px;
57+
}
58+
59+
.monaco-diff-editor .diff-review-actions .action-label {
60+
width: 16px;
61+
height: 16px;
62+
margin: 2px 0;
63+
}
64+
.monaco-diff-editor .action-label.icon.close-diff-review {
65+
background: url('close.svg') center center no-repeat;
66+
}
67+
.monaco-editor.hc-black .action-label.icon.close-diff-review,
68+
.monaco-editor.vs-dark .action-label.icon.close-diff-review {
69+
background: url('close-inverse.svg') center center no-repeat;
70+
}

0 commit comments

Comments
 (0)