opendev/gerrit
Code Issues Proposed changes

Merge "Fix bug in comment control hiding"

Browse Source
This commit is contained in:
Wyatt Allen 2017-03-10 01:53:34 +00:00 committed by Gerrit Code Review
commit 7c5103310b
3 changed files with 66 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -73,14 +73,14 @@ limitations under the License.
</div>
<span
id="messageControlsContainer"
hidden$="[[_computeShowHideTextHidden(_visibleMessages.length, _processedMessages, _hideAutomated)]]">
hidden$="[[_computeShowHideTextHidden(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]">
<gr-button id="oldMessagesBtn" link on-tap="_handleShowAllTap">
[[_computeNumMessagesText(_visibleMessages.length, _processedMessages, _hideAutomated)]]
[[_computeNumMessagesText(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]
</gr-button>
/
<gr-button id="incrementMessagesBtn" link
on-tap="_handleIncrementShownMessages">
[[_computeIncrementText(_visibleMessages.length, _processedMessages, _hideAutomated)]]
[[_computeIncrementText(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]
</gr-button>
</span>
<template

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

@ -255,9 +255,9 @@
* remaining in the list and the number of messages needed to display five
* more visible messages in the list.
*/
_getDelta: function(numVisible, messages, hideAutomated) {
_getDelta: function(visibleMessages, messages, hideAutomated) {
var delta = MESSAGES_INCREMENT;
var msgsRemaining = messages.length - numVisible;
var msgsRemaining = messages.length - visibleMessages.length;
if (hideAutomated) {
var counter = 0;
var i;
@ -273,29 +273,34 @@
* Gets the number of messages that would be visible, but do not currently
* exist in _visibleMessages.
*/
_numRemaining: function(numVisible, messages, hideAutomated) {
var total = hideAutomated ?
messages.filter(function(msg) {
return !this._isAutomated(msg);
}.bind(this)).length :
messages.length;
return total - numVisible;
_numRemaining: function(visibleMessages, messages, hideAutomated) {
if (hideAutomated) {
return this._getHumanMessages(messages).length -
this._getHumanMessages(visibleMessages).length;
}
return messages.length - visibleMessages.length;
},
_computeIncrementText: function(numVisible, messages, hideAutomated) {
var delta = this._getDelta(numVisible, messages, hideAutomated);
_computeIncrementText: function(visibleMessages, messages, hideAutomated) {
var delta = this._getDelta(visibleMessages, messages, hideAutomated);
delta = Math.min(
this._numRemaining(numVisible, messages, hideAutomated), delta);
this._numRemaining(visibleMessages, messages, hideAutomated), delta);
return 'Show ' + Math.min(MESSAGES_INCREMENT, delta) + ' more';
},
_computeShowHideTextHidden: function(numVisible, messages, hideAutomated) {
if (numVisible >= messages.length) { return true; }
if (!hideAutomated) {
return numVisible >= messages.length;
_getHumanMessages: function(messages) {
return messages.filter(function(msg) {
return !this._isAutomated(msg);
}.bind(this));
},
_computeShowHideTextHidden: function(visibleMessages, messages,
hideAutomated) {
if (hideAutomated) {
messages = this._getHumanMessages(messages);
visibleMessages = this._getHumanMessages(visibleMessages);
}
var hiddenMessages = messages.slice(0, messages.length - numVisible);
return this._hasAutomatedMessages(hiddenMessages);
return visibleMessages.length >= messages.length;
},
_handleShowAllTap: function() {
@ -304,9 +309,9 @@
},
_handleIncrementShownMessages: function() {
var len = this._visibleMessages.length;
var delta = this._getDelta(len, this._processedMessages,
var delta = this._getDelta(this._visibleMessages, this._processedMessages,
this._hideAutomated);
var len = this._visibleMessages.length;
var newMessages = this._processedMessages.slice(-(len + delta), -len);
// Add newMessages to the beginning of _visibleMessages
this.splice.apply(this, ['_visibleMessages', 0, 0].concat(newMessages));
@ -317,8 +322,9 @@
this._visibleMessages = messages.slice(-MAX_INITIAL_SHOWN_MESSAGES);
},
_computeNumMessagesText: function(numVisible, messages, hideAutomated) {
var total = this._numRemaining(numVisible, messages, hideAutomated);
_computeNumMessagesText: function(visibleMessages, messages,
hideAutomated) {
var total = this._numRemaining(visibleMessages, messages, hideAutomated);
return total === 1 ? 'Show 1 message' : 'Show all ' + total + ' messages';
},
});

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

@ -128,6 +128,20 @@ limitations under the License.
assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
});
test('message count still respects non-automated on toggle', function() {
element.messages = _.times(10, randomMessage)
.concat(_.times(11, randomAutomated));
flushAsynchronousOperations();
assert.equal(element.$.oldMessagesBtn.innerText, 'Show 1 message');
assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
MockInteractions.tap(element.$.automatedMessageToggle);
flushAsynchronousOperations();
assert.equal(element.$.oldMessagesBtn.innerText, 'Show 1 message');
assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
});
test('show all messages respects expand', function() {
element.messages = _.times(10, randomAutomated)
.concat(_.times(11, randomMessage));
@ -450,19 +464,34 @@ limitations under the License.
test('_getDelta', function() {
var messages = [randomMessage()];
assert.equal(element._getDelta(0, messages, false), 1);
assert.equal(element._getDelta(0, messages, true), 1);
assert.equal(element._getDelta([], messages, false), 1);
assert.equal(element._getDelta([], messages, true), 1);
messages = _.times(7, randomMessage);
assert.equal(element._getDelta(0, messages, false), 5);
assert.equal(element._getDelta(0, messages, true), 5);
assert.equal(element._getDelta([], messages, false), 5);
assert.equal(element._getDelta([], messages, true), 5);
messages = _.times(4, randomMessage)
.concat(_.times(2, randomAutomated))
.concat(_.times(3, randomMessage));
assert.equal(element._getDelta(2, messages, false), 5);
assert.equal(element._getDelta(2, messages, true), 7);
var dummyArr = _.times(2, randomMessage);
assert.equal(element._getDelta(dummyArr, messages, false), 5);
assert.equal(element._getDelta(dummyArr, messages, true), 7);
});
test('_getHumanMessages', function() {
assert.equal(
element._getHumanMessages(_.times(5, randomAutomated)).length, 0);
assert.equal(
element._getHumanMessages(_.times(5, randomMessage)).length, 5);
var messages = _.shuffle(_.times(5, randomMessage)
.concat(_.times(5, randomAutomated)));
messages = element._getHumanMessages(messages);
assert.equal(messages.length, 5);
assert.isFalse(element._hasAutomatedMessages(messages));
});
});
</script>