Reduce number of DOM Elements for Change Log

Gr-hovercard-account is shown only on hover. After fix, the app
creates all its DOM elements after hovering for the first time.
This saves 20 elements per account label, per 1 line in change log.

Gr-comment shows Save/Edit/Cancel actions only on drafts. After fix,
the app creates these actions only on drafts and not on all comments.
This saves 35 elements per comment.

Gr-message shows the Reply button only when expanded. After the fix
the app creates the button only after expanding for the first time.
This saves 20 elements per message.

When tested on /c/gerrit/+/251578 Change:
 - Show all entries was faster by 22% (1s -> 0.78s)
 - Expand all was faster by 47% (4.3s -> 2.3s)
 - Change pageload was faster by 3% (3.4s -> 3.3s)

Change-Id: Idfe2aebc78dda9555005aa5a33caeca31d583d8a
This commit is contained in:
Milutin Kristofic
2020-05-15 21:55:08 +02:00
parent a96173b036
commit c218df50e3
7 changed files with 105 additions and 86 deletions

View File

@@ -218,6 +218,7 @@ export const htmlTemplate = html`
content="[[_messageContentExpanded]]" content="[[_messageContentExpanded]]"
config="[[_projectConfig.commentlinks]]" config="[[_projectConfig.commentlinks]]"
></gr-formatted-text> ></gr-formatted-text>
<template is="dom-if" if="[[_expanded]]">
<template is="dom-if" if="[[_messageContentExpanded]]"> <template is="dom-if" if="[[_messageContentExpanded]]">
<div <div
class="replyActionContainer" class="replyActionContainer"
@@ -245,7 +246,6 @@ export const htmlTemplate = html`
</gr-button> </gr-button>
</div> </div>
</template> </template>
<template is="dom-if" if="[[_expanded]]">
<template is="dom-if" if="[[!_isCleanerLogExperimentEnabled]]"> <template is="dom-if" if="[[!_isCleanerLogExperimentEnabled]]">
<gr-comment-list <gr-comment-list
comments="[[comments]]" comments="[[comments]]"

View File

