Keyboard shortcuts to expand and collapse all comments
This change adds keyboard shortcuts to the "gr-diff-comment-thread"
expand all comments when 'e' is pressed and collapse all comments when
'shift + e' is pressed. Note that the keyboard event is detected on the
thread instead of the comment to minimize the number of events getting
triggered.
Feature: Issue 4738
Change-Id: Iab77349bd1527d7af5e05a827919a78a86909835
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index 305c36a..963084a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -29,6 +29,10 @@
type: Array,
value: function() { return []; },
},
+ keyEventTarget: {
+ type: Object,
+ value: function() { return document.body; },
+ },
patchNum: String,
path: String,
projectConfig: Object,
@@ -41,6 +45,10 @@
_orderedComments: Array,
},
+ behaviors: [
+ Gerrit.KeyboardShortcutBehavior,
+ ],
+
listeners: {
'comment-update': '_handleCommentUpdate',
},
@@ -80,6 +88,22 @@
this._orderedComments = this._sortedComments(this.comments);
},
+ _handleKey: function(e) {
+ if (this.shouldSupressKeyboardShortcut(e)) { return; }
+ if (e.keyCode === 69) { // 'e'
+ e.preventDefault();
+ this._expandCollapseComments(e.shiftKey);
+ }
+ },
+
+ _expandCollapseComments: function(actionIsCollapse) {
+ var comments =
+ Polymer.dom(this.root).querySelectorAll('gr-diff-comment');
+ comments.forEach(function(comment) {
+ comment.collapsed = actionIsCollapse;
+ });
+ },
+
_sortedComments: function(comments) {
comments.sort(function(c1, c2) {
var c1Date = c1.__date || util.parseDate(c1.updated);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
index 641dc0f..eb87f56 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -54,30 +54,25 @@
message: 'i like you, too',
in_reply_to: 'sallys_confession',
updated: '2015-12-25 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_confession',
message: 'i like you, jack',
updated: '2015-12-24 15:00:20.396000000',
- },
- {
+ }, {
id: 'sally_to_dr_finklestein',
message: 'i’m running away',
updated: '2015-10-31 09:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_defiance',
in_reply_to: 'sally_to_dr_finklestein',
message: 'i will poison you so i can get away',
updated: '2015-10-31 15:00:20.396000000',
- },
- {
+ }, {
id: 'dr_finklesteins_response',
in_reply_to: 'sally_to_dr_finklestein',
message: 'no i will pull a thread and your arm will fall off',
updated: '2015-10-31 11:00:20.396000000'
- },
- {
+ }, {
id: 'sallys_mission',
message: 'i have to find santa',
updated: '2015-12-24 21:00:20.396000000'
@@ -89,31 +84,26 @@
id: 'sally_to_dr_finklestein',
message: 'i’m running away',
updated: '2015-10-31 09:00:20.396000000',
- },
- {
+ }, {
id: 'dr_finklesteins_response',
in_reply_to: 'sally_to_dr_finklestein',
message: 'no i will pull a thread and your arm will fall off',
updated: '2015-10-31 11:00:20.396000000'
- },
- {
+ }, {
id: 'sallys_defiance',
in_reply_to: 'sally_to_dr_finklestein',
message: 'i will poison you so i can get away',
updated: '2015-10-31 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_confession',
message: 'i like you, jack',
updated: '2015-12-24 15:00:20.396000000',
- },
- {
+ }, {
id: 'jacks_reply',
message: 'i like you, too',
in_reply_to: 'sallys_confession',
updated: '2015-12-25 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_mission',
message: 'i have to find santa',
updated: '2015-12-24 21:00:20.396000000'
@@ -247,20 +237,17 @@
message: 'i like you, too',
in_reply_to: 'sallys_confession',
updated: '2015-12-25 15:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_confession',
in_reply_to: 'nonexistent_comment',
message: 'i like you, jack',
updated: '2015-12-24 15:00:20.396000000',
- },
- {
+ }, {
id: 'sally_to_dr_finklestein',
in_reply_to: 'nonexistent_comment',
message: 'i’m running away',
updated: '2015-10-31 09:00:20.396000000',
- },
- {
+ }, {
id: 'sallys_defiance',
message: 'i will poison you so i can get away',
updated: '2015-10-31 15:00:20.396000000',
@@ -268,5 +255,37 @@
element.comments = comments;
assert.equal(4, element._orderedComments.length);
});
+
+ test('keyboard shortcuts', function() {
+ var comments = [
+ {
+ id: 'jacks_reply',
+ message: 'i like you, too',
+ in_reply_to: 'sallys_confession',
+ updated: '2015-12-25 15:00:20.396000000',
+ }, {
+ id: 'sallys_confession',
+ in_reply_to: 'nonexistent_comment',
+ message: 'i like you, jack',
+ updated: '2015-12-24 15:00:20.396000000',
+ }, {
+ id: 'sally_to_dr_finklestein',
+ in_reply_to: 'nonexistent_comment',
+ message: 'i’m running away',
+ updated: '2015-10-31 09:00:20.396000000',
+ }, {
+ id: 'sallys_defiance',
+ message: 'i will poison you so i can get away',
+ updated: '2015-10-31 15:00:20.396000000',
+ }];
+ element.comments = comments;
+ var expandCollapseStub = sinon.stub(element, '_expandCollapseComments');
+ MockInteractions.pressAndReleaseKeyOn(element, 69); // 'e'
+ assert.isTrue(expandCollapseStub.lastCall.calledWith(false));
+
+ MockInteractions.pressAndReleaseKeyOn(element, 69, 'shift'); // 'e'
+ assert.isTrue(expandCollapseStub.lastCall.calledWith(true));
+ expandCollapseStub.restore();
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index 864712d..ebb1f87 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -170,9 +170,9 @@
<div class="show-hide">
<label class="show-hide">
<input type="checkbox" class="show-hide"
- checked$="[[_commentCollapsed]]"
+ checked$="[[collapsed]]"
on-change="_handleToggleCollapsed">
- [[_computeShowHideText(_commentCollapsed)]]
+ [[_computeShowHideText(collapsed)]]
</label>
</div>
</div>
@@ -186,7 +186,7 @@
<gr-linked-text class="message"
pre
content="[[comment.message]]"
- collapsed="[[_commentCollapsed]]"
+ collapsed="[[collapsed]]"
config="[[projectConfig.commentlinks]]"></gr-linked-text>
<div class="actions" hidden$="[[!showActions]]">
<gr-button class="action reply" on-tap="_handleReply">Reply</gr-button>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 791f949..2abd0a8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -81,7 +81,7 @@
},
patchNum: String,
showActions: Boolean,
- _commentCollapsed: {
+ collapsed: {
type: Boolean,
value: true,
observer: '_toggleCollapseClass',
@@ -103,7 +103,7 @@
attached: function() {
if (this.editing) {
- this._commentCollapsed = false;
+ this.collapsed = false;
}
},
@@ -226,11 +226,11 @@
},
_handleToggleCollapsed: function() {
- this._commentCollapsed = !this._commentCollapsed;
+ this.collapsed = !this.collapsed;
},
- _toggleCollapseClass: function(_commentCollapsed) {
- if (_commentCollapsed) {
+ _toggleCollapseClass: function(collapsed) {
+ if (collapsed) {
this.$.container.classList.add('collapsed');
} else {
this.$.container.classList.remove('collapsed');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index b05746e..0a591ae 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -66,6 +66,7 @@
test('collapsible comments', function() {
// When a comment (not draft) is loaded, it should be collapsed
+ assert.isTrue(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isFalse(isVisible(element.$$('.actions')),
@@ -80,6 +81,7 @@
// When the header row is clicked, the comment should expand
MockInteractions.tap(element.$.header);
+ assert.isFalse(element.collapsed);
assert.isTrue(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is visible');
assert.isTrue(isVisible(element.$$('.actions')),
@@ -124,6 +126,29 @@
'Should navigate to ' + dest + ' without triggering nav');
showStub.restore();
});
+
+ test('comment expand and collapse', function() {
+ element.collapsed = true;
+ assert.isFalse(isVisible(element.$$('gr-linked-text')),
+ 'gr-linked-text is not visible');
+ assert.isFalse(isVisible(element.$$('.actions')),
+ 'actions are not visible');
+ assert.isFalse(isVisible(element.$$('iron-autogrow-textarea')),
+ 'textarea is not visible');
+ assert.isTrue(isVisible(element.$$('.collapsedContent')),
+ 'header middle content is visible');
+
+ element.collapsed = false;
+ assert.isFalse(element.collapsed);
+ assert.isTrue(isVisible(element.$$('gr-linked-text')),
+ 'gr-linked-text is visible');
+ assert.isTrue(isVisible(element.$$('.actions')),
+ 'actions are visible');
+ assert.isFalse(isVisible(element.$$('iron-autogrow-textarea')),
+ 'textarea is not visible');
+ assert.isFalse(isVisible(element.$$('.collapsedContent')),
+ 'header middle content is is not visible');
+ });
});
suite('gr-diff-comment draft tests', function() {
@@ -213,6 +238,7 @@
assert.ok(e.detail.comment);
done();
});
+ assert.isTrue(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isFalse(isVisible(element.$$('.actions')),
@@ -223,6 +249,7 @@
'header middle content is visible');
MockInteractions.tap(element.$.header);
+ assert.isFalse(element.collapsed);
assert.isTrue(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is visible');
assert.isTrue(isVisible(element.$$('.actions')),
@@ -235,6 +262,7 @@
// When the edit button is pressed, should still see the actions
// and also textarea
MockInteractions.tap(element.$$('.edit'));
+ assert.isFalse(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isTrue(isVisible(element.$$('.actions')),
@@ -247,6 +275,7 @@
// When toggle again, everything should be hidden except for textarea
// and header middle content should be visible
MockInteractions.tap(element.$.header);
+ assert.isTrue(element.collapsed);
assert.isFalse(isVisible(element.$$('gr-linked-text')),
'gr-linked-text is not visible');
assert.isFalse(isVisible(element.$$('.actions')),
