opendev/gerrit
Code Issues Proposed changes

Merge "Preserve expanded state for hidden messages"

Browse Source
This commit is contained in:
Viktar Donich 2017-03-07 19:51:22 +00:00 committed by Gerrit Code Review
commit 3591b5a64e
4 changed files with 96 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
polygerrit-ui/app/elements/change/gr-message/gr-message.html
View File

@ -124,7 +124,7 @@ limitations under the License.
padding: .5em 0 1em;
}
</style>
<div class$="[[_computeClass(expanded, showAvatar)]]">
<div class$="[[_computeClass(_expanded, showAvatar)]]">
<gr-avatar account="[[author]]" image-size="100"></gr-avatar>
<div class="contentContainer">
<div class="name" on-tap="_handleNameTap">[[author.name]]</div>

32
polygerrit-ui/app/elements/change/gr-message/gr-message.js
View File

@ -45,11 +45,6 @@
observer: '_commentsChanged',
},
config: Object,
expanded: {
type: Boolean,
value: true,
reflectToAttribute: true,
},
hideAutomated: {
type: Boolean,
value: false,
@ -72,6 +67,11 @@
computed: '_computeShowReplyButton(message, _loggedIn)',
},
projectConfig: Object,
// Computed property needed to trigger Polymer value observing.
_expanded: {
type: Object,
computed: '_computeExpanded(message.expanded)',
},
_loggedIn: {
type: Boolean,
value: false,
@ -99,19 +99,31 @@
return !!message.message && loggedIn;
},
_computeExpanded: function(expanded) {
return expanded;
},
/**
* If there is no value set on the message object as to whether _expanded
* should be true or not, then _expanded is set to true if there are
* inline comments (otherwise false).
*/
_commentsChanged: function(value) {
this.expanded = Object.keys(value || {}).length > 0;
if (this.message && this.message.expanded === undefined) {
this.set('message.expanded', Object.keys(value || {}).length > 0);
}
},
_handleTap: function(e) {
if (this.expanded) { return; }
this.expanded = true;
if (this.message.expanded) { return; }
e.stopPropagation();
this.set('message.expanded', true);
},
_handleNameTap: function(e) {
if (!this.expanded) { return; }
if (!this.message.expanded) { return; }
e.stopPropagation();
this.expanded = false;
this.set('message.expanded', false);
},
_computeIsAutomated: function(message) {

20
polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
View File

@ -45,6 +45,7 @@
_expanded: {
type: Boolean,
value: false,
observer: '_expandedChanged',
},
_hideAutomated: {
type: Boolean,
@ -71,7 +72,7 @@
var el = this.$$('[data-message-id="' + messageID + '"]');
if (!el) { return; }
el.expanded = true;
el.set('message.expanded', true);
var top = el.offsetTop;
for (var offsetParent = el.offsetParent;
offsetParent;
@ -121,6 +122,15 @@
return result;
},
_expandedChanged: function(exp) {
for (var i = 0; i < this._processedMessages.length; i++) {
this._processedMessages[i].expanded = exp;
if (i < this._visibleMessages.length) {
this.set(['_visibleMessages', i, 'expanded'], exp);
}
}
},
_highlightEl: function(el) {
var highlightedEls =
Polymer.dom(this.root).querySelectorAll('.highlighted');
@ -136,14 +146,10 @@
},
/**
* @param {boolean} expand
*/
* @param {boolean} expand
*/
handleExpandCollapse: function(expand) {
this._expanded = expand;
var messageEls = Polymer.dom(this.root).querySelectorAll('gr-message');
for (var i = 0; i < messageEls.length; i++) {
messageEls[i].expanded = expand;
}
},
_handleExpandCollapseTap: function(e) {

71
polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
View File

@ -122,42 +122,91 @@ limitations under the License.
assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
});
test('show all messages respects expand', function() {
element.messages = _.times(10, randomAutomated)
.concat(_.times(11, randomMessage));
flushAsynchronousOperations();
MockInteractions.tap(element.$$('#collapse-messages')); // Expand all.
flushAsynchronousOperations();
var messages = getMessages();
assert.equal(messages.length, 20);
for (var i = 0; i < messages.length; i++) {
assert.isTrue(messages[i]._expanded);
}
MockInteractions.tap(element.$.oldMessagesBtn);
flushAsynchronousOperations();
messages = getMessages();
assert.equal(messages.length, 21);
for (var i = 0; i < messages.length; i++) {
assert.isTrue(messages[i]._expanded);
}
});
test('show all messages respects collapse', function() {
element.messages = _.times(10, randomAutomated)
.concat(_.times(11, randomMessage));
flushAsynchronousOperations();
MockInteractions.tap(element.$$('#collapse-messages')); // Expand all.
MockInteractions.tap(element.$$('#collapse-messages')); // Collapse all.
flushAsynchronousOperations();
var messages = getMessages();
assert.equal(messages.length, 20);
for (var i = 0; i < messages.length; i++) {
assert.isFalse(messages[i]._expanded);
}
MockInteractions.tap(element.$.oldMessagesBtn);
flushAsynchronousOperations();
messages = getMessages();
assert.equal(messages.length, 21);
for (var i = 0; i < messages.length; i++) {
assert.isFalse(messages[i]._expanded);
}
});
test('expand/collapse all', function() {
var allMessageEls = getMessages();
for (var i = 0; i < allMessageEls.length; i++) {
allMessageEls[i].expanded = false;
allMessageEls[i]._expanded = false;
}
MockInteractions.tap(allMessageEls[1]);
assert.isTrue(allMessageEls[1].expanded);
assert.isTrue(allMessageEls[1]._expanded);
MockInteractions.tap(element.$$('#collapse-messages'));
allMessageEls = getMessages();
for (var i = 0; i < allMessageEls.length; i++) {
assert.isTrue(allMessageEls[i].expanded);
assert.isTrue(allMessageEls[i]._expanded);
}
MockInteractions.tap(element.$$('#collapse-messages'));
allMessageEls = getMessages();
for (var i = 0; i < allMessageEls.length; i++) {
assert.isFalse(allMessageEls[i].expanded);
assert.isFalse(allMessageEls[i]._expanded);
}
});
test('expand/collapse from external keypress', function() {
element.handleExpandCollapse(true);
MockInteractions.tap(element.$$('#collapse-messages'));
var allMessageEls = getMessages();
for (var i = 0; i < allMessageEls.length; i++) {
assert.isTrue(allMessageEls[i].expanded);
assert.isTrue(allMessageEls[i]._expanded);
}
// Expand/collapse all text also changes.
assert.equal(element.$$('#collapse-messages').textContent.trim(),
'Collapse all');
element.handleExpandCollapse(false);
MockInteractions.tap(element.$$('#collapse-messages'));
allMessageEls = getMessages();
for (var i = 0; i < allMessageEls.length; i++) {
assert.isFalse(allMessageEls[i].expanded);
assert.isFalse(allMessageEls[i]._expanded);
}
// Expand/collapse all text also changes.
assert.equal(element.$$('#collapse-messages').textContent.trim(),
@ -172,7 +221,7 @@ limitations under the License.
test('scroll to message', function() {
var allMessageEls = getMessages();
for (var i = 0; i < allMessageEls.length; i++) {
allMessageEls[i].expanded = false;
allMessageEls[i].set('message.expanded', false);
}
var scrollToStub = sinon.stub(window, 'scrollTo');
@ -181,14 +230,14 @@ limitations under the License.
element.scrollToMessage('invalid');
for (var i = 0; i < allMessageEls.length; i++) {
assert.isFalse(allMessageEls[i].expanded,
assert.isFalse(allMessageEls[i]._expanded,
'expected gr-message ' + i + ' to not be expanded');
}
var messageID = messages[1].id;
element.scrollToMessage(messageID);
assert.isTrue(
element.$$('[data-message-id="' + messageID + '"]').expanded);
element.$$('[data-message-id="' + messageID + '"]')._expanded);
assert.isTrue(scrollToStub.calledOnce);
assert.isTrue(highlightStub.calledOnce);