Merge "Fix missing file name in diff view when file has no change but comments"
diff --git a/Documentation/dev-e2e-tests.txt b/Documentation/dev-e2e-tests.txt
index 012a731..86221eb 100644
--- a/Documentation/dev-e2e-tests.txt
+++ b/Documentation/dev-e2e-tests.txt
@@ -18,7 +18,9 @@
to run tests at Git protocol level.
Gatling is written in Scala, but the abstraction provided by the Gatling DSL makes the scenarios
-implementation easy even without any Scala knowledge.
+implementation easy even without any Scala knowledge. The
+link:https://gitenterprise.me/2019/12/20/stress-your-gerrit-with-gatling/[Stress your Gerrit with Gatling]
+blog post has more introductory information.
Examples of scenarios can be found in the `e2e-tests` directory. The files in that directory
should be formatted using the mainstream
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 9714e18..3266cb1 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -476,9 +476,9 @@
----
Plugins which define new Events should register them via the
-`com.google.gerrit.server.events.EventTypes.registerClass()`
-method. This will make the EventType known to the system.
-Deserializing events with the
+`com.google.gerrit.server.events.EventTypes.register()` method.
+This will make the EventType known to the system. Deserializing
+events with the
`com.google.gerrit.server.events.EventDeserializer` class requires
that the event be registered in EventTypes.
diff --git a/e2e-tests/load-tests/project/build.properties b/e2e-tests/load-tests/project/build.properties
index 0cd8b07..a82bb05 100644
--- a/e2e-tests/load-tests/project/build.properties
+++ b/e2e-tests/load-tests/project/build.properties
@@ -1 +1 @@
-sbt.version=1.2.3
+sbt.version=1.3.7
diff --git a/e2e-tests/load-tests/src/test/resources/data/requests.json b/e2e-tests/load-tests/src/test/resources/data/ReplayRecordsFromFeeder.json
similarity index 100%
rename from e2e-tests/load-tests/src/test/resources/data/requests.json
rename to e2e-tests/load-tests/src/test/resources/data/ReplayRecordsFromFeeder.json
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
index b02657a..9e2aca0 100644
--- a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
+++ b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/CloneUsingBothProtocols.scala
@@ -44,8 +44,8 @@
setUp(
test.inject(
- constantUsersPerSec(1) during (2 seconds))
- ).protocols(protocol)
+ constantUsersPerSec(1) during (2 seconds)
+ )).protocols(protocol)
after {
Thread.sleep(5000)
diff --git a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
index c0eab39..5a3bb99 100644
--- a/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
+++ b/e2e-tests/load-tests/src/test/scala/com/google/gerrit/scenarios/ReplayRecordsFromFeeder.scala
@@ -14,59 +14,55 @@
package com.google.gerrit.scenarios
-import com.github.barbasa.gatling.git.protocol.GitProtocol
-import com.github.barbasa.gatling.git.request.builder.GitRequestBuilder
-import io.gatling.core.Predef._
-import io.gatling.core.structure.ScenarioBuilder
import java.io._
-import com.github.barbasa.gatling.git.{
- GatlingGitConfiguration,
- GitRequestSession
-}
+import com.github.barbasa.gatling.git.protocol.GitProtocol
+import com.github.barbasa.gatling.git.request.builder.GitRequestBuilder
+import com.github.barbasa.gatling.git.{GatlingGitConfiguration, GitRequestSession}
+import io.gatling.core.Predef._
+import io.gatling.core.feeder.FileBasedFeederBuilder
+import io.gatling.core.structure.ScenarioBuilder
import org.apache.commons.io.FileUtils
-
-import scala.concurrent.duration._
import org.eclipse.jgit.hooks._
-class ReplayRecordsFromFeederScenario extends Simulation {
+import scala.concurrent.duration._
- val gitProtocol = GitProtocol()
- implicit val conf = GatlingGitConfiguration()
- implicit val postMessageHook: Option[String] = Some(
- s"hooks/${CommitMsgHook.NAME}")
+class ReplayRecordsFromFeeder extends Simulation {
- val feeder = jsonFile("data/requests.json").circular
+ implicit val conf: GatlingGitConfiguration = GatlingGitConfiguration()
+ implicit val postMessageHook: Option[String] = Some(s"hooks/${CommitMsgHook.NAME}")
- val replayCallsScenario: ScenarioBuilder =
- scenario("Git commands")
+ private val name: String = this.getClass.getSimpleName
+ private val file = s"data/$name.json"
+ private val data: FileBasedFeederBuilder[Any]#F = jsonFile(file).circular
+ private val request = new GitRequestBuilder(GitRequestSession("${cmd}", "${url}"))
+ private val protocol: GitProtocol = GitProtocol()
+
+ private val test: ScenarioBuilder = scenario(name)
.repeat(10000) {
- feed(feeder)
- .exec(new GitRequestBuilder(GitRequestSession("${cmd}", "${url}")))
+ feed(data)
+ .exec(request)
}
setUp(
- replayCallsScenario.inject(
+ test.inject(
nothingFor(4 seconds),
atOnceUsers(10),
rampUsers(10) during (5 seconds),
constantUsersPerSec(20) during (15 seconds),
constantUsersPerSec(20) during (15 seconds) randomized
- ))
- .protocols(gitProtocol)
- .maxDuration(60 seconds)
+ )).protocols(protocol)
+ .maxDuration(60 seconds)
after {
+ Thread.sleep(5000)
+ val path = conf.tmpBasePath
try {
- //After is often called too early. Some retries should be implemented.
- Thread.sleep(5000)
- FileUtils.deleteDirectory(new File(conf.tmpBasePath))
+ FileUtils.deleteDirectory(new File(path))
} catch {
- case e: IOException => {
- System.err.println(
- "Unable to delete temporary directory: " + conf.tmpBasePath)
- e.printStackTrace
- }
+ case e: IOException =>
+ System.err.println("Unable to delete temporary directory " + path)
+ e.printStackTrace()
}
}
}
diff --git a/java/com/google/gerrit/extensions/systemstatus/MessageOfTheDay.java b/java/com/google/gerrit/extensions/systemstatus/MessageOfTheDay.java
deleted file mode 100644
index 180a0e6..0000000
--- a/java/com/google/gerrit/extensions/systemstatus/MessageOfTheDay.java
+++ /dev/null
@@ -1,63 +0,0 @@
-// Copyright (C) 2014 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.extensions.systemstatus;
-
-import com.google.gerrit.extensions.annotations.ExtensionPoint;
-import java.util.Calendar;
-import java.util.Date;
-import java.util.TimeZone;
-
-/**
- * Supplies a message of the day when the page is first loaded.
- *
- * <pre>
- * DynamicSet.bind(binder(), MessageOfTheDay.class).to(MyMessage.class);
- * </pre>
- */
-@ExtensionPoint
-public abstract class MessageOfTheDay {
- /**
- * Retrieve the message of the day as an HTML fragment.
- *
- * @return message as an HTML fragment; null if no message is available.
- */
- public abstract String getHtmlMessage();
-
- /**
- * Unique identifier for this message.
- *
- * <p>Messages with the same identifier will be hidden from the user until redisplay has occurred.
- *
- * @return unique message identifier. This identifier should be unique within the server.
- */
- public abstract String getMessageId();
-
- /**
- * When should the message be displayed?
- *
- * <p>Default implementation returns {@code tomorrow at 00:00:00 GMT}.
- *
- * @return a future date after which the message should be redisplayed.
- */
- public Date getRedisplay() {
- Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("GMT"));
- cal.set(Calendar.HOUR_OF_DAY, 0);
- cal.set(Calendar.MINUTE, 0);
- cal.set(Calendar.SECOND, 0);
- cal.set(Calendar.MILLISECOND, 0);
- cal.add(Calendar.DAY_OF_MONTH, 1);
- return cal.getTime();
- }
-}
diff --git a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
index 438f70e..3819786 100644
--- a/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
+++ b/java/com/google/gerrit/metrics/proc/JGitMetricModule.java
@@ -28,12 +28,12 @@
new Description("Bytes of memory retained in JGit block cache.")
.setGauge()
.setUnit(Units.BYTES),
- WindowCacheStats::getOpenBytes);
+ () -> WindowCacheStats.getStats().getOpenByteCount());
metrics.newCallbackMetric(
"jgit/block_cache/open_files",
- Integer.class,
+ Long.class,
new Description("File handles held open by JGit block cache.").setGauge().setUnit("fds"),
- WindowCacheStats::getOpenFiles);
+ () -> WindowCacheStats.getStats().getOpenFileCount());
}
}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 012e4cd..3914205 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -61,7 +61,6 @@
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
-import com.google.gerrit.extensions.systemstatus.MessageOfTheDay;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.extensions.webui.BranchWebLink;
import com.google.gerrit.extensions.webui.DiffWebLink;
@@ -365,7 +364,6 @@
DynamicItem.itemOf(binder(), AvatarProvider.class);
DynamicSet.setOf(binder(), LifecycleListener.class);
DynamicSet.setOf(binder(), TopMenu.class);
- DynamicSet.setOf(binder(), MessageOfTheDay.class);
DynamicMap.mapOf(binder(), DownloadScheme.class);
DynamicMap.mapOf(binder(), DownloadCommand.class);
DynamicMap.mapOf(binder(), CloneCommand.class);
diff --git a/java/com/google/gerrit/server/restapi/config/GetSummary.java b/java/com/google/gerrit/server/restapi/config/GetSummary.java
index 1df485f..d0a1498 100644
--- a/java/com/google/gerrit/server/restapi/config/GetSummary.java
+++ b/java/com/google/gerrit/server/restapi/config/GetSummary.java
@@ -126,8 +126,8 @@
long mTotal = r.totalMemory();
long mInuse = mTotal - mFree;
- int jgitOpen = WindowCacheStats.getOpenFiles();
- long jgitBytes = WindowCacheStats.getOpenBytes();
+ long jgitOpen = WindowCacheStats.getStats().getOpenFileCount();
+ long jgitBytes = WindowCacheStats.getStats().getOpenByteCount();
MemSummaryInfo memSummaryInfo = new MemSummaryInfo();
memSummaryInfo.total = bytes(mTotal);
@@ -135,7 +135,7 @@
memSummaryInfo.free = bytes(mFree);
memSummaryInfo.buffers = bytes(jgitBytes);
memSummaryInfo.max = bytes(mMax);
- memSummaryInfo.openFiles = toInteger(jgitOpen);
+ memSummaryInfo.openFiles = Long.valueOf(jgitOpen);
return memSummaryInfo;
}
@@ -258,7 +258,7 @@
public String free;
public String buffers;
public String max;
- public Integer openFiles;
+ public Long openFiles;
}
public static class ThreadSummaryInfo {
diff --git a/java/com/google/gerrit/sshd/commands/ShowCaches.java b/java/com/google/gerrit/sshd/commands/ShowCaches.java
index 7e0439f..1d756de 100644
--- a/java/com/google/gerrit/sshd/commands/ShowCaches.java
+++ b/java/com/google/gerrit/sshd/commands/ShowCaches.java
@@ -297,6 +297,10 @@
return i != null ? i : 0;
}
+ private static long nullToZero(Long i) {
+ return i != null ? i : 0;
+ }
+
private void sshSummary() {
IoAcceptor acceptor = daemon.getIoAcceptor();
if (acceptor == null) {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index cdffa3c..9624ec2 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -57,7 +57,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -645,14 +644,7 @@
// |
// C0 -- Master
//
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .getProject()
- .setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- InheritableBoolean.TRUE);
- u.save();
- }
+ enableCreateNewChangeForAllNotInTarget();
PushOneCommit push1 =
pushFactory.create(admin.newIdent(), testRepo, PushOneCommit.SUBJECT, "a.txt", "content");
diff --git a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
index 5a9213b..d1980a5 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-pointer-dialog/gr-create-pointer-dialog.html
@@ -38,7 +38,10 @@
input {
width: 20em;
}
- .hideItem {
+ /* Add css selector with #id to increase priority
+ (otherwise ".gr-form-styles section" rule wins) */
+ .hideItem,
+ #itemAnnotationSection.hideItem {
display: none;
}
</style>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
index e3a3b31..5035c92 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.js
@@ -18,7 +18,7 @@
'use strict';
const LookupQueryPatterns = {
- CHANGE_ID: /^\s*i?[0-9a-f]{8,40}\s*$/i,
+ CHANGE_ID: /^\s*i?[0-9a-f]{7,40}\s*$/i,
CHANGE_NUM: /^\s*[1-9][0-9]*\s*$/g,
};
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 0e86746..11e2217 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -353,6 +353,9 @@
z-index: var(--reply-overlay-z-index);
}
}
+ .patch-set-dropdown {
+ margin: var(--spacing-m) 0 0 var(--spacing-m);
+ }
</style>
<div class="container loading" hidden$="[[!_loading]]">Loading...</div>
<div
@@ -640,6 +643,12 @@
</template>
<template is="dom-if" if="[[_isSelectedView(_currentView,
_commentTabs.ROBOT_COMMENTS)]]">
+ <gr-dropdown-list
+ class="patch-set-dropdown"
+ items="[[_robotCommentsPatchSetDropdownItems]]"
+ on-value-change="_handleRobotCommentPatchSetChanged"
+ value="[[_currentRobotCommentsPatchSet]]">
+ </gr-dropdown-list>
<gr-thread-list
threads="[[_robotCommentThreads]]"
change="[[_change]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 124d10b..d2f8c87 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -141,7 +141,7 @@
_robotCommentThreads: {
type: Array,
computed: '_computeRobotCommentThreads(_commentThreads,'
- + ' _selectedRevision)',
+ + ' _currentRobotCommentsPatchSet)',
},
/** @type {?} */
_serverConfig: {
@@ -181,6 +181,7 @@
type: Object,
computed: '_computeCurrentRevision(_change.current_revision, ' +
'_change.revisions)',
+ observer: '_handleCurrentRevisionUpdate',
},
_files: Object,
_changeNum: String,
@@ -318,6 +319,14 @@
_selectedFilesTabPluginEndpoint: {
type: String,
},
+ _robotCommentsPatchSetDropdownItems: {
+ type: Array,
+ value() { return []; },
+ computed: '_computeRobotCommentsPatchSetDropdownItems(_change)',
+ },
+ _currentRobotCommentsPatchSet: {
+ type: Number,
+ },
};
}
@@ -584,12 +593,35 @@
return false;
}
- _computeRobotCommentThreads(commentThreads, selectedRevision) {
- if (!commentThreads || !selectedRevision) return [];
+ _computeRobotCommentsPatchSetDropdownItems(change) {
+ if (!change.revisions) return [];
+ return Object.values(change.revisions)
+ .filter(patch => patch._number !== 'edit')
+ .map(patch => {
+ return {
+ text: 'Patchset ' + patch._number,
+ value: patch._number,
+ };
+ })
+ .sort((a, b) => b.value - a.value);
+ }
+
+ _handleCurrentRevisionUpdate(currentRevision) {
+ this._currentRobotCommentsPatchSet = currentRevision._number;
+ }
+
+ _handleRobotCommentPatchSetChanged(e) {
+ const patchSet = parseInt(e.detail.value);
+ if (patchSet === this._currentRobotCommentsPatchSet) return;
+ this._currentRobotCommentsPatchSet = patchSet;
+ }
+
+ _computeRobotCommentThreads(commentThreads, currentRobotCommentsPatchSet) {
+ if (!commentThreads || !currentRobotCommentsPatchSet) return [];
return commentThreads.filter(thread => {
const comments = thread.comments || [];
return comments.length && comments[0].robot_id && (comments[0].patch_set
- === selectedRevision._number);
+ === currentRobotCommentsPatchSet);
});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 7224278..f0ab218 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -662,8 +662,18 @@
suite('Findings comment tab', () => {
setup(done => {
+ element._change = {
+ change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
+ revisions: {
+ rev2: {_number: 2, commit: {parents: []}},
+ rev1: {_number: 1, commit: {parents: []}},
+ rev13: {_number: 13, commit: {parents: []}},
+ rev3: {_number: 3, commit: {parents: []}},
+ rev4: {_number: 4, commit: {parents: []}},
+ },
+ current_revision: 'rev4',
+ };
element._commentThreads = THREADS;
- element._selectedRevision = {_number: 4};
flush(() => {
done();
});
@@ -676,7 +686,7 @@
'rc2');
});
test('changing patchsets resets robot comments', done => {
- element._selectedRevision = {_number: 3};
+ element.set('_change.current_revision', 'rev3');
flush(() => {
assert.equal(element._robotCommentThreads.length, 0);
done();
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
index 1a1276d..a845ed4 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
@@ -16,12 +16,15 @@
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
+<link rel="import" href="/bower_components/iron-icon/iron-icon.html">
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../plugins/gr-endpoint-decorator/gr-endpoint-decorator.html">
<link rel="import" href="../../plugins/gr-endpoint-param/gr-endpoint-param.html">
+
<link rel="import" href="../../../styles/shared-styles.html">
<dom-module id="gr-confirm-submit-dialog">
@@ -33,6 +36,11 @@
p {
margin-bottom: var(--spacing-l);
}
+ .warningBeforeSubmit {
+ color: var(--error-text-color);
+ vertical-align: top;
+ margin-right: var(--spacing-s);
+ }
@media screen and (max-width: 50em) {
#dialog {
min-width: inherit;
@@ -53,7 +61,17 @@
<gr-endpoint-decorator name="confirm-submit-change">
<p>Ready to submit “<strong>[[change.subject]]</strong>”?</p>
<template is="dom-if" if="[[change.is_private]]">
- <p><strong>Heads Up!</strong> Submitting this private change will also make it public.</p>
+ <p>
+ <iron-icon icon="gr-icons:error" class="warningBeforeSubmit"></iron-icon>
+ <strong>Heads Up!</strong>
+ Submitting this private change will also make it public.
+ </p>
+ </template>
+ <template is="dom-if" if="[[change.unresolved_comment_count]]">
+ <p>
+ <iron-icon icon="gr-icons:error" class="warningBeforeSubmit"></iron-icon>
+ [[_computeUnresolvedCommentsWarning(change)]]
+ </p>
</template>
<gr-endpoint-param name="change" value="[[change]]"></gr-endpoint-param>
<gr-endpoint-param name="action" value="[[action]]"></gr-endpoint-param>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
index ea8bdb5..aa26681 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.js
@@ -57,6 +57,12 @@
this.$.dialog.resetFocus();
}
+ _computeUnresolvedCommentsWarning(change) {
+ const unresolvedCount = change.unresolved_comment_count;
+ const plural = unresolvedCount > 1 ? 's' : '';
+ return `Heads Up! ${unresolvedCount} unresolved comment${plural}.`;
+ }
+
_handleConfirmTap(e) {
e.preventDefault();
e.stopPropagation();
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html
index 515147f..2ae58af 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog_test.html
@@ -61,5 +61,15 @@
assert.notEqual(message.textContent.length, 0);
assert.notEqual(message.textContent.indexOf('my-subject'), -1);
});
+
+ test('_computeUnresolvedCommentsWarning', () => {
+ const change = {unresolved_comment_count: 1};
+ assert.equal(element._computeUnresolvedCommentsWarning(change),
+ 'Heads Up! 1 unresolved comment.');
+
+ const change2 = {unresolved_comment_count: 2};
+ assert.equal(element._computeUnresolvedCommentsWarning(change2),
+ 'Heads Up! 2 unresolved comments.');
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html
index 46fd227..134dd5c 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row.html
@@ -27,42 +27,37 @@
/* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
</style>
<style include="shared-styles">
- .labelContainer {
- align-items: center;
- display: flex;
- margin-bottom: var(--spacing-m);
+ .labelNameCell,
+ .buttonsCell,
+ .selectedValueCell {
+ padding: var(--spacing-s) var(--spacing-m);
+ display: table-cell;
}
- .labelName {
- display: inline-block;
- flex: 0 0 auto;
- margin-right: var(--spacing-m);
- min-width: 7em;
- text-align: left;
- width: 20%;
+ /* We want the :hover highlight to extend to the border of the dialog. */
+ .labelNameCell {
+ padding-left: var(--spacing-xl);
+ }
+ .selectedValueCell {
+ padding-right: var(--spacing-xl);
+ }
+ /* This is a trick to let the selectedValueCell take the remaining width. */
+ .labelNameCell,
+ .buttonsCell {
+ white-space: nowrap;
+ }
+ .selectedValueCell {
+ width: 75%;
}
.labelMessage {
color: var(--deemphasized-text-color);
}
- .placeholder::before {
- content: ' ';
- }
- .selectedValueText {
- color: var(--deemphasized-text-color);
- font-style: italic;
- margin: 0 var(--spacing-m);
- }
- .selectedValueText.hidden {
- display: none;
- }
- .buttonWrapper {
- flex: none;
- }
gr-button {
- min-width: 40px;
+ min-width: 42px;
+ box-sizing: border-box;
--gr-button: {
background-color: var(--button-background-color, var(--table-header-background-color));
color: var(--primary-text-color);
- padding: var(--spacing-xs) var(--spacing-m);
+ padding: 0 var(--spacing-m);
@apply --vote-chip-styles;
}
}
@@ -83,63 +78,62 @@
}
.placeholder {
display: inline-block;
- width: 40px;
+ width: 42px;
+ height: 1px;
+ }
+ .placeholder::before {
+ content: ' ';
+ }
+ .selectedValueCell {
+ color: var(--deemphasized-text-color);
+ font-style: italic;
+ }
+ .selectedValueCell.hidden {
+ display: none;
}
@media only screen and (max-width: 50em) {
- .selectedValueText {
+ .selectedValueCell {
display: none;
}
}
- @media only screen and (max-width: 25em) {
- .labelName {
- margin: 0;
- text-align: center;
- width: 100%;
- }
- .labelContainer {
- display: block;
- }
- }
</style>
- <div class="labelContainer">
- <span class="labelName">[[label.name]]</span>
- <div class="buttonWrapper">
+ <span class="labelNameCell">[[label.name]]</span>
+ <div class="buttonsCell">
+ <template is="dom-repeat"
+ items="[[_computeBlankItems(permittedLabels, label.name, 'start')]]"
+ as="value">
+ <span class="placeholder" data-label$="[[label.name]]"></span>
+ </template>
+ <iron-selector
+ id="labelSelector"
+ attr-for-selected="data-value"
+ selected="[[_computeLabelValue(labels, permittedLabels, label)]]"
+ hidden$="[[!_computeAnyPermittedLabelValues(permittedLabels, label.name)]]"
+ on-selected-item-changed="_setSelectedValueText">
<template is="dom-repeat"
- items="[[_computeBlankItems(permittedLabels, label.name, 'start')]]"
+ items="[[_items]]"
as="value">
- <span class="placeholder" data-label$="[[label.name]]"></span>
+ <gr-button
+ class$="[[_computeButtonClass(value, index, _items.length)]]"
+ has-tooltip
+ data-name$="[[label.name]]"
+ data-value$="[[value]]"
+ title$="[[_computeLabelValueTitle(labels, label.name, value)]]">
+ [[value]]</gr-button>
</template>
- <iron-selector
- id="labelSelector"
- attr-for-selected="data-value"
- selected="[[_computeLabelValue(labels, permittedLabels, label)]]"
- hidden$="[[!_computeAnyPermittedLabelValues(permittedLabels, label.name)]]"
- on-selected-item-changed="_setSelectedValueText">
- <template is="dom-repeat"
- items="[[_items]]"
- as="value">
- <gr-button
- class$="[[_computeButtonClass(value, index, _items.length)]]"
- has-tooltip
- data-name$="[[label.name]]"
- data-value$="[[value]]"
- title$="[[_computeLabelValueTitle(labels, label.name, value)]]">
- [[value]]</gr-button>
- </template>
- </iron-selector>
- <template is="dom-repeat"
- items="[[_computeBlankItems(permittedLabels, label.name, 'end')]]"
- as="value">
- <span class="placeholder" data-label$="[[label.name]]"></span>
- </template>
- <span class="labelMessage"
- hidden$="[[_computeAnyPermittedLabelValues(permittedLabels, label.name)]]">
- You don't have permission to edit this label.
- </span>
- </div>
- <div class$="selectedValueText [[_computeHiddenClass(permittedLabels, label.name)]]">
- <span id="selectedValueLabel">[[_selectedValueText]]</span>
- </div>
+ </iron-selector>
+ <template is="dom-repeat"
+ items="[[_computeBlankItems(permittedLabels, label.name, 'end')]]"
+ as="value">
+ <span class="placeholder" data-label$="[[label.name]]"></span>
+ </template>
+ <span class="labelMessage"
+ hidden$="[[_computeAnyPermittedLabelValues(permittedLabels, label.name)]]">
+ You don't have permission to edit this label.
+ </span>
+ </div>
+ <div class$="selectedValueCell [[_computeHiddenClass(permittedLabels, label.name)]]">
+ <span id="selectedValueLabel">[[_selectedValueText]]</span>
</div>
</template>
<script src="gr-label-score-row.js"></script>
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html
index c607a9f..29b83a3 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.html
@@ -23,18 +23,23 @@
<dom-module id="gr-label-scores">
<template>
<style include="shared-styles">
+ :host {
+ display: table;
+ width: 100%;
+ }
.mergedMessage {
font-style: italic;
text-align: center;
width: 100%;
}
- gr-label-score-row.no-access {
- display: var(--label-no-access-display, initial);
+ gr-label-score-row:hover {
+ background-color: var(--hover-background-color);
}
- @media only screen and (max-width: 25em) {
- :host {
- text-align: center;
- }
+ gr-label-score-row {
+ display: table-row;
+ }
+ gr-label-score-row.no-access {
+ display: var(--label-no-access-display, table-row);
}
</style>
<template is="dom-repeat" items="[[_labels]]" as="label">
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index e77bf57..2ea1134 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -156,6 +156,9 @@
.commentsIcon {
vertical-align: top;
}
+ .score.removed {
+ background-color: var(--vote-color-neutral);
+ }
.score.negative {
background-color: var(--vote-color-disliked);
}
@@ -195,7 +198,7 @@
on behalf of
</span>
<gr-account-label account="[[author]]" class="authorLabel"></gr-account-label>
- <template is="dom-repeat" items="[[_getScores(message)]]" as="score">
+ <template is="dom-repeat" items="[[_getScores(message, labelExtremes)]]" as="score">
<span class$="score [[_computeScoreClass(score, labelExtremes)]]">
[[score.label]] [[score.value]]
</span>
@@ -215,7 +218,7 @@
class="message hideOnCollapsed"
content="[[_messageContentExpanded]]"
config="[[_projectConfig.commentlinks]]"></gr-formatted-text>
- <template is="dom-if" if="[[!_isMessageContentEmpty(message.message)]]">
+ <template is="dom-if" if="[[!_isMessageContentEmpty()]]">
<div class="replyContainer" hidden$="[[!showReplyButton]]" hidden>
<gr-button link small on-click="_handleReplyTap">Reply</gr-button>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.js b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
index 95109bb..d5505a4 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.js
@@ -17,8 +17,8 @@
(function() {
'use strict';
- const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+: /;
- const LABEL_TITLE_SCORE_PATTERN = /^([A-Za-z0-9-]+)([+-]\d+)$/;
+ const PATCH_SET_PREFIX_PATTERN = /^Patch Set \d+:\s*(.*)/;
+ const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?$/;
/**
* @appliesMixin Gerrit.FireMixin
@@ -99,11 +99,13 @@
},
_messageContentExpanded: {
type: String,
- computed: '_computeMessageContentExpanded(message.message)',
+ computed:
+ '_computeMessageContentExpanded(message.message, message.tag)',
},
_messageContentCollapsed: {
type: String,
- computed: '_computeMessageContentCollapsed(message.message)',
+ computed:
+ '_computeMessageContentCollapsed(message.message, message.tag)',
},
_commentCountText: {
type: Number,
@@ -166,29 +168,54 @@
}
}
- _computeMessageContentExpanded(content) {
- return this._computeMessageContent(content, true);
+ _computeMessageContentExpanded(content, tag) {
+ return this._computeMessageContent(content, tag, true);
}
- _computeMessageContentCollapsed(content) {
- return this._computeMessageContent(content, false);
+ _computeMessageContentCollapsed(content, tag) {
+ return this._computeMessageContent(content, tag, false);
}
- _computeMessageContent(content, isExpanded) {
- if (!content) return '';
+ _computeMessageContent(content, tag, isExpanded) {
+ content = content || '';
+ tag = tag || '';
+ const isNewPatchSet = tag.endsWith(':newPatchSet') ||
+ tag.endsWith(':newWipPatchSet');
const lines = content.split('\n');
const filteredLines = lines.filter(line => {
- if (!isExpanded && line.startsWith('>')) return false;
- if (line.startsWith('Patch Set ')) return false;
- if (line.startsWith('(') && line.endsWith(' comment)')) return false;
- if (line.startsWith('(') && line.endsWith(' comments)')) return false;
+ if (!isExpanded && line.startsWith('>')) {
+ return false;
+ }
+ if (line.startsWith('(') && line.endsWith(' comment)')) {
+ return false;
+ }
+ if (line.startsWith('(') && line.endsWith(' comments)')) {
+ return false;
+ }
+ if (!isNewPatchSet && line.match(PATCH_SET_PREFIX_PATTERN)) {
+ return false;
+ }
return true;
});
- return filteredLines.join('\n').trim();
+ const mappedLines = filteredLines.map(line => {
+ // The change message formatting is not very consistent, so
+ // unfortunately we have to do a bit of tweaking here:
+ // Labels should be stripped from lines like this:
+ // Patch Set 29: Verified+1
+ // Rebase messages (which have a ':newPatchSet' tag) should be kept on
+ // lines like this:
+ // Patch Set 27: Patch Set 26 was rebased
+ if (isNewPatchSet) {
+ line = line.replace(PATCH_SET_PREFIX_PATTERN, '$1');
+ }
+ return line;
+ });
+ return mappedLines.join('\n').trim();
}
- _isMessageContentEmpty(content) {
- return this._computeMessageContent(content).trim().length === 0;
+ _isMessageContentEmpty() {
+ return !this._messageContentExpanded
+ || this._messageContentExpanded.length === 0;
}
_computeAuthor(message) {
@@ -247,17 +274,28 @@
return event.type === 'REVIEWER_UPDATE';
}
- _getScores(message) {
- if (!message.message) { return []; }
+ _getScores(message, labelExtremes) {
+ if (!message || !message.message || !labelExtremes) {
+ return [];
+ }
const line = message.message.split('\n', 1)[0];
const patchSetPrefix = PATCH_SET_PREFIX_PATTERN;
- if (!line.match(patchSetPrefix)) { return []; }
+ if (!line.match(patchSetPrefix)) {
+ return [];
+ }
const scoresRaw = line.split(patchSetPrefix)[1];
- if (!scoresRaw) { return []; }
+ if (!scoresRaw) {
+ return [];
+ }
return scoresRaw.split(' ')
.map(s => s.match(LABEL_TITLE_SCORE_PATTERN))
- .filter(ms => ms && ms.length === 3)
- .map(ms => { return {label: ms[1], value: ms[2]}; });
+ .filter(ms =>
+ ms && ms.length === 4 && labelExtremes.hasOwnProperty(ms[2]))
+ .map(ms => {
+ const label = ms[2];
+ const value = ms[1] === '-' ? 'removed' : ms[3];
+ return {label, value};
+ });
}
_computeScoreClass(score, labelExtremes) {
@@ -265,6 +303,9 @@
if ([score, labelExtremes].some(arg => arg === undefined)) {
return '';
}
+ if (score.value === 'removed') {
+ return 'removed';
+ }
const classes = [];
if (score.value > 0) {
classes.push('positive');
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
index 01bc691..20d87d2 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.html
@@ -184,16 +184,72 @@
assert.deepEqual(stub.lastCall.args[0].detail, {id: element.message.id});
});
+ suite('compute messages', () => {
+ test('empty', () => {
+ assert.equal(element._computeMessageContent('', '', true), '');
+ assert.equal(element._computeMessageContent('', '', false), '');
+ });
+
+ test('new patchset', () => {
+ const original = 'Uploaded patch set 1.';
+ const tag = 'autogenerated:gerrit:newPatchSet';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, original);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, original);
+ });
+
+ test('new patchset rebased', () => {
+ const original = 'Patch Set 27: Patch Set 26 was rebased';
+ const tag = 'autogenerated:gerrit:newPatchSet';
+ const expected = 'Patch Set 26 was rebased';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+
+ test('ready for review', () => {
+ const original = 'Patch Set 1:\n\nThis change is ready for review.';
+ const tag = undefined;
+ const expected = 'This change is ready for review.';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+
+ test('vote', () => {
+ const original = 'Patch Set 1: Code-Style+1';
+ const tag = undefined;
+ const expected = '';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+
+ test('comments', () => {
+ const original = 'Patch Set 1:\n\n(3 comments)';
+ const tag = undefined;
+ const expected = '';
+ let actual = element._computeMessageContent(original, tag, true);
+ assert.equal(actual, expected);
+ actual = element._computeMessageContent(original, tag, false);
+ assert.equal(actual, expected);
+ });
+ });
+
test('votes', () => {
element.message = {
author: {},
expanded: false,
- message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Ready+1',
+ message: 'Patch Set 1: Verified+1 Code-Review-2 Trybot-Label3+1 Blub+1',
};
element.labelExtremes = {
'Verified': {max: 1, min: -1},
'Code-Review': {max: 2, min: -2},
- 'Trybot-Ready': {max: 3, min: 0},
+ 'Trybot-Label3': {max: 3, min: 0},
};
flushAsynchronousOperations();
const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
@@ -209,6 +265,25 @@
assert.isFalse(scoreChips[2].classList.contains('min'));
});
+ test('removed votes', () => {
+ element.message = {
+ author: {},
+ expanded: false,
+ message: 'Patch Set 1: Verified+1 -Code-Review -Commit-Queue',
+ };
+ element.labelExtremes = {
+ 'Verified': {max: 1, min: -1},
+ 'Code-Review': {max: 2, min: -2},
+ 'Commit-Queue': {max: 3, min: 0},
+ };
+ flushAsynchronousOperations();
+ const scoreChips = Polymer.dom(element.root).querySelectorAll('.score');
+ assert.equal(scoreChips.length, 3);
+
+ assert.isTrue(scoreChips[1].classList.contains('removed'));
+ assert.isTrue(scoreChips[2].classList.contains('removed'));
+ });
+
test('false negative vote', () => {
element.message = {
author: {},
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index cd759d3..20680b5 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -367,8 +367,7 @@
acc[val] = (acc[val] || 0) + 1;
return acc;
}, {all: messages.length});
- this.$.reporting.reportInteraction('messages-count',
- JSON.stringify(tagsCounted));
+ this.$.reporting.reportInteraction('messages-count', tagsCounted);
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
index b0747f4..9c3f53b 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
@@ -553,6 +553,23 @@
assert.isFalse(element._hasAutomatedMessages(messages));
});
+ test('initially show only 20 messages', () => {
+ sandbox.stub(element.$.reporting, 'reportInteraction',
+ (eventName, details) => {
+ assert.equal(typeof(eventName), 'string');
+ if (details) {
+ assert.equal(typeof(details), 'object');
+ }
+ });
+ const messages = Array.from(Array(23).keys())
+ .map(() => {
+ return {};
+ });
+ element._processedMessagesChanged(messages);
+
+ assert.equal(element._visibleMessages.length, 20);
+ });
+
test('_computeLabelExtremes', () => {
const computeSpy = sandbox.spy(element, '_computeLabelExtremes');
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index d54669f..05a6209f 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -64,6 +64,10 @@
padding: var(--spacing-m) var(--spacing-xl);
width: 100%;
}
+ section.labelsContainer {
+ /* We want the :hover highlight to extend to the border of the dialog. */
+ padding: var(--spacing-m) 0;
+ }
.actions {
background-color: var(--dialog-background-color);
bottom: 0;
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
index 1569094..06edb9b 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html
@@ -67,10 +67,10 @@
</div>
<div class="diffContainer">
<gr-diff
- prefs="[[overridePartialPrefs(prefs)]]"
- change-num="[[changeNum]]"
- path="[[item.filepath]]"
- diff="[[item.preview]]"></gr-diff>
+ prefs="[[overridePartialPrefs(prefs)]]"
+ change-num="[[changeNum]]"
+ path="[[item.filepath]]"
+ diff="[[item.preview]]"></gr-diff>
</div>
</template>
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
index 7561f1b..45e8c07 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js
@@ -75,6 +75,18 @@
});
},
+ attached() {
+ this.refitOverlay = () => {
+ // re-center the dialog as content changed
+ this.$.applyFixOverlay.fire('iron-resize');
+ };
+ this.addEventListener('diff-context-expanded', this.refitOverlay);
+ },
+
+ detached() {
+ this.removeEventListener('diff-context-expanded', this.refitOverlay);
+ },
+
_showSelectedFixSuggestion(fixSuggestion) {
this._currentFix = fixSuggestion;
return this._fetchFixPreview(fixSuggestion.fix_id);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 40fbe3c..021d962 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -18,6 +18,7 @@
<link rel="import" href="../../../behaviors/fire-behavior/fire-behavior.html">
<link rel="import" href="../gr-coverage-layer/gr-coverage-layer.html">
<link rel="import" href="../gr-diff-processor/gr-diff-processor.html">
+<link rel="import" href="../../../elements/shared/gr-hovercard/gr-hovercard.html">
<link rel="import" href="../gr-ranged-comment-layer/gr-ranged-comment-layer.html">
<dom-module id="gr-diff-builder">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 33cd4cb..05bff9f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -574,14 +574,30 @@
const date = (new Date(commit.time * 1000)).toLocaleDateString();
const blameNode = this._createElement('span',
isStartOfRange ? 'startOfRange' : '');
- const shaNode = this._createElement('span', 'sha');
- shaNode.innerText = commit.id.substr(0, 7);
- shaNode.onclick = function() {
- location.href = '/q/' + shaNode.innerText;
- };
+ const shaNode = this._createElement('span', 'blameDate');
+ shaNode.innerText = `${date}`;
+ shaNode.onclick = function() {
+ location.href = '/q/' + commit.id;
+ };
blameNode.appendChild(shaNode);
- blameNode.append(` on ${date} by ${commit.author}`);
+
+ const shortName = commit.author.split(' ')[0];
+ const authorNode = this._createElement('span', 'blameAuthor');
+ authorNode.innerText = ` ${shortName}`;
+ blameNode.appendChild(authorNode);
+
+ const hoverCardFragment = this._createElement('span', 'blameHoverCard');
+ hoverCardFragment.innerText =
+ `Commit ${commit.id}
+Author: ${commit.author}
+Date: ${date}
+
+${commit.commit_msg}`;
+ const hovercard = this._createElement('gr-hovercard');
+ hovercard.appendChild(hoverCardFragment);
+ blameNode.appendChild(hovercard);
+
return blameNode;
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 867a067..589d7c2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -1146,6 +1146,30 @@
assert.equal(result.getAttribute('data-line-number'), '3');
assert.equal(result.firstChild, mocbBlameCell);
});
+
+ test('_getBlameForBaseLine', () => {
+ const mockCommit = {
+ time: 1576105200,
+ id: 1234567890,
+ author: 'Clark Kent',
+ commit_msg: 'Testing Commit',
+ ranges: [1],
+ };
+ const blameNode = builder._getBlameForBaseLine(1, mockCommit);
+
+ const authors = blameNode.getElementsByClassName('blameAuthor');
+ assert.equal(authors.length, 1);
+ assert.equal(authors[0].innerText, ' Clark');
+
+ const date = (new Date(mockCommit.time * 1000)).toLocaleDateString();
+ Polymer.dom.flush();
+ const cards = blameNode.getElementsByClassName('blameHoverCard');
+ assert.equal(cards.length, 1);
+ assert.equal(cards[0].innerHTML,
+ `Commit 1234567890<br>Author: Clark Kent<br>Date: ${date}`
+ + '<br><br>Testing Commit'
+ );
+ });
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
index 99d0498..1e2d963 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
@@ -25,7 +25,9 @@
scroll-behavior="[[_scrollBehavior]]"
cursor-target-class="target-row"
focus-on-move="[[_focusOnMove]]"
- target="{{diffRow}}"></gr-cursor-manager>
+ target="{{diffRow}}"
+ scroll-top-margin="[[scrollTopMargin]]"
+ ></gr-cursor-manager>
</template>
<script src="gr-diff-cursor.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index 726ac10..e8d629c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -96,6 +96,19 @@
},
_listeningForScroll: Boolean,
+
+ /**
+ * gr-diff-view has gr-fixed-panel on top. The panel can
+ * intersect a main element and partially hides a content of
+ * the main element. To correctly calculates visibility of an
+ * element, the cursor must know how much height occuped by a fixed
+ * panel.
+ * The scrollTopMargin defines margin occuped by fixed panel.
+ */
+ scrollTopMargin: {
+ type: Number,
+ value: 0,
+ },
};
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 7275ae5..42fc4a6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -210,7 +210,9 @@
class$="[[_computeContainerClass(_editMode)]]"
floating-disabled="[[_panelFloatingDisabled]]"
keep-on-scroll
- ready-for-measure="[[!_loading]]">
+ ready-for-measure="[[!_loading]]"
+ on-floating-height-changed="_onChangeHeaderPanelHeightChanged"
+ >
<header>
<h3>
<a href$="[[_computeChangePath(_change, _patchRange.*, _change.revisions)]]">
@@ -357,7 +359,7 @@
</gr-diff-preferences-dialog>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-storage id="storage"></gr-storage>
- <gr-diff-cursor id="cursor"></gr-diff-cursor>
+ <gr-diff-cursor id="cursor" scroll-top-margin="[[_scrollTopMargin]]"></gr-diff-cursor>
<gr-comment-api id="commentAPI"></gr-comment-api>
<gr-reporting id="reporting"></gr-reporting>
</template>
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 72527a1..2fbf59b 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
@@ -193,6 +193,19 @@
type: Object,
value: () => new Set(),
},
+
+ /**
+ * gr-diff-view has gr-fixed-panel on top. The panel can
+ * intersect a main element and partially hides a content of
+ * the main element. To correctly calculates visibility of an
+ * element, the cursor must know how much height occuped by a fixed
+ * panel.
+ * The scrollTopMargin defines margin occuped by fixed panel.
+ */
+ _scrollTopMargin: {
+ type: Number,
+ value: 0,
+ },
};
}
@@ -1169,6 +1182,10 @@
_handleReloadingDiffPreference() {
this._getDiffPreferences();
}
+
+ _onChangeHeaderPanelHeightChanged(e) {
+ this._scrollTopMargin = e.detail.value;
+ }
}
customElements.define(GrDiffView.is, GrDiffView);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index ff52ddd..172a151 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -82,6 +82,7 @@
}
.diff-row {
outline: none;
+ user-select: none;
}
.diff-row.target-row.target-side-left .lineNum.left,
.diff-row.target-row.target-side-right .lineNum.right,
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
index 54cd529..333ca9d 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
@@ -353,8 +353,7 @@
secondary
class="action show-fix"
hidden$="[[_hasNoFix(comment)]]"
- on-click="_handleShowFix"
- disabled="[[robotButtonDisabled]]">
+ on-click="_handleShowFix">
Show Fix
</gr-button>
<gr-endpoint-decorator name="robot-comment-controls">
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
index fd70fd9..37c7147 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
@@ -89,6 +89,16 @@
type: Boolean,
value: false,
},
+
+ /**
+ * The scrollTopMargin defines height of invisible area at the top
+ * of the page. If cursor locates inside this margin - it is
+ * not visible, because it is covered by some other element.
+ */
+ scrollTopMargin: {
+ type: Number,
+ value: 0,
+ },
};
}
@@ -304,13 +314,14 @@
_targetIsVisible(top) {
const dims = this._getWindowDims();
return this.scrollBehavior === ScrollBehavior.KEEP_VISIBLE &&
- top > dims.pageYOffset &&
+ top > (dims.pageYOffset + this.scrollTopMargin) &&
top < dims.pageYOffset + dims.innerHeight;
}
_calculateScrollToValue(top, target) {
const dims = this._getWindowDims();
- return top - (dims.innerHeight / 3) + (target.offsetHeight / 2);
+ return top + this.scrollTopMargin - (dims.innerHeight / 3) +
+ (target.offsetHeight / 2);
}
_scrollToTarget() {
diff --git a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
index 77418df..475f6e2 100644
--- a/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
+++ b/polygerrit-ui/app/elements/shared/gr-dialog/gr-dialog.html
@@ -27,6 +27,7 @@
color: var(--primary-text-color);
display: block;
max-height: 90vh;
+ overflow: auto;
}
.container {
display: flex;
@@ -42,6 +43,13 @@
display: flex;
flex-shrink: 1;
width: 100%;
+ flex: 1;
+ /* IMPORTANT: required for firefox */
+ min-height: 0px;
+ }
+ main .overflow-container {
+ flex: 1;
+ overflow: auto;
}
footer {
display: flex;
@@ -58,7 +66,11 @@
</style>
<div class="container" on-keydown="_handleKeydown">
<header class="font-h3"><slot name="header"></slot></header>
- <main><slot name="main"></slot></main>
+ <main>
+ <div class="overflow-container">
+ <slot name="main"></slot>
+ </div>
+ </main>
<footer>
<slot name="footer"></slot>
<gr-button id="cancel" class$="[[_computeCancelClass(cancelLabel)]]" link on-click="_handleCancelTap">
diff --git a/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js b/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js
index 02e23e8..d4995cc 100644
--- a/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js
+++ b/polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js
@@ -25,7 +25,10 @@
static get properties() {
return {
- floatingDisabled: Boolean,
+ floatingDisabled: {
+ type: Boolean,
+ value: false,
+ },
readyForMeasure: {
type: Boolean,
observer: '_readyForMeasureObserver',
@@ -58,10 +61,38 @@
type: Object,
value: null,
},
+ /**
+ * If place before any other content defines how much
+ * of the content below it is covered by this panel
+ */
+ floatingHeight: {
+ type: Number,
+ value: 0,
+ notify: true,
+ },
+
_webComponentsReady: Boolean,
};
}
+ static get observers() {
+ return [
+ '_updateFloatingHeight(floatingDisabled, _isMeasured, _headerHeight)',
+ ];
+ }
+
+ _updateFloatingHeight(floatingDisabled, isMeasured, headerHeight) {
+ if ([
+ floatingDisabled,
+ isMeasured,
+ headerHeight,
+ ].some(arg => arg === undefined)) {
+ return;
+ }
+ this.floatingHeight =
+ (!floatingDisabled && isMeasured) ? headerHeight : 0;
+ }
+
/** @override */
attached() {
super.attached();
