Merge changes I6888e279,I9191025b
* changes:
Make ContributorAgreement an AutoValue
Make PermissionRule an AutoValue
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 25d6518..3bb6564 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7427,24 +7427,24 @@
[options="header",cols="1,^1,5"]
|============================
-|Field Name ||Description
-|`message` |optional|
+|Field Name ||Description
+|`message` |optional|
The message to be added as review comment.
-|`tag` |optional|
+|`tag` |optional|
Apply this tag to the review comment message, votes, and inline
comments. Tags may be used by CI or other automated systems to
distinguish them from human reviews. Votes/comments that contain `tag` with
'autogenerated:' prefix can be filtered out in the web UI.
-|`labels` |optional|
+|`labels` |optional|
The votes that should be added to the revision as a map that maps the
label names to the voting values.
-|`comments` |optional|
+|`comments` |optional|
The comments that should be added as a map that maps a file path to a
list of link:#comment-input[CommentInput] entities.
-|`robot_comments` |optional|
+|`robot_comments` |optional|
The robot comments that should be added as a map that maps a file path
to a list of link:#robot-comment-input[RobotCommentInput] entities.
-|`drafts` |optional|
+|`drafts` |optional|
Draft handling that defines how draft comments are handled that are
already in the database but that were not also described in this
input. +
@@ -7453,36 +7453,36 @@
Only `KEEP` is allowed when used in conjunction with `on_behalf_of`. +
If not set, the default is `KEEP`. If `on_behalf_of` is set, then no other value
besides `KEEP` is allowed.
-|`notify` |optional|
+|`notify` |optional|
Notify handling that defines to whom email notifications should be sent
after the review is stored. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
If not set, the default is `ALL`.
-|`notify_details` |optional|
+|`notify_details` |optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
-|`omit_duplicate_comments` |optional|
+|`omit_duplicate_comments` |optional|
If `true`, comments with the same content at the same place will be omitted.
-|`on_behalf_of` |optional|
+|`on_behalf_of` |optional|
link:rest-api-accounts.html#account-id[\{account-id\}] the review
should be posted on behalf of. To use this option the caller must
have been granted `labelAs-NAME` permission for all keys of labels.
-|`reviewers` |optional|
+|`reviewers` |optional|
A list of link:rest-api-changes.html#reviewer-input[ReviewerInput]
representing reviewers that should be added to the change.
-|`ready` |optional|
+|`ready` |optional|
If true, and if the change is work in progress, then start review.
It is an error for both `ready` and `work_in_progress` to be true.
-|`work_in_progress` |optional|
+|`work_in_progress` |optional|
If true, mark the change as work in progress. It is an error for both
`ready` and `work_in_progress` to be true.
-|`add_to_attention_set` |optional|
+|`add_to_attention_set` |optional|
list of link:#attention-set-input[AttentionSetInput] entities to add
to the link:#attention-set[attention set].
-remove_from_attention_set` |optional|
+|`remove_from_attention_set` |optional|
list of link:#attention-set-input[AttentionSetInput] entities to remove
from the link:#attention-set[attention set].
-ignore_default_attention_set_rules`|optional|
+|`ignore_default_attention_set_rules`|optional|
If set to true, ignore all default attention set rules described in the
link:#attention-set[attention set]. Updates in add_to_attention_set
and remove_from_attention_set are not ignored.
diff --git a/package.json b/package.json
index d051c41..329e3cb 100644
--- a/package.json
+++ b/package.json
@@ -22,6 +22,8 @@
},
"scripts": {
"clean": "git clean -fdx && bazel clean --expunge",
+ "compile:local": "tsc --project ./polygerrit-ui/app/tsconfig.json",
+ "compile:watch": "npm run compile:local -- --preserveWatchOutput --watch",
"start": "polygerrit-ui/run-server.sh",
"test": "./polygerrit-ui/app/run_test.sh",
"safe_bazelisk": "if which bazelisk >/dev/null; then bazel_bin=bazelisk; else bazel_bin=bazel; fi && $bazel_bin",
diff --git a/plugins/download-commands b/plugins/download-commands
index 47b783e..c4ef993 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit 47b783ea75036664dd591d2d3f1bcd06b68cdd5e
+Subproject commit c4ef993fa5e8578641d6447c831ace13743dd5de
diff --git a/polygerrit-ui/README.md b/polygerrit-ui/README.md
index ce274f2..3a66e97 100644
--- a/polygerrit-ui/README.md
+++ b/polygerrit-ui/README.md
@@ -4,7 +4,7 @@
contains several typescript files and uses typescript compiler. This is a
preparation for the upcoming migration to typescript and we actively working on
it. We want to avoid massive typescript-related changes until the preparation
-work is done. Thanks for your understanding!
+work is done. Thanks for your understanding!
Follow the
@@ -93,7 +93,7 @@
manually. For example, if IntelliJ IDEA shows
`Cannot find parent 'tsconfig.json'` error, you can try to setup typescript
options `--project polygerrit-ui/app/tsconfig.json` in the IDE settings.
-
+
## Serving files locally
@@ -171,29 +171,58 @@
For daily development you typically only want to run and debug individual tests.
There are several ways to run tests.
-* Run all tests in headless mode:
+* Run all tests in headless mode (exactly like CI does):
```sh
npm run test
```
+This command uses bazel rules for running frontend tests. Bazel fetches
+all nessecary dependencies and runs all required rules.
* Run all tests in debug mode (the command opens Chrome browser with
the default Karma page; you should click the "Debug" button to start testing):
```sh
+# The following command doesn't compile code before tests
npm run test:debug
```
* Run a single test file:
```
-# Headless mode
+# Headless mode (doesn't compile code before run)
npm run test:single async-foreach-behavior_test.js
-# Debug mode
+
+# Debug mode (doesn't compile code before run)
+npm run test:debug async-foreach-behavior_test.js
+```
+
+Commands `test:debug` and `test:single` assumes that compiled code is located
+in the `./ts-out/polygerrit-ui/app` directory. It's up to you how to achieve it.
+For example, the following options are possible:
+* You can configure IDE for recompiling source code on changes
+* You can use `compile:local` command for running compiler once and
+`compile:watch` for running compiler in watch mode (`compile:...` places
+compile code exactly in the `./ts-out/polygerrit-ui/app` directory)
+
+```sh
+# Compile frontend once and run tests from a file:
+npm run compile:local && npm run test:single async-foreach-behavior_test.js
+
+# Watch mode:
+## Terminal 1:
+npm run compile:watch
+## Terminal 2:
npm run test:debug async-foreach-behavior_test.js
```
* You can run tests in IDE. :
- [IntelliJ: running unit tests on Karma](https://www.jetbrains.com/help/idea/running-unit-tests-on-karma.html#ws_karma_running)
- You should configure IDE to compile typescript before running tests.
-
+
+**NOTE**: Bazel plugin for IntelliJ has a bug - it recompiles typescript
+project only if .ts and/or .d.ts files have been changed. If only .js files
+were changed, the plugin doesn't run compiler. As a workaround, setup
+"Run npm script 'compile:local" action instead of the "Compile Typescript" in
+the "Before launch" section for IntelliJ. This is a temporary problem until
+typescript migration is complete.
## Style guide
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js
index 2db1c09..b525a82 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js
@@ -577,6 +577,7 @@
detail: {
event: e,
goKey: this._inGoKeyMode(),
+ vKey: this._inVKeyMode(),
},
composed: true, bubbles: true,
}));
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 3fb8442..0690a87 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -281,7 +281,7 @@
type: Boolean,
computed: '_computeSendButtonDisabled(canBeStarted, ' +
'draftCommentThreads, draft, _reviewersMutated, _labelsChanged, ' +
- '_includeComments, disabled, _commentEditing)',
+ '_includeComments, disabled, _commentEditing, _attentionModified)',
observer: '_sendDisabledChanged',
},
draftCommentThreads: {
@@ -1028,7 +1028,8 @@
_computeSendButtonDisabled(
canBeStarted, draftCommentThreads, text, reviewersMutated,
- labelsChanged, includeComments, disabled, commentEditing) {
+ labelsChanged, includeComments, disabled, commentEditing,
+ attentionModified) {
// Polymer 2: check for undefined
if ([
canBeStarted,
@@ -1039,14 +1040,15 @@
includeComments,
disabled,
commentEditing,
+ attentionModified,
].includes(undefined)) {
return undefined;
}
-
if (commentEditing || disabled) { return true; }
if (canBeStarted === true) { return false; }
const hasDrafts = includeComments && draftCommentThreads.length;
- return !hasDrafts && !text.length && !reviewersMutated && !labelsChanged;
+ return !hasDrafts && !text.length && !reviewersMutated && !labelsChanged &&
+ !attentionModified;
}
_computePatchSetWarning(patchNum, labelsChanged) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
index 456adb5..29f1e8a 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
@@ -1235,7 +1235,8 @@
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
));
});
@@ -1250,7 +1251,24 @@
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
+ ));
+ });
+
+ test('_computeSendButtonDisabled_attentionModified true', () => {
+ const fn = element._computeSendButtonDisabled.bind(element);
+ // Mock everything false
+ assert.isFalse(fn(
+ /* canBeStarted= */ false,
+ /* draftCommentThreads= */ [],
+ /* text= */ '',
+ /* reviewersMutated= */ false,
+ /* labelsChanged= */ false,
+ /* includeComments= */ false,
+ /* disabled= */ false,
+ /* commentEditing= */ false,
+ /* attentionModified= */ true
));
});
@@ -1265,7 +1283,8 @@
/* labelsChanged= */ false,
/* includeComments= */ true,
/* disabled= */ false,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
));
});
@@ -1280,7 +1299,8 @@
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
));
});
@@ -1295,7 +1315,8 @@
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
));
});
@@ -1310,7 +1331,8 @@
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
));
});
@@ -1325,7 +1347,8 @@
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ false,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
));
});
@@ -1340,7 +1363,8 @@
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ true,
- /* commentEditing= */ false
+ /* commentEditing= */ false,
+ /* attentionModified= */ false
));
assert.isTrue(fn(
/* buttonLabel= */ 'Send',
@@ -1350,7 +1374,8 @@
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ false,
- /* commentEditing= */ true
+ /* commentEditing= */ true,
+ /* attentionModified= */ false
));
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
index 665e4a6..b82d4b8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
@@ -23,6 +23,7 @@
import {htmlTemplate} from './gr-diff-highlight_html.js';
import {GrAnnotation} from './gr-annotation.js';
import {GrRangeNormalizer} from './gr-range-normalizer.js';
+import {strToClassName} from '../../../utils/dom-util.js';
/**
* @extends PolymerElement
@@ -125,20 +126,27 @@
// As gr-ranged-comment-layer now does not notify the layer re-render and
// lack of access to the thread or the lineEl from the ranged-comment-layer,
// need to update range class for styles here.
- const currentLine = threadEl.assignedSlot.parentElement.previousSibling;
- if (currentLine && currentLine.querySelector) {
+ let curNode = threadEl.assignedSlot;
+ while (curNode) {
+ if (curNode.nodeName === 'TABLE') break;
+ curNode = curNode.parentElement;
+ }
+ if (curNode && curNode.querySelectorAll) {
if (highlightRange) {
- const rangeNode = currentLine.querySelector('.range');
- if (rangeNode) {
+ const rangeNodes = curNode
+ .querySelectorAll(`.range.${strToClassName(threadEl.rootId)}`);
+ rangeNodes.forEach(rangeNode => {
rangeNode.classList.add('rangeHighlight');
rangeNode.classList.remove('range');
- }
+ });
} else {
- const rangeNode = currentLine.querySelector('.rangeHighlight');
- if (rangeNode) {
+ const rangeNodes = curNode.querySelectorAll(
+ `.rangeHighlight.${strToClassName(threadEl.rootId)}`
+ );
+ rangeNodes.forEach(rangeNode => {
rangeNode.classList.remove('rangeHighlight');
rangeNode.classList.add('range');
- }
+ });
}
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index ad72ee1..d3cdfbe 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -563,8 +563,9 @@
this.$.cursor.moveToNextCommentThread();
} else {
if (this.modifierPressed(e)) { return; }
+ // navigate to next file if key is not being held down
this.$.cursor.moveToNextChunk(/* opt_clipToTop = */false,
- /* opt_navigateToNextFile = */true);
+ /* opt_navigateToNextFile = */!e.detail.keyboardEvent.repeat);
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 252b9bc..4d96d53 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -352,7 +352,7 @@
function commentRangeFromThreadEl(threadEl) {
const side = threadEl.getAttribute('comment-side');
const range = JSON.parse(threadEl.getAttribute('range'));
- return {side, range, hovering: false};
+ return {side, range, hovering: false, rootId: threadEl.rootId};
}
const addedCommentRanges = addedThreadEls
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
index 806e147..6a9b0bf 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
@@ -297,12 +297,19 @@
previous: detail.patchNum,
current: e.detail.value,
latest: latestPatchNum,
+ commentCount: this.changeComments.computeCommentCount(
+ {patchNum: e.detail.value}),
});
detail.patchNum = e.detail.value;
} else {
if (this.patchNumEquals(detail.basePatchNum, e.detail.value)) return;
this.reporting.reportInteraction('left-patchset-changed',
- {previous: detail.basePatchNum, current: e.detail.value});
+ {
+ previous: detail.basePatchNum,
+ current: e.detail.value,
+ commentCount: this.changeComments.computeCommentCount(
+ {patchNum: e.detail.value}),
+ });
detail.basePatchNum = e.detail.value;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
index c774f1a..231e4b5 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
@@ -20,6 +20,7 @@
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {htmlTemplate} from './gr-ranged-comment-layer_html.js';
import {GrDiffLine} from '../gr-diff/gr-diff-line.js';
+import {strToClassName} from '../../../utils/dom-util.js';
// Polymer 1 adds # before array's key, while Polymer 2 doesn't
const HOVER_PATH_PATTERN = /^(commentRanges\.#?\d+)\.hovering$/;
@@ -92,7 +93,8 @@
for (const range of ranges) {
GrAnnotation.annotateElement(el, range.start,
range.end - range.start,
- range.hovering ? HOVER_HIGHLIGHT : RANGE_HIGHLIGHT);
+ (range.hovering ? HOVER_HIGHLIGHT : RANGE_HIGHLIGHT) +
+ ` ${strToClassName(range.rootId)}`);
}
}
@@ -136,11 +138,11 @@
// If the entire set of comments was changed.
if (record.path === 'commentRanges') {
this._rangesMap = {left: {}, right: {}};
- for (const {side, range, hovering} of record.value) {
+ for (const {side, range, rootId, hovering} of record.value) {
this._updateRangesMap({
side, range, hovering,
operation: (forLine, start, end, hovering) => {
- forLine.push({start, end, hovering});
+ forLine.push({start, end, hovering, rootId});
}});
}
}
@@ -150,7 +152,7 @@
if (match) {
// The #number indicates the key of that item in the array
// not the index, especially in polymer 1.
- const {side, range, hovering} = this.get(match[1]);
+ const {side, range, hovering, rootId} = this.get(match[1]);
this._updateRangesMap({
side, range, hovering, skipLayerUpdate: true,
@@ -158,6 +160,7 @@
const index = forLine.findIndex(lineRange =>
lineRange.start === start && lineRange.end === end);
forLine[index].hovering = hovering;
+ forLine[index].rootId = rootId;
}});
}
@@ -165,21 +168,22 @@
if (record.path === 'commentRanges.splices') {
for (const indexSplice of record.value.indexSplices) {
const removed = indexSplice.removed;
- for (const {side, range, hovering} of removed) {
+ for (const {side, range, hovering, rootId} of removed) {
this._updateRangesMap({
side, range, hovering, operation: (forLine, start, end) => {
const index = forLine.findIndex(lineRange =>
- lineRange.start === start && lineRange.end === end);
+ lineRange.start === start && lineRange.end === end &&
+ rootId === lineRange.rootId);
forLine.splice(index, 1);
}});
}
const added = indexSplice.object.slice(
indexSplice.index, indexSplice.index + indexSplice.addedCount);
- for (const {side, range, hovering} of added) {
+ for (const {side, range, hovering, rootId} of added) {
this._updateRangesMap({
side, range, hovering,
operation: (forLine, start, end, hovering) => {
- forLine.push({start, end, hovering});
+ forLine.push({start, end, hovering, rootId});
}});
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js
index 2ce0afa..5f32677 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js
@@ -36,6 +36,7 @@
start_character: 6,
start_line: 36,
},
+ rootId: 'a',
},
{
side: 'right',
@@ -45,6 +46,7 @@
start_character: 10,
start_line: 10,
},
+ rootId: 'b',
},
{
side: 'right',
@@ -54,6 +56,7 @@
start_character: 5,
start_line: 100,
},
+ rootId: 'c',
},
{
side: 'right',
@@ -63,6 +66,7 @@
start_character: 32,
start_line: 55,
},
+ rootId: 'd',
},
];
@@ -106,7 +110,7 @@
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
- assert.equal(lastCall.args[3], 'style-scope gr-diff range');
+ assert.equal(lastCall.args[3], 'style-scope gr-diff range generated_a');
});
test('type=Remove has-comment hovering', () => {
@@ -124,7 +128,9 @@
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
- assert.equal(lastCall.args[3], 'style-scope gr-diff rangeHighlight');
+ assert.equal(
+ lastCall.args[3], 'style-scope gr-diff rangeHighlight generated_a'
+ );
});
test('type=Both has-comment', () => {
@@ -141,7 +147,7 @@
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
- assert.equal(lastCall.args[3], 'style-scope gr-diff range');
+ assert.equal(lastCall.args[3], 'style-scope gr-diff range generated_a');
});
test('type=Both has-comment off side', () => {
@@ -169,7 +175,7 @@
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
- assert.equal(lastCall.args[3], 'style-scope gr-diff range');
+ assert.equal(lastCall.args[3], 'style-scope gr-diff range generated_b');
});
});
diff --git a/polygerrit-ui/app/elements/gr-app-element.js b/polygerrit-ui/app/elements/gr-app-element.js
index a01e8a4..db098c5 100644
--- a/polygerrit-ui/app/elements/gr-app-element.js
+++ b/polygerrit-ui/app/elements/gr-app-element.js
@@ -421,10 +421,11 @@
}
_handleShortcutTriggered(event) {
- const {event: e, goKey} = event.detail;
+ const {event: e, goKey, vKey} = event.detail;
// eg: {key: "k:keydown", ..., from: "gr-diff-view"}
let key = `${e.key}:${e.type}`;
if (goKey) key = 'g+' + key;
+ if (vKey) key = 'v+' + key;
if (e.shiftKey) key = 'shift+' + key;
if (e.ctrlKey) key = 'ctrl+' + key;
if (e.metaKey) key = 'meta+' + key;
diff --git a/polygerrit-ui/app/utils/dom-util.js b/polygerrit-ui/app/utils/dom-util.js
index a9f080f..e26bf74 100644
--- a/polygerrit-ui/app/utils/dom-util.js
+++ b/polygerrit-ui/app/utils/dom-util.js
@@ -175,4 +175,18 @@
element = element.parentElement;
}
return isDescendant;
+}
+
+/**
+ * Convert any string into a valid class name.
+ *
+ * For class names, naming rules:
+ * Must begin with a letter A-Z or a-z
+ * Can be followed by: letters (A-Za-z), digits (0-9), hyphens ("-"), and underscores ("_")
+ *
+ * @param {string} str
+ * @param {string} prefix
+ */
+export function strToClassName(str = '', prefix = 'generated_') {
+ return `${prefix}${str.replace(/[^a-zA-Z0-9-_]/g, '_')}`;
}
\ No newline at end of file
diff --git a/polygerrit-ui/app/utils/dom-util_test.js b/polygerrit-ui/app/utils/dom-util_test.js
index c317578..4306cd2 100644
--- a/polygerrit-ui/app/utils/dom-util_test.js
+++ b/polygerrit-ui/app/utils/dom-util_test.js
@@ -15,7 +15,7 @@
* limitations under the License.
*/
import '../test/common-test-setup-karma.js';
-import {getComputedStyleValue, querySelector, querySelectorAll, descendedFromClass, getEventPath} from './dom-util.js';
+import {strToClassName, getComputedStyleValue, querySelector, querySelectorAll, descendedFromClass, getEventPath} from './dom-util.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
@@ -136,4 +136,16 @@
querySelector(testEl, '.b')));
});
});
+
+ suite('strToClassName', () => {
+ test('basic tests', () => {
+ assert.equal(strToClassName(''), 'generated_');
+ assert.equal(strToClassName('11'), 'generated_11');
+ assert.equal(strToClassName('0.123'), 'generated_0_123');
+ assert.equal(strToClassName('0.123', 'prefix_'), 'prefix_0_123');
+ assert.equal(strToClassName('0>123', 'prefix_'), 'prefix_0_123');
+ assert.equal(strToClassName('0<123', 'prefix_'), 'prefix_0_123');
+ assert.equal(strToClassName('0+1+23', 'prefix_'), 'prefix_0_1_23');
+ });
+ });
});
\ No newline at end of file
diff --git a/resources/com/google/gerrit/server/mail/Comment.soy b/resources/com/google/gerrit/server/mail/Comment.soy
index 1eb016b..fc92b31 100644
--- a/resources/com/google/gerrit/server/mail/Comment.soy
+++ b/resources/com/google/gerrit/server/mail/Comment.soy
@@ -41,7 +41,7 @@
{for $group in $commentFiles}
// Insert a space before the newline so that Gmail does not mistakenly link
// the following line with the file link. See issue 9201.
- {$group.link}{sp}{\n}
+ {if $group.link}{$group.link}{sp}{/if}{\n}
{$group.title}:{\n}
{\n}