@@ -62,7 +62,7 @@ suite('gr-message tests', () => {
date: '2016-01-12 20:24:49.448000000', date: '2016-01-12 20:24:49.448000000',
message: 'Uploaded patch set 1.', message: 'Uploaded patch set 1.',
_revision_number: 1, _revision_number: 1,
expanded: false, expanded: true,
}; };
element.addEventListener('reply', e => { element.addEventListener('reply', e => {
@@ -87,7 +87,7 @@ suite('gr-message tests', () => {
date: '2016-01-12 20:24:49.448000000', date: '2016-01-12 20:24:49.448000000',
message: 'Uploaded patch set 1.', message: 'Uploaded patch set 1.',
_revision_number: 1, _revision_number: 1,
expanded: false, expanded: true,
}; };
flushAsynchronousOperations(); flushAsynchronousOperations();
@@ -105,7 +105,7 @@ suite('gr-message tests', () => {
date: '2016-01-12 20:24:49.448000000', date: '2016-01-12 20:24:49.448000000',
message: 'Uploaded patch set 1.', message: 'Uploaded patch set 1.',
_revision_number: 1, _revision_number: 1,
expanded: false, expanded: true,
}; };
element.addEventListener('change-message-deleted', e => { element.addEventListener('change-message-deleted', e => {
@@ -378,7 +378,7 @@ suite('gr-message tests', () => {
date: '2016-01-12 20:24:49.448000000', date: '2016-01-12 20:24:49.448000000',
message: 'Uploaded patch set 1.', message: 'Uploaded patch set 1.',
_revision_number: 1, _revision_number: 1,
expanded: false, expanded: true,
}; };
flushAsynchronousOperations(); flushAsynchronousOperations();
@@ -414,7 +414,7 @@ suite('gr-message tests', () => {
date: '2016-01-12 20:24:49.448000000', date: '2016-01-12 20:24:49.448000000',
message: 'Uploaded patch set 1.', message: 'Uploaded patch set 1.',
_revision_number: 1, _revision_number: 1,
expanded: false, expanded: true,
}; };
flushAsynchronousOperations(); flushAsynchronousOperations();
@@ -444,7 +444,7 @@ suite('gr-message tests', () => {
date: '2016-01-12 20:24:49.448000000', date: '2016-01-12 20:24:49.448000000',
message: 'not empty', message: 'not empty',
_revision_number: 1, _revision_number: 1,
expanded: false, expanded: true,
}; };
flushAsynchronousOperations(); flushAsynchronousOperations();
replyEl = element.shadowRoot.querySelector('.replyActionContainer'); replyEl = element.shadowRoot.querySelector('.replyActionContainer');

View File

@@ -483,7 +483,10 @@ class GrComment extends mixinBehaviors( [
this.$.container.classList.toggle('editing', editing); this.$.container.classList.toggle('editing', editing);
if (this.comment && this.comment.id) { if (this.comment && this.comment.id) {
this.shadowRoot.querySelector('.cancel').hidden = !editing; const cancelButton = this.shadowRoot.querySelector('.cancel');
if (cancelButton) {
cancelButton.hidden = !editing;
}
} }
if (this.comment) { if (this.comment) {
this.comment.__editing = this.editing; this.comment.__editing = this.editing;

View File

@@ -361,6 +361,7 @@ export const htmlTemplate = html`
Resolved Resolved
</label> </label>
</div> </div>
<template is="dom-if" if="[[draft]]">
<div class="rightActions"> <div class="rightActions">
<gr-button <gr-button
link="" link=""
@@ -388,6 +389,7 @@ export const htmlTemplate = html`
>Save</gr-button >Save</gr-button
> >
</div> </div>
</template>
</div> </div>
<div class="robotActions" hidden$="[[!_showRobotActions]]"> <div class="robotActions" hidden$="[[!_showRobotActions]]">
<template is="dom-if" if="[[isRobotComment]]"> <template is="dom-if" if="[[isRobotComment]]">

View File

@@ -348,6 +348,8 @@ suite('gr-comment tests', () => {
test('edit reports interaction', () => { test('edit reports interaction', () => {
const reportStub = sandbox.stub(element.reporting, const reportStub = sandbox.stub(element.reporting,
'recordDraftInteraction'); 'recordDraftInteraction');
element.draft = true;
flushAsynchronousOperations();
MockInteractions.tap(element.shadowRoot MockInteractions.tap(element.shadowRoot
.querySelector('.edit')); .querySelector('.edit'));
assert.isTrue(reportStub.calledOnce); assert.isTrue(reportStub.calledOnce);
@@ -357,6 +359,7 @@ suite('gr-comment tests', () => {
const reportStub = sandbox.stub(element.reporting, const reportStub = sandbox.stub(element.reporting,
'recordDraftInteraction'); 'recordDraftInteraction');
element.draft = true; element.draft = true;
flushAsynchronousOperations();
MockInteractions.tap(element.shadowRoot MockInteractions.tap(element.shadowRoot
.querySelector('.discard')); .querySelector('.discard'));
assert.isTrue(reportStub.calledOnce); assert.isTrue(reportStub.calledOnce);
@@ -427,6 +430,7 @@ suite('gr-comment tests', () => {
.querySelector('.robotActions').hasAttribute('hidden')); .querySelector('.robotActions').hasAttribute('hidden'));
element.draft = true; element.draft = true;
flushAsynchronousOperations();
assert.isTrue(isVisible(element.shadowRoot assert.isTrue(isVisible(element.shadowRoot
.querySelector('.edit')), 'edit is visible'); .querySelector('.edit')), 'edit is visible');
assert.isTrue(isVisible(element.shadowRoot assert.isTrue(isVisible(element.shadowRoot
@@ -552,6 +556,8 @@ suite('gr-comment tests', () => {
// When the edit button is pressed, should still see the actions // When the edit button is pressed, should still see the actions
// and also textarea // and also textarea
element.draft = true;
flushAsynchronousOperations();
MockInteractions.tap(element.shadowRoot MockInteractions.tap(element.shadowRoot
.querySelector('.edit')); .querySelector('.edit'));
flushAsynchronousOperations(); flushAsynchronousOperations();
@@ -655,6 +661,8 @@ suite('gr-comment tests', () => {
test('draft creation/cancellation', done => { test('draft creation/cancellation', done => {
assert.isFalse(element.editing); assert.isFalse(element.editing);
element.draft = true;
flushAsynchronousOperations();
MockInteractions.tap(element.shadowRoot MockInteractions.tap(element.shadowRoot
.querySelector('.edit')); .querySelector('.edit'));
assert.isTrue(element.editing); assert.isTrue(element.editing);
@@ -787,6 +795,7 @@ suite('gr-comment tests', () => {
const cancelDebounce = sandbox.stub(element, 'cancelDebouncer'); const cancelDebounce = sandbox.stub(element, 'cancelDebouncer');
element.draft = true; element.draft = true;
flushAsynchronousOperations();
MockInteractions.tap(element.shadowRoot MockInteractions.tap(element.shadowRoot
.querySelector('.edit')); .querySelector('.edit'));
element._messageText = 'good news, everyone!'; element._messageText = 'good news, everyone!';
@@ -852,6 +861,7 @@ suite('gr-comment tests', () => {
const saveStub = sandbox.stub(element, 'save').returns(Promise.resolve()); const saveStub = sandbox.stub(element, 'save').returns(Promise.resolve());
element.showActions = true; element.showActions = true;
element.draft = true; element.draft = true;
flushAsynchronousOperations();
MockInteractions.tap(element.$.header); MockInteractions.tap(element.$.header);
MockInteractions.tap(element.shadowRoot MockInteractions.tap(element.shadowRoot
.querySelector('.edit')); .querySelector('.edit'));

View File

@@ -64,6 +64,7 @@ export const htmlTemplate = html`
} }
</style> </style>
<div id="container" role="tooltip" tabindex="-1"> <div id="container" role="tooltip" tabindex="-1">
<template is="dom-if" if="[[_isShowing]]">
<div class="top"> <div class="top">
<div class="avatar"> <div class="avatar">
<gr-avatar account="[[account]]" image-size="56"></gr-avatar> <gr-avatar account="[[account]]" image-size="56"></gr-avatar>
@@ -92,5 +93,6 @@ export const htmlTemplate = html`
<iron-icon icon="gr-icons:attention"></iron-icon> <iron-icon icon="gr-icons:attention"></iron-icon>
<span>It is this user's turn to take action.</span> <span>It is this user's turn to take action.</span>
</div> </div>
</template>
</div> </div>
`; `;

View File

@@ -49,6 +49,8 @@ limitations under the License.
setup(() => { setup(() => {
element = fixture('basic'); element = fixture('basic');
element.account = Object.assign({}, ACCOUNT); element.account = Object.assign({}, ACCOUNT);
element.show({});
flushAsynchronousOperations();
}); });
test('account name is shown', () => { test('account name is shown', () => {