opendev/gerrit
Code Issues Proposed changes

Keyboard shortcuts to expand and collapse all comments

Browse Source 
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
This commit is contained in:
Becky Siegel 2016-10-10 17:31:56 -07:00
parent 4b9082e57a
commit eb4ea184f7
5 changed files with 106 additions and 34 deletions

24
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);

71
polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html

@ -54,30 +54,25 @@ limitations under the License.
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 @@ limitations under the License.
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 @@ limitations under the License.
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 @@ limitations under the License.
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>

6
polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html

@ -170,9 +170,9 @@ limitations under the License.
<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 @@ limitations under the License.
<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>

10
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');

29
polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html

@ -66,6 +66,7 @@ limitations under the License.
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 @@ limitations under the License.
// 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 @@ limitations under the License.
'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 @@ limitations under the License.
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 @@ limitations under the License.
'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 @@ limitations under the License.
// 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 @@ limitations under the License.
// 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')),