Merge "Fix gr-comment-thread_test errors"
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 1220300..7b2bf12 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.mail.send;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.util.AttentionSetUtil.additionsOnly;
import com.google.common.base.Splitter;
@@ -59,6 +58,7 @@
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
+import java.util.stream.Collectors;
import org.apache.james.mime4j.dom.field.FieldName;
import org.eclipse.jgit.diff.DiffFormatter;
import org.eclipse.jgit.internal.JGitText;
@@ -487,8 +487,8 @@
for (String reviewer : getEmailsByState(ReviewerStateInternal.CC)) {
footers.add(MailHeader.CC.withDelimiter() + reviewer);
}
- for (String attentionSet : getAttentionSet()) {
- footers.add(MailHeader.ATTENTION.withDelimiter() + attentionSet);
+ for (String attentionUser : getAttentionSet()) {
+ footers.add(MailHeader.ATTENTION.withDelimiter() + attentionUser);
}
}
@@ -521,7 +521,7 @@
attentionSet =
additionsOnly(changeData.attentionSet()).stream()
.map(a -> getNameEmailFor(a.account()))
- .collect(toImmutableSet());
+ .collect(Collectors.toSet());
} catch (StorageException e) {
logger.atWarning().withCause(e).log("Cannot get change attention set");
}
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index a1703a2..4e56daf 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -829,13 +829,18 @@
shouldSuppressKeyboardShortcut(event: CustomKeyboardEvent) {
const e = getKeyboardEvent(event);
// TODO(TS): maybe override the EventApi, narrow it down to Element always
- const tagName = ((dom(e) as EventApi).rootTarget as Element).tagName;
+ const target = (dom(e) as EventApi).rootTarget as Element;
+ const tagName = target.tagName;
+ const type = target.getAttribute('type');
if (
- tagName === 'INPUT' ||
+ // Suppress shortcuts on <input> and <textarea>, but not on
+ // checkboxes, because we want to enable workflows like 'click
+ // mark-reviewed and then press ] to go to the next file'.
+ (tagName === 'INPUT' && type !== 'checkbox') ||
tagName === 'TEXTAREA' ||
+ // Suppress shortcuts if the key is 'enter' and target is an anchor.
(e.keyCode === 13 && tagName === 'A')
) {
- // Suppress shortcuts if the key is 'enter' and target is an anchor.
return true;
}
for (let i = 0; e.path && i < e.path.length; i++) {
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
index defc7dc..180dbe7 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
@@ -311,6 +311,17 @@
MockInteractions.keyDownOn(inputEl, 75, null, 'k');
});
+ test('doesn’t block kb shortcuts for checkboxes', done => {
+ const inputEl = document.createElement('input');
+ inputEl.setAttribute('type', 'checkbox');
+ element.appendChild(inputEl);
+ element._handleKey = e => {
+ assert.isFalse(element.shouldSuppressKeyboardShortcut(e));
+ done();
+ };
+ MockInteractions.keyDownOn(inputEl, 75, null, 'k');
+ });
+
test('blocks kb shortcuts for textarea els', done => {
const textareaEl = document.createElement('textarea');
element.appendChild(textareaEl);
