Show the lines of code around a comment as context
Gerrit now supports requesting the lines of code around a comment
to show the context around the comment.
We show the line number as well as the line of code itself.
Highlighting of the selected comment range will be added in a follow
up change.
We re-use the existing diff component to render the context
The context is currently shown in the Comments Tab as well as the
Change Log.
Change-Id: Ieb6ea2ca6d9c071cb0f9b9e15238223314f2446b
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 5d7125c..7ee68b7 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -178,6 +178,11 @@
theme?: string;
}
+export declare interface RenderPreferences {
+ hide_left_side?: boolean;
+ disable_context_control_buttons?: boolean;
+}
+
/**
* Whether whitespace changes should be ignored and if yes, which whitespace
* changes should be ignored
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 81aff90..355be72 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -681,6 +681,7 @@
logged-in="[[_loggedIn]]"
only-show-robot-comments-with-human-reply=""
unresolved-only
+ show-comment-context
></gr-thread-list>
</template>
<template
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
index 6515af0..0409a42 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -259,6 +259,7 @@
change-num="[[changeNum]]"
logged-in="[[_loggedIn]]"
hide-toggle-buttons
+ show-comment-context
>
</gr-thread-list>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 22b2e43..28bccc6 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -77,6 +77,9 @@
@property({type: Array})
_sortedThreads: CommentThread[] = [];
+ @property({type: Boolean})
+ showCommentContext = false;
+
@property({
computed:
'_computeDisplayedThreads(_sortedThreads.*, unresolvedOnly, ' +
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
index 8a52555..1be2da1 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
@@ -25,7 +25,6 @@
gr-comment-thread {
display: block;
margin-bottom: var(--spacing-m);
- max-width: 80ch;
}
.header {
align-items: center;
@@ -172,6 +171,7 @@
<gr-comment-thread
show-file-path=""
show-ported-comment="[[thread.ported]]"
+ show-comment-context="[[showCommentContext]]"
change-num="[[changeNum]]"
comments="[[thread.comments]]"
diff-side="[[thread.diffSide]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 7946061..7c32bf7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -66,7 +66,10 @@
// @ts-ignore
import * as shadow from 'shadow-selection-polyfill/shadow.js';
-import {CreateCommentEventDetail as CreateCommentEventDetailApi} from '../../../api/diff';
+import {
+ CreateCommentEventDetail as CreateCommentEventDetailApi,
+ RenderPreferences,
+} from '../../../api/diff';
import {isSafari} from '../../../utils/dom-util';
const NO_NEWLINE_BASE = 'No newline at end of base file.';
@@ -158,6 +161,9 @@
@property({type: Object, observer: '_prefsObserver'})
prefs?: DiffPreferencesInfo;
+ @property({type: Object, observer: '_renderPrefsChanged'})
+ renderPrefs?: RenderPreferences;
+
@property({type: Boolean})
displayLine = false;
@@ -727,6 +733,16 @@
}
}
+ _renderPrefsChanged(renderPrefs?: RenderPreferences) {
+ if (!renderPrefs) return;
+ if (renderPrefs.hide_left_side) {
+ this.classList.add('no-left');
+ }
+ if (renderPrefs.disable_context_control_buttons) {
+ this.updateStyles({'--context-control-display': 'none'});
+ }
+ }
+
_diffChanged(newValue?: DiffInfo) {
this._setLoading(true);
this._cleanup();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index b0f48ce..4b477cd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -272,6 +272,7 @@
/* Context controls */
.contextControl {
+ display: var(--context-control-display, table-row-group);
background-color: transparent;
border: none;
--divider-height: var(--spacing-s);
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index 6ee82c9..1af2adf 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -17,6 +17,7 @@
import '../../../styles/shared-styles';
import '../gr-storage/gr-storage';
import '../gr-comment/gr-comment';
+import '../../diff/gr-diff/gr-diff';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
@@ -24,6 +25,7 @@
import {htmlTemplate} from './gr-comment-thread_html';
import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {
+ computeDiffFromContext,
isDraft,
isRobot,
sortComments,
@@ -33,9 +35,14 @@
} from '../../../utils/comment-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {appContext} from '../../../services/app-context';
-import {CommentSide, Side, SpecialFilePath} from '../../../constants/constants';
+import {
+ CommentSide,
+ createDefaultDiffPrefs,
+ Side,
+ SpecialFilePath,
+} from '../../../constants/constants';
import {computeDisplayPath} from '../../../utils/path-list-util';
-import {customElement, observe, property} from '@polymer/decorators';
+import {computed, customElement, observe, property} from '@polymer/decorators';
import {
CommentRange,
ConfigInfo,
@@ -50,6 +57,9 @@
import {CustomKeyboardEvent} from '../../../types/events';
import {LineNumber, FILE} from '../../diff/gr-diff/gr-diff-line';
import {GrButton} from '../gr-button/gr-button';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
+import {RenderPreferences} from '../../../api/diff';
const UNRESOLVED_EXPAND_COUNT = 5;
const NEWLINE_PATTERN = /\n/g;
@@ -122,7 +132,7 @@
patchNum?: PatchSetNum;
@property({type: String})
- path?: string;
+ path: string | undefined;
@property({type: String, observer: '_projectNameChanged'})
projectName?: RepoName;
@@ -164,6 +174,15 @@
@property({type: Object})
_projectConfig?: ConfigInfo;
+ @property({type: Object})
+ _prefs: DiffPreferencesInfo = createDefaultDiffPrefs();
+
+ @property({type: Object})
+ _renderPrefs: RenderPreferences = {
+ hide_left_side: true,
+ disable_context_control_buttons: true,
+ };
+
@property({type: Boolean, reflectToAttribute: true})
isRobotComment = false;
@@ -176,6 +195,9 @@
@property({type: Boolean})
showPatchset = true;
+ @property({type: Boolean})
+ showCommentContext = false;
+
get keyBindings() {
return {
'e shift+e': '_handleEKey',
@@ -188,6 +210,10 @@
readonly storage = new GrStorage();
+ private isCommentContextExperimentEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.COMMENT_CONTEXT
+ );
+
readonly restApiService = appContext.restApiService;
/** @override */
@@ -204,9 +230,34 @@
this._getLoggedIn().then(loggedIn => {
this._showActions = loggedIn;
});
+ this.restApiService.getDiffPreferences().then(prefs => {
+ if (!prefs) return;
+ this._prefs = {
+ ...prefs,
+ show_file_comment_button: false,
+ // override explicitly so that diff doesn't take too much width
+ // compared to the context
+ line_wrapping: false,
+ };
+ });
this._setInitialExpandedState();
}
+ @computed('comments', 'path')
+ get _diff() {
+ if (this.comments === undefined || this.path === undefined) return;
+ if (!this.comments[0]?.context_lines?.length) return;
+ return computeDiffFromContext(this.comments[0].context_lines, this.path);
+ }
+
+ _shouldShowCommentContext(diff?: DiffInfo) {
+ return (
+ this.isCommentContextExperimentEnabled &&
+ this.showCommentContext &&
+ !!diff
+ );
+ }
+
addOrEditDraft(lineNum?: LineNumber, rangeParam?: CommentRange) {
const lastComment = this.comments[this.comments.length - 1] || {};
if (isDraft(lastComment)) {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
index 1a89a79..7d5ba11 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_html.ts
@@ -23,6 +23,15 @@
font-size: var(--font-size-normal);
font-weight: var(--font-weight-normal);
line-height: var(--line-height-normal);
+ /* Explicitly set the background color of the diff to be white. We
+ * cannot use the diff content type ab because of the skip chunk preceding
+ * it, diff processor assumes the chunk of type skip/ab can be collapsed
+ * and hides our diff behind context control buttons.
+ * */
+ --dark-add-highlight-color: white;
+ --diff-container-styles: {
+ border: 1px solid var(--border-color);
+ }
}
gr-button {
margin-left: var(--spacing-m);
@@ -34,24 +43,29 @@
margin-left: auto;
padding: var(--spacing-s) var(--spacing-m);
}
- #container {
+ .comment-box {
+ width: 80ch;
+ max-width: 100%;
background-color: var(--comment-background-color);
color: var(--comment-text-color);
- display: var(--gr-comment-thread-display, block);
- margin: 0 var(--spacing-s) var(--spacing-s);
- white-space: normal;
box-shadow: var(--elevation-level-2);
border-radius: var(--border-radius);
+ }
+ #container {
+ display: var(--gr-comment-thread-display, flex);
+ align-items: flex-start;
+ margin: 0 var(--spacing-s) var(--spacing-s);
+ white-space: normal;
/** This is required for firefox to continue the inheritance */
-webkit-user-select: inherit;
-moz-user-select: inherit;
-ms-user-select: inherit;
user-select: inherit;
}
- #container.unresolved {
+ .comment-box.unresolved {
background-color: var(--unresolved-comment-background-color);
}
- #container.robotComment {
+ .comment-box.robotComment {
background-color: var(--robot-comment-background-color);
}
#commentInfoContainer {
@@ -75,7 +89,11 @@
.fileName {
padding: var(--spacing-m) var(--spacing-s) var(--spacing-m);
}
+ gr-diff {
+ margin-left: var(--spacing-l);
+ }
</style>
+
<template is="dom-if" if="[[showFilePath]]">
<template is="dom-if" if="[[showFileName]]">
<div class="fileName">
@@ -100,71 +118,81 @@
</template>
</div>
</template>
- <div
- id="container"
- class$="[[_computeHostClass(unresolved, isRobotComment)]]"
- >
- <template
- id="commentList"
- is="dom-repeat"
- items="[[_orderedComments]]"
- as="comment"
- >
- <gr-comment
- comment="{{comment}}"
- comments="{{comments}}"
- robot-button-disabled="[[_shouldDisableAction(_showActions, _lastComment)]]"
- change-num="[[changeNum]]"
- project-name="[[projectName]]"
- patch-num="[[patchNum]]"
- draft="[[_isDraft(comment)]]"
- show-actions="[[_showActions]]"
- show-patchset="[[showPatchset]]"
- show-ported-comment="[[_computeShowPortedComment(comment)]]"
- side="[[comment.side]]"
- project-config="[[_projectConfig]]"
- on-create-fix-comment="_handleCommentFix"
- on-comment-discard="_handleCommentDiscard"
- on-comment-save="_handleCommentSavedOrDiscarded"
- ></gr-comment>
- </template>
- <div
- id="commentInfoContainer"
- hidden$="[[_hideActions(_showActions, _lastComment)]]"
- >
- <span id="unresolvedLabel">[[_getUnresolvedLabel(unresolved)]]</span>
- <div id="actions">
- <gr-button
- id="replyBtn"
- link=""
- class="action reply"
- on-click="_handleCommentReply"
- >Reply</gr-button
- >
- <gr-button
- id="quoteBtn"
- link=""
- class="action quote"
- on-click="_handleCommentQuote"
- >Quote</gr-button
- >
- <template is="dom-if" if="[[unresolved]]">
+ <div id="container">
+ <div class$="[[_computeHostClass(unresolved, isRobotComment)]] comment-box">
+ <template
+ id="commentList"
+ is="dom-repeat"
+ items="[[_orderedComments]]"
+ as="comment"
+ >
+ <gr-comment
+ comment="{{comment}}"
+ comments="{{comments}}"
+ robot-button-disabled="[[_shouldDisableAction(_showActions, _lastComment)]]"
+ change-num="[[changeNum]]"
+ project-name="[[projectName]]"
+ patch-num="[[patchNum]]"
+ draft="[[_isDraft(comment)]]"
+ show-actions="[[_showActions]]"
+ show-patchset="[[showPatchset]]"
+ show-ported-comment="[[_computeShowPortedComment(comment)]]"
+ side="[[comment.side]]"
+ project-config="[[_projectConfig]]"
+ on-create-fix-comment="_handleCommentFix"
+ on-comment-discard="_handleCommentDiscard"
+ on-comment-save="_handleCommentSavedOrDiscarded"
+ ></gr-comment>
+ </template>
+ <div
+ id="commentInfoContainer"
+ hidden$="[[_hideActions(_showActions, _lastComment)]]"
+ >
+ <span id="unresolvedLabel">[[_getUnresolvedLabel(unresolved)]]</span>
+ <div id="actions">
<gr-button
- id="ackBtn"
+ id="replyBtn"
link=""
- class="action ack"
- on-click="_handleCommentAck"
- >Ack</gr-button
+ class="action reply"
+ on-click="_handleCommentReply"
+ >Reply</gr-button
>
<gr-button
- id="doneBtn"
+ id="quoteBtn"
link=""
- class="action done"
- on-click="_handleCommentDone"
- >Done</gr-button
+ class="action quote"
+ on-click="_handleCommentQuote"
+ >Quote</gr-button
>
- </template>
+ <template is="dom-if" if="[[unresolved]]">
+ <gr-button
+ id="ackBtn"
+ link=""
+ class="action ack"
+ on-click="_handleCommentAck"
+ >Ack</gr-button
+ >
+ <gr-button
+ id="doneBtn"
+ link=""
+ class="action done"
+ on-click="_handleCommentDone"
+ >Done</gr-button
+ >
+ </template>
+ </div>
</div>
</div>
+ <template is="dom-if" if="[[_shouldShowCommentContext(_diff)]]">
+ <gr-diff
+ id="diff"
+ change-num="[[changeNum]]"
+ diff="[[_diff]]"
+ path="[[path]]"
+ prefs="[[_prefs]]"
+ render-prefs="[[_renderPrefs]]"
+ >
+ </gr-diff>
+ </template>
</div>
`;
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index f96afaa..878b8f7 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -31,4 +31,5 @@
NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
PORTING_COMMENTS = 'UiFeature__porting_comments',
NEW_IMAGE_DIFF_UI = 'UiFeature__new_image_diff_ui',
+ COMMENT_CONTEXT = 'UiFeature__comment_context',
}
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 0c4a3c6..61456a2 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -24,12 +24,14 @@
CommentRange,
PatchRange,
ParentPatchSetNum,
+ ContextLine,
} from '../types/common';
import {CommentSide, Side} from '../constants/constants';
import {parseDate} from './date-util';
import {LineNumber} from '../elements/diff/gr-diff/gr-diff-line';
import {CommentIdToCommentThreadMap} from '../elements/diff/gr-comment-api/gr-comment-api';
import {isMergeParent, getParentIndex} from './patch-set-util';
+import {DiffInfo} from '../types/diff';
export interface DraftCommentProps {
__draft?: boolean;
@@ -285,3 +287,32 @@
};
}
}
+
+export function computeDiffFromContext(context: ContextLine[], path: string) {
+ const diff: DiffInfo = {
+ meta_a: {
+ name: '',
+ content_type: '',
+ lines: 0,
+ web_links: [],
+ },
+ meta_b: {
+ name: path,
+ content_type: '',
+ lines: context.length + context?.[0].line_number,
+ web_links: [],
+ },
+ change_type: 'MODIFIED',
+ intraline_status: 'OK',
+ diff_header: [],
+ content: [
+ {
+ skip: context[0].line_number - 1,
+ },
+ {
+ b: context.map(line => line.context_line),
+ },
+ ],
+ };
+ return diff;
+}
