opendev/gerrit
Code Issues Proposed changes

Remove unnecessary checks around computing threads

Browse Source 
Also change the observer on `threads` and `threads.splices`
instead of `threads.*` for simplicity and readability,
sub properties changes are propagated from the parents
and become a `threads` change (override) when reach gr-thread-list.

Change-Id: If0e29c4d9fcaf325d6b04b6046dcb3a1f4062e07
This commit is contained in:
Tao Zhou
2020-06-02 17:22:45 +02:00
parent cedc3b7bc0
commit 7ffe256adb
1 changed files with 12 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
View File

@@ -74,48 +74,35 @@ class GrThreadList extends GestureEventListeners(
}; };
} }
static get observers() { return ['_computeSortedThreads(threads.*)']; } static get observers() {
return ['_updateSortedThreads(threads, threads.splices)'];
}
_computeShowDraftToggle(loggedIn) { _computeShowDraftToggle(loggedIn) {
return loggedIn ? 'show' : ''; return loggedIn ? 'show' : '';
} }
/** /**
* Order as follows: * Observer on threads and update _sortedThreads when needed.
* - Unresolved threads with drafts (reverse chronological)
* - Unresolved threads without drafts (reverse chronological)
* - Resolved threads with drafts (reverse chronological)
* - Resolved threads without drafts (reverse chronological)
* *
* @param {!Object} changeRecord * @param {Array<Object>} threads
* @param {!Object} spliceRecord
*/ */
_computeSortedThreads(changeRecord) { _updateSortedThreads(threads, spliceRecord) {
const baseThreads = changeRecord.base; if (!threads) {
const threads = changeRecord.value;
if (!baseThreads || !threads) {
this._sortedThreads = []; this._sortedThreads = [];
return; return;
} }
// TODO: should change how data flows to solve the root cause
// We only want to sort on thread additions / removals to avoid // We only want to sort on thread additions / removals to avoid
// re-rendering on modifications (add new reply / edit draft etc) // re-rendering on modifications (add new reply / edit draft etc)
// https://polymer-library.polymer-project.org/3.0/docs/devguide/observers#array-observation // https://polymer-library.polymer-project.org/3.0/docs/devguide/observers#array-observation
let shouldSort = true; const isArrayMutation = spliceRecord &&
if (threads.indexSplices) { (spliceRecord.indexSplices.addedCount !== 0
// Array splice mutations || spliceRecord.indexSplices.removed.length);
shouldSort = threads.indexSplices.addedCount !== 0
|| threads.indexSplices.removed.length;
} else {
// A replace mutation
shouldSort = threads.length !== baseThreads.length;
}
this._updateSortedThreads(baseThreads, shouldSort);
}
_updateSortedThreads(threads, shouldSort) {
if (this._sortedThreads if (this._sortedThreads
&& this._sortedThreads.length === threads.length && this._sortedThreads.length === threads.length
&& !shouldSort) { && !isArrayMutation) {
// Instead of replacing the _sortedThreads which will trigger a re-render, // Instead of replacing the _sortedThreads which will trigger a re-render,
// we override all threads inside of it // we override all threads inside of it
for (const thread of threads) { for (const thread of threads) {