Merge "Include project name in ProjectConfig parsing log"
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 3e82254..8d30db2 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -561,7 +561,8 @@
+
To enable the actual usage of contributor agreement the project
specific config option in the `project.config` must be set:
-link:config-project-config.html[receive.requireContributorAgreement].
+link:config-project-config.html#receive.requireContributorAgreement[
+receive.requireContributorAgreement].
[[auth.trustContainerAuth]]auth.trustContainerAuth::
+
diff --git a/Documentation/pg-plugin-checks-api.txt b/Documentation/pg-plugin-checks-api.txt
index 8786cc4..e4cb5d0 100644
--- a/Documentation/pg-plugin-checks-api.txt
+++ b/Documentation/pg-plugin-checks-api.txt
@@ -10,6 +10,10 @@
when a change page is loaded. Such a call would return a list of `Runs` and each
run can contain a list of `Results`.
+`Results` messages will render as markdown. It follows the
+[CommonMark](https://commonmark.org/help/) spec except inline images and direct
+HTML are not rendered and kept as plaintext.
+
The details of the ChecksApi are documented in the
link:https://gerrit.googlesource.com/gerrit/+/master/polygerrit-ui/app/api/checks.ts[source code].
Note that this link points to the `master` branch and might thus reflect a
diff --git a/Documentation/user-attention-set.txt b/Documentation/user-attention-set.txt
index c4b8aa7..5e5d3f8 100644
--- a/Documentation/user-attention-set.txt
+++ b/Documentation/user-attention-set.txt
@@ -146,8 +146,14 @@
image::images/browser-notification-preference.png["user preference for browser notifications", align="center"]
-Current implementation works only when gerrit is open in one of the tabs. We
-poll every 5 minutes for changes in attention set.
+The notifications work only when Gerrit is open in one of the browser tabs.
+The latency to get the notification is up to 5 minutes.
+
+If you are not getting notifications:
+ - Check your user preferences - Allow browser notification setting
+ - Make sure notifications are turned on for the Gerrit site in the browser
+ - Make sure browser notifications are turned on in your operating system
+ - Your host can have browser notifications disabled for some user groups
=== Bold Changes / Mark Reviewed
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 396ba74..44e7854 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -1386,22 +1386,21 @@
throw new BadRequestException("Expected JSON object");
}
- @SuppressWarnings("unchecked")
private static Object createInstance(Type type)
throws NoSuchMethodException, InstantiationException, IllegalAccessException,
InvocationTargetException {
if (type instanceof Class) {
- Class<Object> clazz = (Class<Object>) type;
- Constructor<Object> c = clazz.getDeclaredConstructor();
+ Class<?> clazz = (Class<?>) type;
+ Constructor<?> c = clazz.getDeclaredConstructor();
c.setAccessible(true);
return c.newInstance();
}
if (type instanceof ParameterizedType) {
Type rawType = ((ParameterizedType) type).getRawType();
- if (rawType instanceof Class && List.class.isAssignableFrom((Class<Object>) rawType)) {
+ if (rawType instanceof Class && List.class.isAssignableFrom((Class<?>) rawType)) {
return new ArrayList<>();
}
- if (rawType instanceof Class && Map.class.isAssignableFrom((Class<Object>) rawType)) {
+ if (rawType instanceof Class && Map.class.isAssignableFrom((Class<?>) rawType)) {
return new HashMap<>();
}
}
diff --git a/java/com/google/gerrit/server/approval/ApprovalCopier.java b/java/com/google/gerrit/server/approval/ApprovalCopier.java
index cd2d79e..a1889da 100644
--- a/java/com/google/gerrit/server/approval/ApprovalCopier.java
+++ b/java/com/google/gerrit/server/approval/ApprovalCopier.java
@@ -46,6 +46,7 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.approval.ApprovalContext;
import com.google.gerrit.server.query.approval.ApprovalQueryBuilder;
+import com.google.gerrit.server.util.LabelVote;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
@@ -353,10 +354,25 @@
Predicate<ApprovalContext> copyConditionPredicate =
approvalQueryBuilder.parse(labelType.getCopyCondition().get());
boolean canCopy = copyConditionPredicate.asMatchable().match(ctx);
- ImmutableSet.Builder<String> passingAtoms = ImmutableSet.builder();
- ImmutableSet.Builder<String> failingAtoms = ImmutableSet.builder();
- evaluateAtoms(copyConditionPredicate, ctx, passingAtoms, failingAtoms);
- return ApprovalCopyResult.create(canCopy, passingAtoms.build(), failingAtoms.build());
+ ImmutableSet.Builder<String> passingAtomsBuilder = ImmutableSet.builder();
+ ImmutableSet.Builder<String> failingAtomsBuilder = ImmutableSet.builder();
+ evaluateAtoms(copyConditionPredicate, ctx, passingAtomsBuilder, failingAtomsBuilder);
+ ImmutableSet<String> passingAtoms = passingAtomsBuilder.build();
+ ImmutableSet<String> failingAtoms = failingAtomsBuilder.build();
+ logger.atFine().log(
+ "%s copy %s of account %d on change %d from patch set %d to patch set %d"
+ + " (copyCondition = %s, passingAtoms = %s, failingAtoms = %s, changeKind = %s)",
+ canCopy ? "Can" : "Cannot",
+ LabelVote.create(labelType.getName(), approvalValue).format(),
+ approverId.get(),
+ changeNotes.getChangeId().get(),
+ sourcePatchSetId.get(),
+ targetPatchSet.id().get(),
+ labelType.getCopyCondition().get(),
+ passingAtoms,
+ failingAtoms,
+ changeKind.name());
+ return ApprovalCopyResult.create(canCopy, passingAtoms, failingAtoms);
}
} catch (QueryParseException e) {
logger.atWarning().withCause(e).log(
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 5b8d784..eb4f6bf 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -1046,6 +1046,15 @@
public static final IndexedField<ChangeData, String>.SearchSpec COMMIT_MESSAGE_EXACT =
COMMIT_MESSAGE_EXACT_FIELD.exact(ChangeQueryBuilder.FIELD_MESSAGE_EXACT);
+ /** Commit message of the current patch set. */
+ public static final IndexedField<ChangeData, String> SUBJECT_FIELD =
+ IndexedField.<ChangeData>stringBuilder("Subject")
+ .required()
+ .build(changeGetter(Change::getSubject));
+
+ public static final IndexedField<ChangeData, String>.SearchSpec SUBJECT_SPEC =
+ SUBJECT_FIELD.fullText(ChangeQueryBuilder.FIELD_SUBJECT);
+
/** Summary or inline comment. */
public static final IndexedField<ChangeData, Iterable<String>> COMMENT_FIELD =
IndexedField.<ChangeData>iterableStringBuilder("Comment")
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 6c92f111..5677b40 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -207,6 +207,15 @@
.remove(ChangeField.STAR_SPEC, ChangeField.STARBY_SPEC, ChangeField.DRAFTBY_SPEC)
.remove(ChangeField.STAR_FIELD, ChangeField.STARBY_FIELD, ChangeField.DRAFTBY_FIELD)
.build();
+
+ /** Add subject field. */
+ static final Schema<ChangeData> V80 =
+ new Schema.Builder<ChangeData>()
+ .add(V79)
+ .addIndexedFields(ChangeField.SUBJECT_FIELD)
+ .addSearchSpecs(ChangeField.SUBJECT_SPEC)
+ .build();
+
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java
index c2672a9..4637191 100644
--- a/java/com/google/gerrit/server/query/change/ChangePredicates.java
+++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java
@@ -332,6 +332,10 @@
return new ChangeIndexPredicate(ChangeField.COMMIT_MESSAGE, message);
}
+ public static Predicate<ChangeData> subject(String subject) {
+ return new ChangeIndexPredicate(ChangeField.SUBJECT_SPEC, subject);
+ }
+
/**
* Returns a predicate that matches changes where the provided {@code comment} appears in any
* comment on any patch set of the change. Uses full-text search semantics.
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index b433b25..8262e58 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -186,6 +186,7 @@
public static final String FIELD_MERGEABLE = "mergeable2";
public static final String FIELD_MERGED_ON = "mergedon";
public static final String FIELD_MESSAGE = "message";
+ public static final String FIELD_SUBJECT = "subject";
public static final String FIELD_MESSAGE_EXACT = "messageexact";
public static final String FIELD_OWNER = "owner";
public static final String FIELD_OWNERIN = "ownerin";
@@ -1140,6 +1141,12 @@
return ChangePredicates.message(text);
}
+ @Operator
+ public Predicate<ChangeData> subject(String value) throws QueryParseException {
+ checkFieldAvailable(ChangeField.SUBJECT_SPEC, ChangeQueryBuilder.FIELD_SUBJECT);
+ return ChangePredicates.subject(value);
+ }
+
private Predicate<ChangeData> starredBySelf() throws QueryParseException {
return ChangePredicates.starBy(
args.starredChangesUtil, self(), StarredChangesUtil.DEFAULT_LABEL);
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index 947ca6a..62fdcbb 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -236,6 +236,7 @@
cherryPickInput = createCherryPickInput(revertInput);
Instant timestamp = TimeUtil.now();
+ String initialMessage = revertInput.message;
for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
cherryPickInput.base = null;
Project.NameKey project = projectAndBranch.project();
@@ -253,6 +254,7 @@
.collect(Collectors.toSet());
revertAllChangesInProjectAndBranch(
+ initialMessage,
revertInput,
project,
sortedChangesInProjectAndBranch,
@@ -265,7 +267,9 @@
return revertSubmissionInfo;
}
+ // Warning: reuses and modifies revertInput.message.
private void revertAllChangesInProjectAndBranch(
+ String initialMessage,
RevertInput revertInput,
Project.NameKey project,
Iterator<PatchSetData> sortedChangesInProjectAndBranch,
@@ -273,8 +277,6 @@
Instant timestamp)
throws IOException, RestApiException, UpdateException, ConfigInvalidException,
PermissionBackendException {
-
- String initialMessage = revertInput.message;
while (sortedChangesInProjectAndBranch.hasNext()) {
ChangeNotes changeNotes = sortedChangesInProjectAndBranch.next().data().notes();
if (cherryPickInput.base == null) {
@@ -282,6 +284,7 @@
cherryPickInput.base = getBase(changeNotes, commitIdsInProjectAndBranch).name();
}
+ // Set revert message for the current revert change.
revertInput.message = getMessage(initialMessage, changeNotes);
if (cherryPickInput.base.equals(changeNotes.getCurrentPatchSet().commitId().getName())) {
// This is the code in case this is the first revert of this project + branch, and the
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index 527253c..e0e980e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -63,6 +63,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
+import java.util.regex.Pattern;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Repository;
@@ -1278,10 +1279,38 @@
.distinct()
.count())
.isEqualTo(1);
+
+ // Size
List<ChangeApi> revertChanges = getChangeApis(revertSubmissionInfo);
+ assertThat(revertChanges).hasSize(3);
+ assertThat(gApi.changes().id(revertChanges.get(1).id()).current().related().changes).hasSize(2);
+
+ // Contents
assertThat(revertChanges.get(0).current().files().get("c.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(1).current().files().get("a.txt").linesDeleted).isEqualTo(1);
assertThat(revertChanges.get(2).current().files().get("b.txt").linesDeleted).isEqualTo(1);
+
+ // Commit message
+ assertThat(revertChanges.get(0).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"first change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+ assertThat(revertChanges.get(1).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"second change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+ assertThat(revertChanges.get(2).current().commit(false).message)
+ .matches(
+ Pattern.compile(
+ "Revert \"third change\"\n\n"
+ + "This reverts commit [a-f0-9]+\\.\n\n"
+ + "Change-Id: I[a-f0-9]+\n"));
+
+ // Relationships
String sha1FirstChange = resultCommits.get(0).getCommit().getName();
String sha1ThirdChange = resultCommits.get(2).getCommit().getName();
String sha1SecondRevert = revertChanges.get(2).current().commit(false).commit;
@@ -1291,9 +1320,6 @@
.isEqualTo(sha1ThirdChange);
assertThat(revertChanges.get(1).current().commit(false).parents.get(0).commit)
.isEqualTo(sha1SecondRevert);
-
- assertThat(revertChanges).hasSize(3);
- assertThat(gApi.changes().id(revertChanges.get(1).id()).current().related().changes).hasSize(2);
}
@Test
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index d7ec030..23f33fc 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -148,6 +148,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -635,7 +636,7 @@
Change change1 = insert(repo, newChange(repo), userId);
assertQuery("is:uploader", change1);
assertQuery("uploader:" + userId.get(), change1);
- change1 = newPatchSet(repo, change1, user2CurrentUser);
+ change1 = newPatchSet(repo, change1, user2CurrentUser, /* message= */ Optional.empty());
// Uploader has changed
assertQuery("uploader:" + userId.get());
assertQuery("uploader:" + user2.get(), change1);
@@ -770,8 +771,9 @@
Account.Id user2 =
accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
CurrentUser user2CurrentUser = userFactory.create(user2);
- newPatchSet(repo, change1, user2CurrentUser);
+ change1 = newPatchSet(repo, change1, user2CurrentUser, /* message= */ Optional.empty());
assertQuery("uploaderin:Administrators");
+ assertQuery("uploaderin:\"Registered Users\"", change1);
}
@Test
@@ -1033,6 +1035,55 @@
}
@Test
+ public void bySubject() throws Exception {
+ assume().that(getSchema().hasField(ChangeField.SUBJECT_SPEC)).isTrue();
+ TestRepository<Repo> repo = createProject("repo");
+ RevCommit commit1 =
+ repo.parseBody(
+ repo.commit()
+ .message(
+ "First commit with test subject\n\n"
+ + "Message body\n\n"
+ + "Change-Id: I986c6a013dd5b3a2e8a0271c04deac2c9752b920")
+ .create());
+ Change change1 = insert(repo, newChangeForCommit(repo, commit1));
+ RevCommit commit2 =
+ repo.parseBody(
+ repo.commit()
+ .message(
+ "Second commit with test subject\n\n"
+ + "Message body for another commit\n\n"
+ + "Change-Id: I986c6a013dd5b3a2e8a0271c04deac2c9752b921")
+ .create());
+ Change change2 = insert(repo, newChangeForCommit(repo, commit2));
+ RevCommit commit3 =
+ repo.parseBody(
+ repo.commit()
+ .message(
+ "Third commit with test subject\n\n"
+ + "Last message body\n\n"
+ + "Change-Id: I986c6a013dd5b3a2e8a0271c04deac2c9752b921")
+ .create());
+ Change change3 = insert(repo, newChangeForCommit(repo, commit3));
+
+ assertQuery("subject:First", change1);
+ assertQuery("subject:Second", change2);
+ assertQuery("subject:Third", change3);
+ assertQuery("subject:\"commit with test subject\"", change3, change2, change1);
+ assertQuery("subject:\"Message body\"");
+ assertQuery("subject:body");
+ change1 =
+ newPatchSet(
+ repo,
+ change1,
+ user,
+ Optional.of("Rework of commit with test subject\n\n" + "Message body\n\n"));
+ assertQuery("subject:Rework", change1);
+ assertQuery("subject:First");
+ assertQuery("subject:\"commit with test subject\"", change1, change3, change2);
+ }
+
+ @Test
public void fullTextWithNumbers() throws Exception {
TestRepository<Repo> repo = createProject("repo");
RevCommit commit1 = repo.parseBody(repo.commit().message("12345 67890").create());
@@ -2807,7 +2858,7 @@
gApi.changes().id(change2.getId().get()).current().review(new ReviewInput().message("comment"));
PatchSet.Id ps3_1 = change3.currentPatchSetId();
- change3 = newPatchSet(repo, change3, user);
+ change3 = newPatchSet(repo, change3, user, /* message= */ Optional.empty());
assertThat(change3.currentPatchSetId()).isNotEqualTo(ps3_1);
// Response to previous patch set still counts as reviewing.
gApi.changes()
@@ -4040,13 +4091,18 @@
}
}
- protected Change newPatchSet(TestRepository<Repo> repo, Change c, CurrentUser user)
+ protected Change newPatchSet(
+ TestRepository<Repo> repo, Change c, CurrentUser user, Optional<String> message)
throws Exception {
// Add a new file so the patch set is not a trivial rebase, to avoid default
// Code-Review label copying.
int n = c.currentPatchSetId().get() + 1;
RevCommit commit =
- repo.parseBody(repo.commit().message("message").add("file" + n, "contents " + n).create());
+ repo.parseBody(
+ repo.commit()
+ .message(message.orElse("message"))
+ .add("file" + n, "contents " + n)
+ .create());
PatchSetInserter inserter =
patchSetFactory
diff --git a/polygerrit-ui/app/api/plugin.ts b/polygerrit-ui/app/api/plugin.ts
index 733f112..3ed48e6 100644
--- a/polygerrit-ui/app/api/plugin.ts
+++ b/polygerrit-ui/app/api/plugin.ts
@@ -14,6 +14,7 @@
import {ChangeActionsPluginApi} from './change-actions';
import {RestPluginApi} from './rest';
import {HookApi, RegisterOptions} from './hook';
+import {StylePluginApi} from './styles';
export enum TargetElement {
CHANGE_ACTIONS = 'changeactions',
@@ -77,10 +78,11 @@
moduleName?: string,
options?: RegisterOptions
): HookApi<T>;
- // DEPRECATED: Just add <style> elements to `document.head`.
+ // DEPRECATED: Use styleApi() instead.
registerStyleModule(endpoint: string, moduleName: string): void;
reporting(): ReportingPluginApi;
restApi(): RestPluginApi;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
screen(screenName: string, moduleName?: string): any;
+ styleApi(): StylePluginApi;
}
diff --git a/polygerrit-ui/app/api/styles.ts b/polygerrit-ui/app/api/styles.ts
index f5b22a1..6ca8496 100644
--- a/polygerrit-ui/app/api/styles.ts
+++ b/polygerrit-ui/app/api/styles.ts
@@ -22,6 +22,7 @@
toString(): string;
}
+/** Accessible via `window.Gerrit.styles`. */
export declare interface Styles {
font: Style;
form: Style;
@@ -32,3 +33,22 @@
table: Style;
modal: Style;
}
+
+/** Accessible via `window.Gerrit.install(plugin => {plugin.styleApi()})`. */
+export declare interface StylePluginApi {
+ /**
+ * Convenience method for adding a CSS rule to a <style> element in <head>.
+ *
+ * Note that you can only insert one rule per call. See `insertRule()`
+ * documentation of `CSSStyleSheet`:
+ * https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleSheet/insertRule
+ *
+ * @param rule the css rule, e.g.:
+ * ```
+ * html.darkTheme {
+ * --header-background-color: blue;
+ * }
+ * ```
+ */
+ insertCSSRule(rule: string): void;
+}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index adc3bd7..a78ba1c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -1351,7 +1351,6 @@
showRevertDialog() {
const change = this.change;
if (!change) return;
- // The search is still broken if there is a " in the topic.
const query = `submissionid: "${change.submission_id}"`;
/* A chromium plugin expects that the modifyRevertMsg hook will only
be called after the revert button is pressed, hence we populate the
@@ -1365,7 +1364,11 @@
return;
}
assertIsDefined(this.confirmRevertDialog, 'confirmRevertDialog');
- this.confirmRevertDialog.populate(change, this.commitMessage, changes);
+ this.confirmRevertDialog.populate(
+ change,
+ this.commitMessage,
+ changes.length
+ );
this.showActionDialog(this.confirmRevertDialog);
});
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index a1021a0..bdcdc31 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -1582,13 +1582,8 @@
'Revert submission 199 0' +
'\n\n' +
'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
+ '\n\n' +
+ 'Reverted changes: /q/submissionid:199+0\n';
assert.equal(confirmRevertDialog.message, expectedMsg);
const radioInputs = queryAll<HTMLInputElement>(
confirmRevertDialog,
@@ -1648,13 +1643,8 @@
'Revert submission 199 0' +
'\n\n' +
'Reason for revert: <INSERT REASONING HERE>' +
- '\n' +
- 'Reverted Changes:' +
- '\n' +
- '1234567890:random' +
- '\n' +
- '23456:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...' +
- '\n';
+ '\n\n' +
+ 'Reverted changes: /q/submissionid:199+0\n';
const singleChangeMsg =
'Revert "random commit message"\n\nThis reverts ' +
'commit 2000.\n\nReason' +
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index fe2bb4d..91b82dc 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -2112,7 +2112,6 @@
this.viewState.basePatchNum = PARENT;
const patchChanged = this.hasPatchRangeChanged(this.viewState);
- let patchNumChanged = this.hasPatchNumChanged(this.viewState);
this.patchRange = {
patchNum: this.viewState.patchNum,
@@ -2136,14 +2135,13 @@
...this.patchRange,
patchNum: computeLatestPatchNum(this.allPatchSets),
};
- patchNumChanged = true;
}
if (patchChanged) {
// We need to collapse all diffs when viewState changes so that a non
// existing diff is not requested. See Issue 125270 for more details.
this.fileList?.resetFileState();
this.fileList?.collapseAllDiffs();
- this.reloadPatchNumDependentResources(patchNumChanged).then(() => {
+ this.reloadPatchNumDependentResources().then(() => {
this.sendShowChangeEvent();
});
}
@@ -2978,25 +2976,8 @@
* Kicks off requests for resources that rely on the patch range
* (`this.patchRange`) being defined.
*/
- reloadPatchNumDependentResources(patchNumChanged?: boolean) {
- assertIsDefined(this.changeNum, 'changeNum');
- if (!this.patchRange?.patchNum) throw new Error('missing patchNum');
- const promises = [this.loadAndSetCommitInfo()];
- if (patchNumChanged) {
- promises.push(
- this.getCommentsModel().reloadPortedComments(
- this.changeNum,
- this.patchRange?.patchNum
- )
- );
- promises.push(
- this.getCommentsModel().reloadPortedDrafts(
- this.changeNum,
- this.patchRange?.patchNum
- )
- );
- }
- return Promise.all(promises);
+ reloadPatchNumDependentResources() {
+ return this.loadAndSetCommitInfo();
}
// Private but used in tests
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 4fafcd9..9d89192 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -103,10 +103,6 @@
import {rootUrl} from '../../../utils/url-util';
import {testResolver} from '../../../test/common-test-setup';
import {UserModel, userModelToken} from '../../../models/user/user-model';
-import {
- CommentsModel,
- commentsModelToken,
-} from '../../../models/comments/comments-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
suite('gr-change-view tests', () => {
@@ -114,7 +110,6 @@
let setUrlStub: sinon.SinonStub;
let userModel: UserModel;
let changeModel: ChangeModel;
- let commentsModel: CommentsModel;
const ROBOT_COMMENTS_LIMIT = 10;
@@ -382,7 +377,6 @@
sinon.stub(element.actions, 'reload').returns(Promise.resolve());
});
userModel = testResolver(userModelToken);
- commentsModel = testResolver(commentsModelToken);
changeModel = testResolver(changeModelToken);
});
@@ -1490,7 +1484,7 @@
.callsFake(() => Promise.resolve());
const reloadPatchDependentStub = sinon
.stub(element, 'reloadPatchNumDependentResources')
- .callsFake(() => Promise.resolve([undefined, undefined, undefined]));
+ .callsFake(() => Promise.resolve());
assertIsDefined(element.fileList);
await element.fileList.updateComplete;
const collapseStub = sinon.stub(element.fileList, 'collapseAllDiffs');
@@ -1525,46 +1519,6 @@
assert.isTrue(collapseStub.calledTwice);
});
- test('reload ported comments when patchNum changes', async () => {
- assertIsDefined(element.fileList);
- sinon.stub(element, 'loadData').callsFake(() => Promise.resolve());
- sinon.stub(element, 'loadAndSetCommitInfo');
- await element.updateComplete;
- const reloadPortedCommentsStub = sinon.stub(
- commentsModel,
- 'reloadPortedComments'
- );
- const reloadPortedDraftsStub = sinon.stub(
- commentsModel,
- 'reloadPortedDrafts'
- );
- sinon.stub(element.fileList, 'collapseAllDiffs');
-
- const value: ChangeViewState = {
- ...createChangeViewState(),
- view: GerritView.CHANGE,
- patchNum: 1 as RevisionPatchSetNum,
- };
- element.viewState = value;
- await element.updateComplete;
-
- element.initialLoadComplete = true;
- element.change = {
- ...createChangeViewChange(),
- revisions: {
- rev1: createRevision(1),
- rev2: createRevision(2),
- },
- };
-
- value.basePatchNum = 1 as BasePatchSetNum;
- value.patchNum = 2 as RevisionPatchSetNum;
- element.viewState = {...value};
- await element.updateComplete;
- assert.isTrue(reloadPortedCommentsStub.calledOnce);
- assert.isTrue(reloadPortedDraftsStub.calledOnce);
- });
-
test('do not reload entire page when patchRange doesnt change', async () => {
assertIsDefined(element.fileList);
const reloadStub = sinon
@@ -2382,7 +2336,7 @@
sinon.stub(element, 'getLatestCommitMessage').returns(Promise.resolve());
sinon
.stub(element, 'reloadPatchNumDependentResources')
- .returns(Promise.resolve([undefined, undefined, undefined]));
+ .returns(Promise.resolve());
});
test("don't report changeDisplayed on reply", async () => {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
index b238d77..6b37284 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -14,9 +14,9 @@
import {BindValueChangeEvent} from '../../../types/events';
import {resolve} from '../../../models/dependency';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {createSearchUrl} from '../../../models/views/search';
const ERR_COMMIT_NOT_FOUND = 'Unable to find the commit hash of this change.';
-const CHANGE_SUBJECT_LIMIT = 50;
const INSERT_REASON_STRING = '<INSERT REASONING HERE>';
// TODO(dhruvsri): clean up repeated definitions after moving to js modules
@@ -58,8 +58,9 @@
@state()
private showRevertSubmission = false;
+ // Value supplied by populate(). Non-private for access in tests.
@state()
- private changesCount?: number;
+ changesCount?: number;
@state()
showErrorMessage = false;
@@ -189,15 +190,15 @@
);
}
- populate(change: ChangeInfo, commitMessage: string, changes: ChangeInfo[]) {
- this.changesCount = changes.length;
+ populate(change: ChangeInfo, commitMessage: string, changesCount: number) {
+ this.changesCount = changesCount;
// The option to revert a single change is always available
this.populateRevertSingleChangeMessage(
change,
commitMessage,
change.current_revision
);
- this.populateRevertSubmissionMessage(change, changes, commitMessage);
+ this.populateRevertSubmissionMessage(change, commitMessage);
}
populateRevertSingleChangeMessage(
@@ -225,12 +226,6 @@
this.originalRevertMessages[this.revertType] = this.message;
}
- private getTrimmedChangeSubject(subject: string) {
- if (!subject) return '';
- if (subject.length < CHANGE_SUBJECT_LIMIT) return subject;
- return subject.substring(0, CHANGE_SUBJECT_LIMIT) + '...';
- }
-
private modifyRevertSubmissionMsg(
change: ChangeInfo,
msg: string,
@@ -243,30 +238,22 @@
);
}
- populateRevertSubmissionMessage(
- change: ChangeInfo,
- changes: ChangeInfo[],
- commitMessage: string
- ) {
+ populateRevertSubmissionMessage(change: ChangeInfo, commitMessage: string) {
// Follow the same convention of the revert
const commitHash = change.current_revision;
if (!commitHash) {
fireAlert(this, ERR_COMMIT_NOT_FOUND);
return;
}
- if (!changes || changes.length <= 1) return;
- const revertTitle = `Revert submission ${change.submission_id}`;
- let message =
- revertTitle +
+ if (this.changesCount! <= 1) return;
+ const message =
+ `Revert submission ${change.submission_id}` +
'\n\n' +
'Reason for revert: <INSERT ' +
- 'REASONING HERE>\n';
- message += 'Reverted Changes:\n';
- changes.forEach(change => {
- message +=
- `${change.change_id.substring(0, 10)}:` +
- `${this.getTrimmedChangeSubject(change.subject)}\n`;
- });
+ 'REASONING HERE>\n\n' +
+ 'Reverted changes: ' +
+ createSearchUrl({query: `submissionid:${change.submission_id}`}) +
+ '\n';
this.message = this.modifyRevertSubmissionMsg(
change,
message,
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
index 59416da..38309f2 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
@@ -6,7 +6,7 @@
import {fixture, html, assert} from '@open-wc/testing';
import '../../../test/common-test-setup';
import {createChange} from '../../../test/test-data-generators';
-import {CommitId} from '../../../types/common';
+import {ChangeSubmissionId, CommitId} from '../../../types/common';
import {EventType} from '../../../types/events';
import './gr-confirm-revert-dialog';
import {GrConfirmRevertDialog} from './gr-confirm-revert-dialog';
@@ -111,4 +111,22 @@
'Reason for revert: <INSERT REASONING HERE>\n';
assert.equal(element.message, expected);
});
+
+ test('revert submission', () => {
+ element.changesCount = 3;
+ element.populateRevertSubmissionMessage(
+ {
+ ...createChange(),
+ submission_id: '5545' as ChangeSubmissionId,
+ current_revision: 'abcd123' as CommitId,
+ },
+ 'one line commit\n\nChange-Id: abcdefg\n'
+ );
+
+ const expected =
+ 'Revert submission 5545\n\n' +
+ 'Reason for revert: <INSERT REASONING HERE>\n\n' +
+ 'Reverted changes: /q/submissionid:5545\n';
+ assert.equal(element.message, expected);
+ });
});
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts
index a2beee2..90b05f6 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-change.ts
@@ -69,8 +69,8 @@
.notCurrent {
color: var(--warning-foreground);
}
- .indirectAncestor {
- color: var(--indirect-ancestor-text-color);
+ .indirectRelation {
+ color: var(--indirect-relation-text-color);
}
.submittableCheck {
padding-left: var(--spacing-s);
@@ -147,8 +147,8 @@
change._revision_number !== change._current_revision_number
) {
classes.push('notCurrent');
- } else if (!isChangeInfo(change) && this.isIndirectAncestor(change)) {
- classes.push('indirectAncestor');
+ } else if (!isChangeInfo(change) && this.isIndirectRelation(change)) {
+ classes.push('indirectRelation');
} else if (change.submittable) {
classes.push('submittable');
} else if (change.status === ChangeStatus.NEW) {
@@ -169,15 +169,15 @@
change._revision_number !== change._current_revision_number
) {
return 'Not current';
- } else if (!isChangeInfo(change) && this.isIndirectAncestor(change)) {
- return 'Indirect ancestor';
+ } else if (!isChangeInfo(change) && this.isIndirectRelation(change)) {
+ return 'Indirect relation';
} else if (change.submittable) {
return 'Submittable';
}
return '';
}
- private isIndirectAncestor(change: RelatedChangeAndCommitInfo) {
+ private isIndirectRelation(change: RelatedChangeAndCommitInfo) {
return (
this.connectedRevisions &&
!this.connectedRevisions.includes(change.commit.commit)
diff --git a/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts b/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts
index e01bb34..ab95711 100644
--- a/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts
+++ b/polygerrit-ui/app/elements/core/gr-notifications-prompt/gr-notifications-prompt.ts
@@ -51,7 +51,7 @@
#notificationsPrompt {
position: absolute;
right: 30px;
- top: 100px;
+ top: 50px;
z-index: 150; /* Less than gr-hovercard's, higher than rest */
display: flex;
background-color: var(--background-color-primary);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api.ts
new file mode 100644
index 0000000..13eefc8
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api.ts
@@ -0,0 +1,43 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {PluginApi} from '../../../api/plugin';
+import {StylePluginApi} from '../../../api/styles';
+import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
+
+function getOrCreatePluginStyleEl(): HTMLStyleElement {
+ const el =
+ document.head.querySelector<HTMLStyleElement>('style#plugin-style');
+ if (el) return el;
+
+ const styleEl = document.createElement('style');
+ styleEl.setAttribute('id', 'plugin-style');
+ // Append at the end so that they override the default light and dark theme
+ // styles.
+ document.head.appendChild(styleEl);
+ return styleEl;
+}
+
+export class GrPluginStyleApi implements StylePluginApi {
+ constructor(
+ private readonly reporting: ReportingService,
+ private readonly plugin: PluginApi
+ ) {
+ this.reporting.trackApi(this.plugin, 'style', 'constructor');
+ }
+
+ insertCSSRule(rule: string): void {
+ this.reporting.trackApi(this.plugin, 'style', 'insertCSSRule');
+
+ const styleEl = getOrCreatePluginStyleEl();
+ try {
+ styleEl.sheet?.insertRule(rule);
+ } catch (error) {
+ console.error(
+ `Failed to insert CSS rule for plugin ${this.plugin.getPluginName()}: ${error}`
+ );
+ }
+ }
+}
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api_test.ts
new file mode 100644
index 0000000..469d667
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-plugin-style-api_test.ts
@@ -0,0 +1,51 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-js-api-interface';
+import {query, queryAndAssert} from '../../../utils/common-util';
+import {assert} from '@open-wc/testing';
+import {StylePluginApi} from '../../../api/styles';
+
+suite('gr-plugin-style-api tests', () => {
+ let styleApi: StylePluginApi;
+
+ setup(() => {
+ window.Gerrit.install(
+ p => (styleApi = p.styleApi()),
+ '0.1',
+ 'http://test.com/plugins/testplugin/static/test.js'
+ );
+ });
+
+ teardown(() => {
+ const styleEl = query<HTMLStyleElement>(
+ document.head,
+ 'style#plugin-style'
+ );
+ styleEl?.remove();
+ });
+
+ test('insertCSSRule adds a rule', async () => {
+ styleApi.insertCSSRule('html{color:green;}');
+ const styleEl = queryAndAssert<HTMLStyleElement>(
+ document.head,
+ 'style#plugin-style'
+ );
+ const styleSheet = styleEl.sheet;
+ assert.equal(styleSheet?.cssRules.length, 1);
+ });
+
+ test('insertCSSRule re-uses the <style> element', async () => {
+ styleApi.insertCSSRule('html{color:green;}');
+ styleApi.insertCSSRule('html{margin:0px;}');
+ const styleEl = queryAndAssert<HTMLStyleElement>(
+ document.head,
+ 'style#plugin-style'
+ );
+ const styleSheet = styleEl.sheet;
+ assert.equal(styleSheet?.cssRules.length, 2);
+ });
+});
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
index 2ed3794..7170051 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.ts
@@ -35,6 +35,8 @@
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
import {RestApiService} from '../../../services/gr-rest-api/gr-rest-api';
import {PluginsModel} from '../../../models/plugins/plugins-model';
+import {GrPluginStyleApi} from './gr-plugin-style-api';
+import {StylePluginApi} from '../../../api/styles';
/**
* Plugin-provided custom components can affect content in extension
@@ -244,6 +246,10 @@
return new GrReportingJsApi(this.report, this);
}
+ styleApi(): StylePluginApi {
+ return new GrPluginStyleApi(this.report, this);
+ }
+
admin(): AdminPluginApi {
return new GrAdminApi(this.report, this);
}
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
index 2de5b5f..8b3e2a6 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
@@ -533,7 +533,7 @@
} catch (err) {
fireNetworkError(err as Error);
if (req.errFn) {
- req.errFn.call(undefined, null, err as Error);
+ await req.errFn.call(undefined, null, err as Error);
xhr = undefined;
} else {
throw err;
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
index dd08544..9f52517 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts
@@ -357,12 +357,8 @@
}
private handleTabKey(e: KeyboardEvent) {
- // Tab should have normal behavior if the picker is closed or if the user
- // has only typed ':'.
- if (
- !this.isDropdownVisible() ||
- (this.isEmojiDropdownActive() && this.currentSearchString === '')
- ) {
+ // Tab should have normal behavior if the picker is closed.
+ if (!this.isDropdownVisible()) {
return;
}
e.preventDefault();
@@ -372,12 +368,9 @@
// private but used in test
handleEnterByKey(e: KeyboardEvent) {
- // Enter should have newline behavior if the picker is closed or if the user
- // has only typed ':'. Also make sure that shortcuts aren't clobbered.
- if (
- !this.isDropdownVisible() ||
- (this.isEmojiDropdownActive() && this.currentSearchString === '')
- ) {
+ // Enter should have newline behavior if the picker is closed. Also make
+ // sure that shortcuts aren't clobbered.
+ if (!this.isDropdownVisible()) {
this.indent(e);
return;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
index 634c387..1460246 100644
--- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts
@@ -617,19 +617,6 @@
await element.updateComplete;
assert.equal(element.text, '💯');
});
-
- test('enter key - ignored on just colon without more information', async () => {
- const enterSpy = sinon.spy(element.emojiSuggestions!, 'getCursorTarget');
- pressKey(element.textarea! as HTMLElement, Key.ENTER);
- assert.isFalse(enterSpy.called);
- element.textarea!.focus();
- element.textarea!.selectionStart = 1;
- element.textarea!.selectionEnd = 1;
- element.text = ':';
- await element.updateComplete;
- pressKey(element.textarea! as HTMLElement, Key.ENTER);
- assert.isFalse(enterSpy.called);
- });
});
suite('gr-textarea monospace', () => {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
index e5fce12..14bb17e 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -30,6 +30,29 @@
number: number;
}
+/**
+ * From <tr> diff row go up to <tbody> diff chunk.
+ *
+ * In Lit based diff there is a <gr-diff-row> element in between the two.
+ */
+export function fromRowToChunk(
+ rowEl: HTMLElement
+): HTMLTableSectionElement | undefined {
+ const parent = rowEl.parentElement;
+ if (!parent) return undefined;
+ if (parent.tagName === 'TBODY') {
+ return parent as HTMLTableSectionElement;
+ }
+
+ const grandParent = parent.parentElement;
+ if (!grandParent) return undefined;
+ if (grandParent.tagName === 'TBODY') {
+ return grandParent as HTMLTableSectionElement;
+ }
+
+ return undefined;
+}
+
/** A subset of the GrDiff API that the cursor is using. */
export interface GrDiffCursorable extends HTMLElement {
isRangeSelected(): boolean;
@@ -179,8 +202,7 @@
moveToNextChunk(clipToTop?: boolean): CursorMoveResult {
const result = this.cursorManager.next({
filter: (row: HTMLElement) => this._isFirstRowOfChunk(row),
- getTargetHeight: target =>
- (target?.parentNode as HTMLElement)?.scrollHeight || 0,
+ getTargetHeight: target => fromRowToChunk(target)?.scrollHeight || 0,
clipToTop,
});
this._fixSide();
@@ -413,13 +435,14 @@
}
_isFirstRowOfChunk(row: HTMLElement) {
- const parentClassList = (row.parentNode as HTMLElement).classList;
- const isInChunk =
- parentClassList.contains('section') && parentClassList.contains('delta');
- const previousRow = row.previousSibling as HTMLElement;
- const firstContentRow =
- !previousRow || previousRow.classList.contains('moveControls');
- return isInChunk && firstContentRow;
+ const chunk = fromRowToChunk(row);
+ if (!chunk) return false;
+
+ const isInDeltaChunk = chunk.classList.contains('delta');
+ if (!isInDeltaChunk) return false;
+
+ const firstRow = chunk.querySelector('tr:not(.moveControls)');
+ return firstRow === row;
}
_rowHasThread(row: HTMLElement): boolean {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
index 1e554b7..b9db280 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-cursor/gr-diff-cursor_test.ts
@@ -7,7 +7,12 @@
import '../gr-diff/gr-diff';
import './gr-diff-cursor';
import {fixture, html, assert} from '@open-wc/testing';
-import {mockPromise, queryAll, queryAndAssert} from '../../../test/test-utils';
+import {
+ mockPromise,
+ queryAll,
+ queryAndAssert,
+ waitUntil,
+} from '../../../test/test-utils';
import {createDiff} from '../../../test/test-data-generators';
import {createDefaultDiffPrefs} from '../../../constants/constants';
import {GrDiffCursor} from './gr-diff-cursor';
@@ -41,37 +46,29 @@
diff = createDiff();
diffElement.prefs = createDefaultDiffPrefs();
+ diffElement.renderPrefs = {use_lit_components: true};
diffElement.diff = diff;
await promise;
});
test('diff cursor functionality (side-by-side)', () => {
- // The cursor has been initialized to the first delta.
assert.isOk(cursor.diffRow);
- const firstDeltaRow = queryAndAssert<HTMLElement>(
+ const deltaRows = queryAll<HTMLTableRowElement>(
diffElement,
- '.section.delta .diff-row'
+ '.section.delta tr.diff-row'
);
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.equal(cursor.diffRow, deltaRows[0]);
cursor.moveDown();
- assert.isOk(firstDeltaRow.nextElementSibling);
- assert.notEqual(cursor.diffRow, firstDeltaRow);
- assert.equal(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
+ assert.notEqual(cursor.diffRow, deltaRows[0]);
+ assert.equal(cursor.diffRow, deltaRows[1]);
cursor.moveUp();
- assert.isOk(firstDeltaRow.nextElementSibling);
- assert.notEqual(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, deltaRows[1]);
+ assert.equal(cursor.diffRow, deltaRows[0]);
});
test('moveToFirstChunk', async () => {
@@ -115,20 +112,26 @@
] as HTMLElement[];
assert.equal(chunks.length, 2);
+ const rows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(rows.length, 2);
+
// Verify it works on fresh diff.
cursor.moveToFirstChunk();
assert.ok(cursor.diffRow);
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 0);
+ assert.equal(cursor.diffRow, rows[0]);
assert.equal(cursor.side, Side.RIGHT);
// Verify it works from other cursor positions.
cursor.moveToNextChunk();
assert.ok(cursor.diffRow);
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 1);
+ assert.equal(cursor.diffRow, rows[1]);
assert.equal(cursor.side, Side.LEFT);
+
cursor.moveToFirstChunk();
assert.ok(cursor.diffRow);
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 0);
+ assert.equal(cursor.diffRow, rows[0]);
assert.equal(cursor.side, Side.RIGHT);
});
@@ -164,20 +167,31 @@
await waitForEventOnce(diffElement, 'render');
cursor._updateStops();
- const chunks = [...queryAll(diffElement, '.section.delta')];
+ const chunks = [
+ ...queryAll(diffElement, '.section.delta'),
+ ] as HTMLElement[];
assert.equal(chunks.length, 2);
+ const rows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(rows.length, 2);
+
// Verify it works on fresh diff.
cursor.moveToLastChunk();
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 1);
+ assert.ok(cursor.diffRow);
+ assert.equal(cursor.diffRow, rows[1]);
assert.equal(cursor.side, Side.RIGHT);
// Verify it works from other cursor positions.
cursor.moveToPreviousChunk();
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 0);
+ assert.ok(cursor.diffRow);
+ assert.equal(cursor.diffRow, rows[0]);
assert.equal(cursor.side, Side.LEFT);
+
cursor.moveToLastChunk();
- assert.equal(chunks.indexOf(cursor.diffRow!.parentElement!), 1);
+ assert.ok(cursor.diffRow);
+ assert.equal(cursor.diffRow, rows[1]);
assert.equal(cursor.side, Side.RIGHT);
});
@@ -221,30 +235,22 @@
});
test('diff cursor functionality (unified)', () => {
- // The cursor has been initialized to the first delta.
assert.isOk(cursor.diffRow);
- const firstDeltaRow = queryAndAssert<HTMLElement>(
- diffElement,
- '.section.delta .diff-row'
- );
- assert.equal(cursor.diffRow, firstDeltaRow);
+ const rows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(cursor.diffRow, rows[0]);
cursor.moveDown();
- assert.notEqual(cursor.diffRow, firstDeltaRow);
- assert.equal(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
+ assert.notEqual(cursor.diffRow, rows[0]);
+ assert.equal(cursor.diffRow, rows[1]);
cursor.moveUp();
- assert.notEqual(
- cursor.diffRow,
- firstDeltaRow.nextElementSibling as HTMLElement
- );
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, rows[1]);
+ assert.equal(cursor.diffRow, rows[0]);
});
});
@@ -253,19 +259,21 @@
// mode.
assert.equal(diffElement.viewMode, 'SIDE_BY_SIDE');
- const firstDeltaSection = queryAndAssert<HTMLElement>(
- diffElement,
- '.section.delta'
- );
- const firstDeltaRow = queryAndAssert<HTMLElement>(
- firstDeltaSection,
- '.diff-row'
- );
+ const rows = [
+ ...queryAll(diffElement, '.section tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(rows.length, 50);
+ const deltaRows = [
+ ...queryAll(diffElement, '.section.delta tr.diff-row'),
+ ] as HTMLTableRowElement[];
+ assert.equal(deltaRows.length, 14);
+ const indexFirstDelta = rows.indexOf(deltaRows[0]);
+ const rowBeforeFirstDelta = rows[indexFirstDelta - 1];
// Because the first delta in this diff is on the right, it should be set
// to the right side.
assert.equal(cursor.side, Side.RIGHT);
- assert.equal(cursor.diffRow, firstDeltaRow);
+ assert.equal(cursor.diffRow, deltaRows[0]);
const firstIndex = cursor.cursorManager.index;
// Move the side to the left. Because this delta only has a right side, we
@@ -274,33 +282,26 @@
cursor.moveLeft();
assert.equal(cursor.side, Side.LEFT);
- assert.notEqual(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, rows[0]);
+ assert.equal(cursor.diffRow, rowBeforeFirstDelta);
assert.equal(cursor.cursorManager.index, firstIndex - 1);
- assert.equal(
- cursor.diffRow!.parentElement,
- firstDeltaSection.previousSibling
- );
// If we move down, we should skip everything in the first delta because
// we are on the left side and the first delta has no content on the left.
cursor.moveDown();
assert.equal(cursor.side, Side.LEFT);
- assert.notEqual(cursor.diffRow, firstDeltaRow);
+ assert.notEqual(cursor.diffRow, rowBeforeFirstDelta);
+ assert.notEqual(cursor.diffRow, rows[0]);
assert.isTrue(cursor.cursorManager.index > firstIndex);
- assert.equal(cursor.diffRow!.parentElement, firstDeltaSection.nextSibling);
});
test('chunk skip functionality', () => {
- const chunks = [...queryAll(diffElement, '.section.delta')];
- const indexOfChunk = function (chunk: HTMLElement) {
- return Array.prototype.indexOf.call(chunks, chunk);
- };
+ const deltaChunks = [...queryAll(diffElement, 'tbody.section.delta')];
// We should be initialized to the first chunk. Since this chunk only has
// content on the right side, our side should be right.
- let currentIndex = indexOfChunk(cursor.diffRow!.parentElement!);
- assert.equal(currentIndex, 0);
+ assert.equal(cursor.diffRow, deltaChunks[0].querySelector('tr'));
assert.equal(cursor.side, Side.RIGHT);
// Move to the next chunk.
@@ -308,9 +309,7 @@
// Since this chunk only has content on the left side. we should have been
// automatically moved over.
- const previousIndex = currentIndex;
- currentIndex = indexOfChunk(cursor.diffRow!.parentElement!);
- assert.equal(currentIndex, previousIndex + 1);
+ assert.equal(cursor.diffRow, deltaChunks[1].querySelector('tr'));
assert.equal(cursor.side, Side.LEFT);
});
@@ -358,10 +357,10 @@
test('renders moveControls with simple descriptions', () => {
const [movedIn, movedOut] = [
- ...queryAll(diffElement, '.dueToMove .moveControls'),
+ ...queryAll<HTMLElement>(diffElement, '.dueToMove tr.moveControls'),
];
- assert.equal(movedIn.textContent, 'Moved in');
- assert.equal(movedOut.textContent, 'Moved out');
+ assert.include(movedIn.innerText, 'Moved in');
+ assert.include(movedOut.innerText, 'Moved out');
});
});
@@ -409,10 +408,10 @@
test('renders moveControls with simple descriptions', () => {
const [movedIn, movedOut] = [
- ...queryAll(diffElement, '.dueToMove .moveControls'),
+ ...queryAll<HTMLElement>(diffElement, '.dueToMove tr.moveControls'),
];
- assert.equal(movedIn.textContent, 'Moved from lines 4 - 6');
- assert.equal(movedOut.textContent, 'Moved to lines 2 - 4');
+ assert.include(movedIn.innerText, 'Moved from lines 4 - 6');
+ assert.include(movedOut.innerText, 'Moved to lines 2 - 4');
});
test('startLineAnchor of movedIn chunk fires events', async () => {
@@ -609,6 +608,7 @@
const showContext = queryAndAssert<HTMLElement>(controls, '.showContext');
showContext.click();
await waitForEventOnce(diffElement, 'render');
+ await waitUntil(() => spy.called);
assert.isTrue(spy.called);
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts
index db53eae..a790736 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection.ts
@@ -4,10 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../../../styles/shared-styles';
-import {
- normalize,
- NormalizedRange,
-} from '../gr-diff-highlight/gr-range-normalizer';
+import {normalize} from '../gr-diff-highlight/gr-range-normalizer';
import {
descendedFromClass,
parentWithClass,
@@ -21,7 +18,6 @@
getSideByLineEl,
isThreadEl,
} from '../gr-diff/gr-diff-utils';
-import {assertIsDefined} from '../../../utils/common-util';
/**
* Possible CSS classes indicating the state of selection. Dynamically added/
@@ -121,19 +117,17 @@
}
handleCopy = (e: ClipboardEvent) => {
- let commentSelected = false;
const target = e.composedPath()[0];
if (!(target instanceof Element)) return;
if (target instanceof HTMLTextAreaElement) return;
if (!descendedFromClass(target, 'diff-row', this.diffTable)) return;
if (!this.diffTable) return;
- if (this.diffTable.classList.contains(SelectionClass.COMMENT)) {
- commentSelected = true;
- }
+ if (this.diffTable.classList.contains(SelectionClass.COMMENT)) return;
+
const lineEl = getLineElByChild(target);
if (!lineEl) return;
const side = getSideByLineEl(lineEl);
- const text = this.getSelectedText(side, commentSelected);
+ const text = this.getSelectedText(side);
if (text && e.clipboardData) {
e.clipboardData.setData('Text', text);
e.preventDefault();
@@ -166,14 +160,11 @@
* @param commentSelected Whether or not a comment is selected.
* @return The selected text.
*/
- getSelectedText(side: Side, commentSelected: boolean) {
+ getSelectedText(side: Side) {
const sel = this.getSelection();
if (!sel || sel.rangeCount !== 1) {
return ''; // No multi-select support yet.
}
- if (commentSelected) {
- return this.getCommentLines(sel, side);
- }
const range = normalize(sel.getRangeAt(0));
const startLineEl = getLineElByChild(range.startContainer);
if (!startLineEl) return;
@@ -253,82 +244,4 @@
this.linesCache[side] = lines;
return lines;
}
-
- /**
- * Query the diffElement for comments and check whether they lie inside the
- * selection range.
- *
- * @param sel The selection of the window.
- * @param side The side that is currently selected.
- * @return The selected comment text.
- */
- getCommentLines(sel: Selection, side: Side) {
- const range = normalize(sel.getRangeAt(0));
- const content = [];
- assertIsDefined(this.diffTable, 'diffTable');
- const messages = this.diffTable.querySelectorAll(
- `.side-by-side [data-side="${side}"] .message *, .unified .message *`
- );
-
- for (let i = 0; i < messages.length; i++) {
- const el = messages[i];
- // Check if the comment element exists inside the selection.
- if (sel.containsNode(el, true)) {
- // Padded elements require newlines for accurate spacing.
- if (
- el.parentElement!.id === 'container' ||
- el.parentElement!.nodeName === 'BLOCKQUOTE'
- ) {
- if (content.length && content[content.length - 1] !== '') {
- content.push('');
- }
- }
-
- if (
- el.id === 'output' &&
- !descendedFromClass(el, 'collapsed', this.diffTable)
- ) {
- content.push(this.getTextContentForRange(el, sel, range));
- }
- }
- }
-
- return content.join('\n');
- }
-
- /**
- * Given a DOM node, a selection, and a selection range, recursively get all
- * of the text content within that selection.
- * Using a domNode that isn't in the selection returns an empty string.
- *
- * @param domNode The root DOM node.
- * @param sel The selection.
- * @param range The normalized selection range.
- * @return The text within the selection.
- */
- getTextContentForRange(
- domNode: Node,
- sel: Selection,
- range: NormalizedRange
- ) {
- if (!sel.containsNode(domNode, true)) {
- return '';
- }
-
- let text = '';
- if (domNode instanceof Text) {
- text = domNode.textContent || '';
- if (domNode === range.endContainer) {
- text = text.substring(0, range.endOffset);
- }
- if (domNode === range.startContainer) {
- text = text.substring(range.startOffset);
- }
- } else {
- for (const childNode of domNode.childNodes) {
- text += this.getTextContentForRange(childNode, sel, range);
- }
- }
- return text;
- }
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
index 8acaf04..1da6e0c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-selection/gr-diff-selection_test.ts
@@ -8,7 +8,6 @@
import {GrDiffSelection} from './gr-diff-selection';
import {createDiff} from '../../../test/test-data-generators';
import {DiffInfo, Side} from '../../../api/diff';
-import {GrFormattedText} from '../../../elements/shared/gr-formatted-text/gr-formatted-text';
import {fixture, html, assert} from '@open-wc/testing';
import {mouseDown} from '../../../test/test-utils';
@@ -43,15 +42,7 @@
<td class="lineNum right" data-value="2">2</td>
<td class="content">
<div class="contentText" data-side="right">more more more</div>
- <div data-side="right">
- <div class="comment-thread">
- <div class="gr-formatted-text message">
- <span id="output" class="gr-formatted-text"
- >This is a comment on the right</span
- >
- </div>
- </div>
- </div>
+ <div data-side="right"></div>
</td>
</tr>
<tr class="diff-row">
@@ -59,15 +50,7 @@
<td class="lineNum left" data-value="3">3</td>
<td class="content">
<div class="contentText" data-side="left">ga ga</div>
- <div data-side="left">
- <div class="comment-thread">
- <div class="gr-formatted-text message">
- <span id="output" class="gr-formatted-text"
- >This is <a>a</a> different comment 💩 unicode is fun</span
- >
- </div>
- </div>
- </div>
+ <div data-side="left"></div>
</td>
<td class="lineNum right" data-value="3">3</td>
</tr>
@@ -76,11 +59,7 @@
<td class="lineNum left" data-value="4">4</td>
<td class="content">
<div class="contentText" data-side="left">ga ga</div>
- <div data-side="left">
- <div class="comment-thread">
- <textarea data-side="right">test for textarea copying</textarea>
- </div>
- </div>
+ <div data-side="left"></div>
</td>
<td class="lineNum right" data-value="4">4</td>
</tr>
@@ -190,7 +169,7 @@
test('asks for text for left side Elements', () => {
const getSelectedTextStub = sinon.stub(element, 'getSelectedText');
emulateCopyOn(diffTable.querySelector('div.contentText'));
- assert.deepEqual([Side.LEFT, false], getSelectedTextStub.lastCall.args);
+ assert.deepEqual([Side.LEFT], getSelectedTextStub.lastCall.args);
});
test('reacts to copy for content Elements', () => {
@@ -257,45 +236,7 @@
2
);
selection.addRange(range);
- assert.equal(element.getSelectedText(Side.LEFT, false), 'ba\nzin\nga');
- });
-
- test('copies comments', () => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.add('selected-comment');
- element.diffTable!.classList.remove('selected-right');
- const selection = document.getSelection();
- if (selection === null) assert.fail('no selection');
- selection.removeAllRanges();
- const range = document.createRange();
- range.setStart(
- diffTable.querySelector('.gr-formatted-text *')!.firstChild!,
- 3
- );
- range.setEnd(
- diffTable.querySelectorAll('.gr-formatted-text *')[2].childNodes[2],
- 7
- );
- selection.addRange(range);
- assert.equal(
- 's is a comment\nThis is a differ',
- element.getSelectedText(Side.LEFT, true)
- );
- });
-
- test('respects astral chars in comments', () => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.add('selected-comment');
- element.diffTable!.classList.remove('selected-right');
- const selection = document.getSelection();
- if (selection === null) assert.fail('no selection');
- selection.removeAllRanges();
- const range = document.createRange();
- const nodes = diffTable.querySelectorAll('.gr-formatted-text *');
- range.setStart(nodes[2].childNodes[2], 13);
- range.setEnd(nodes[2].childNodes[2], 23);
- selection.addRange(range);
- assert.equal('mment 💩 u', element.getSelectedText(Side.LEFT, true));
+ assert.equal(element.getSelectedText(Side.LEFT), 'ba\nzin\nga');
});
test('defers to default behavior for textarea', () => {
@@ -323,7 +264,7 @@
10
);
selection.addRange(range);
- assert.equal(element.getSelectedText(Side.RIGHT, false), ' other');
+ assert.equal(element.getSelectedText(Side.RIGHT), ' other');
});
test('copies to end of side (issue 7895)', () => {
@@ -339,54 +280,6 @@
2
);
selection.addRange(range);
- assert.equal(element.getSelectedText(Side.LEFT, false), 'ba\nzin\nga');
- });
-
- suite('getTextContentForRange', () => {
- let selection: Selection;
- let range: Range;
- let nodes: NodeListOf<GrFormattedText>;
-
- setup(() => {
- element.diffTable!.classList.add('selected-left');
- element.diffTable!.classList.add('selected-comment');
- element.diffTable!.classList.remove('selected-right');
- const s = document.getSelection();
- if (s === null) assert.fail('no selection');
- selection = s;
- selection.removeAllRanges();
- range = document.createRange();
- nodes = diffTable.querySelectorAll('.gr-formatted-text *');
- });
-
- test('multi level element contained in range', () => {
- range.setStart(nodes[2].childNodes[0], 1);
- range.setEnd(nodes[2].childNodes[2], 7);
- selection.addRange(range);
- assert.equal(
- element.getTextContentForRange(diffTable, selection, range),
- 'his is a differ'
- );
- });
-
- test('multi level element as startContainer of range', () => {
- range.setStart(nodes[2].childNodes[1], 0);
- range.setEnd(nodes[2].childNodes[2], 7);
- selection.addRange(range);
- assert.equal(
- element.getTextContentForRange(diffTable, selection, range),
- 'a differ'
- );
- });
-
- test('startContainer === endContainer', () => {
- range.setStart(nodes[0].firstChild!, 2);
- range.setEnd(nodes[0].firstChild!, 12);
- selection.addRange(range);
- assert.equal(
- element.getTextContentForRange(diffTable, selection, range),
- 'is is a co'
- );
- });
+ assert.equal(element.getSelectedText(Side.LEFT), 'ba\nzin\nga');
});
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
index 7a724b9..5738d65 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-group.ts
@@ -9,6 +9,7 @@
import {assertIsDefined, assert} from '../../../utils/common-util';
import {untilRendered} from '../../../utils/dom-util';
import {isDefined} from '../../../types/types';
+import {LitElement} from 'lit';
export enum GrDiffGroupType {
/** Unchanged context. */
@@ -487,12 +488,13 @@
// This is a temporary hack while migration to lit based diff rendering:
// Elements with 'display: contents;' do not have a height, so they
// won't work as intended with `untilRendered()`.
- const watchEl =
- this.element.tagName === 'GR-DIFF-SECTION'
- ? this.element.firstElementChild
- : this.element;
- assertIsDefined(watchEl);
- await untilRendered(watchEl as HTMLElement);
+ const isLitDiff = this.element.tagName === 'GR-DIFF-SECTION';
+ if (isLitDiff) {
+ await (this.element as LitElement).updateComplete;
+ await untilRendered(this.element.firstElementChild as HTMLElement);
+ } else {
+ await untilRendered(this.element);
+ }
}
/**
diff --git a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
index c8f4fa6..b383fd7 100644
--- a/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
+++ b/polygerrit-ui/app/mixins/hovercard-mixin/hovercard-mixin.ts
@@ -219,6 +219,7 @@
);
}
this.addEventListener('request-dependency', this.resolveDep);
+ this.addEventListener('reload', this.reload);
}
private removeTargetEventListeners() {
@@ -231,6 +232,7 @@
}
this.targetCleanups = [];
this.removeEventListener('request-dependency', this.resolveDep);
+ this.removeEventListener('reload', this.reload);
}
/**
@@ -246,6 +248,10 @@
}
}
+ readonly reload = () => {
+ this.dispatchEventThroughTarget('reload');
+ };
+
readonly mouseDebounceHide = (e: MouseEvent) => {
this.debounceHide({mouseEvent: e});
};
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 60e1a91..86c6ba2 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -226,7 +226,7 @@
--tooltip-button-text-color: var(--gerrit-blue-dark);
--negative-red-text-color: var(--red-600);
--positive-green-text-color: var(--green-700);
- --indirect-ancestor-text-color: var(--green-700);
+ --indirect-relation-text-color: var(--green-700);
/* background colors */
/* primary background colors */
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index 2836247..ecae845 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -113,7 +113,7 @@
--tooltip-button-text-color: var(--gerrit-blue-light);
--negative-red-text-color: var(--red-200);
--positive-green-text-color: var(--green-200);
- --indirect-ancestor-text-color: var(--green-200);
+ --indirect-relation-text-color: var(--green-200);
/* background colors */
/* primary background colors */
@@ -294,7 +294,13 @@
const styleEl = document.createElement('style');
styleEl.setAttribute('id', 'dark-theme');
safeStyleEl.setTextContent(styleEl, darkThemeCss);
- document.head.appendChild(styleEl);
+
+ // We would like to insert the dark theme styles after the light theme such
+ // that the dark theme values override the defaults in the light theme. But
+ // OTOH we want to insert before any plugin provided styles, because we do NOT
+ // want to override those.
+ const pluginStyleEl = document.head.querySelector('style#plugin-style');
+ document.head.insertBefore(styleEl, pluginStyleEl);
}
export function removeTheme() {
