commit 548f0f425038cd4eb43e9d50fafa27b2293f2e9d
Author: Dhruv Srivastava <dhruvsri@google.com>
Date:   Fri Aug 28 14:24:13 2020 +0200

    Toggle default to show unresolved comments
    
    Screenshots: https://imgur.com/a/4Yw7OcR
    Change-Id: I4cf36dda500f8561803470811d795f0b5260c235

diff --git a/polygerrit-ui/app/constants/messages.ts b/polygerrit-ui/app/constants/messages.ts
index 15692bd..5b4a534 100644
--- a/polygerrit-ui/app/constants/messages.ts
+++ b/polygerrit-ui/app/constants/messages.ts
@@ -15,10 +15,6 @@
  * limitations under the License.
  */
 
-/** @desc Default message shown when no threads in gr-thread-list */
-export const NO_THREADS_MSG =
-  'There are no inline comment threads on any diff for this change.';
-
 /** @desc Message shown when no threads in gr-thread-list for robot comments */
 export const NO_ROBOT_COMMENTS_THREADS_MSG =
   'There are no findings for this patchset.';
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index f0b8372..6af8960 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -637,6 +637,7 @@ export const htmlTemplate = html`
           logged-in="[[_loggedIn]]"
           only-show-robot-comments-with-human-reply=""
           on-thread-list-modified="_handleReloadDiffComments"
+          unresolved-only
         ></gr-thread-list>
       </template>
       <template
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
index 7585d16..6a32834 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.ts
@@ -24,12 +24,14 @@ import {PolymerElement} from '@polymer/polymer/polymer-element';
 import {htmlTemplate} from './gr-thread-list_html';
 import {parseDate} from '../../../utils/date-util';
 
-import {NO_THREADS_MSG} from '../../../constants/messages';
 import {CommentSide, SpecialFilePath} from '../../../constants/constants';
 import {customElement, observe, property} from '@polymer/decorators';
-import {CommentThread, isDraft, UIRobot} from '../../../utils/comment-util';
-import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
+import {
+  PolymerSpliceChange,
+  PolymerDeepPropertyChange,
+} from '@polymer/polymer/interfaces';
 import {ChangeInfo} from '../../../types/common';
+import {CommentThread, isDraft, UIRobot} from '../../../utils/comment-util';
 
 interface CommentThreadWithInfo {
   thread: CommentThread;
@@ -64,8 +66,19 @@ export class GrThreadList extends GestureEventListeners(
   @property({type: Array})
   _sortedThreads: CommentThread[] = [];
 
+  @property({
+    computed:
+      '_computeDisplayedThreads(_sortedThreads.*, unresolvedOnly, ' +
+      '_draftsOnly, onlyShowRobotCommentsWithHumanReply)',
+    type: Array,
+  })
+  _displayedThreads: CommentThread[] = [];
+
+  // thread-list is used in multiple places like the change log, hence
+  // keeping the default to be false. When used in comments tab, it's
+  // set as true.
   @property({type: Boolean})
-  _unresolvedOnly = false;
+  unresolvedOnly = false;
 
   @property({type: Boolean})
   _draftsOnly = false;
@@ -76,13 +89,53 @@ export class GrThreadList extends GestureEventListeners(
   @property({type: Boolean})
   hideToggleButtons = false;
 
-  @property({type: String})
-  emptyThreadMsg = NO_THREADS_MSG;
-
   _computeShowDraftToggle(loggedIn?: boolean) {
     return loggedIn ? 'show' : '';
   }
 
+  _showEmptyThreadsMessage(
+    threads: CommentThread[],
+    displayedThreads: CommentThread[],
+    unresolvedOnly: boolean
+  ) {
+    if (!threads || !displayedThreads) return false;
+    return !threads.length || (unresolvedOnly && !displayedThreads.length);
+  }
+
+  _computeEmptyThreadsMessage(threads: CommentThread[]) {
+    return !threads.length ? 'No comments.' : 'No unresolved comments';
+  }
+
+  _showPartyPopper(threads: CommentThread[]) {
+    return !!threads.length;
+  }
+
+  _computeResolvedCommentsMessage(
+    threads: CommentThread[],
+    displayedThreads: CommentThread[],
+    unresolvedOnly: boolean
+  ) {
+    if (unresolvedOnly && threads.length && !displayedThreads.length) {
+      return (
+        `Show ${threads.length} resolved comment` +
+        (threads.length > 1 ? 's' : '')
+      );
+    }
+    return '';
+  }
+
+  _showResolvedCommentsButton(
+    threads: CommentThread[],
+    displayedThreads: CommentThread[],
+    unresolvedOnly: boolean
+  ) {
+    return unresolvedOnly && threads.length && !displayedThreads.length;
+  }
+
+  _handleResolvedCommentsMessageClick() {
+    this.unresolvedOnly = !this.unresolvedOnly;
+  }
+
   _compareThreads(c1: CommentThreadWithInfo, c2: CommentThreadWithInfo) {
     if (c1.thread.path !== c2.thread.path) {
       // '/PATCHSET' will not come before '/COMMIT' when sorting
@@ -166,6 +219,7 @@ export class GrThreadList extends GestureEventListeners(
   ) {
     if (!threads || threads.length === 0) {
       this._sortedThreads = [];
+      this._displayedThreads = [];
       return;
     }
     // We only want to sort on thread additions / removals to avoid
@@ -196,14 +250,34 @@ export class GrThreadList extends GestureEventListeners(
       .map(threadInfo => threadInfo.thread);
   }
 
+  _computeDisplayedThreads(
+    sortedThreadsRecord?: PolymerDeepPropertyChange<
+      CommentThread[],
+      CommentThread[]
+    >,
+    unresolvedOnly?: boolean,
+    draftsOnly?: boolean,
+    onlyShowRobotCommentsWithHumanReply?: boolean
+  ) {
+    if (!sortedThreadsRecord || !sortedThreadsRecord.base) return [];
+    return sortedThreadsRecord.base.filter(t =>
+      this._shouldShowThread(
+        t,
+        unresolvedOnly,
+        draftsOnly,
+        onlyShowRobotCommentsWithHumanReply
+      )
+    );
+  }
+
   _isFirstThreadWithFileName(
-    sortedThreads: CommentThread[],
+    displayedThreads: CommentThread[],
     thread: CommentThread,
     unresolvedOnly?: boolean,
     draftsOnly?: boolean,
     onlyShowRobotCommentsWithHumanReply?: boolean
   ) {
-    const threads = sortedThreads.filter(t =>
+    const threads = displayedThreads.filter(t =>
       this._shouldShowThread(
         t,
         unresolvedOnly,
@@ -219,13 +293,13 @@ export class GrThreadList extends GestureEventListeners(
   }
 
   _shouldRenderSeparator(
-    sortedThreads: CommentThread[],
+    displayedThreads: CommentThread[],
     thread: CommentThread,
     unresolvedOnly?: boolean,
     draftsOnly?: boolean,
     onlyShowRobotCommentsWithHumanReply?: boolean
   ) {
-    const threads = sortedThreads.filter(t =>
+    const threads = displayedThreads.filter(t =>
       this._shouldShowThread(
         t,
         unresolvedOnly,
@@ -240,7 +314,7 @@ export class GrThreadList extends GestureEventListeners(
     return (
       index > 0 &&
       this._isFirstThreadWithFileName(
-        sortedThreads,
+        displayedThreads,
         thread,
         unresolvedOnly,
         draftsOnly,
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
index d74c985..e55f98a 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_html.ts
@@ -57,15 +57,23 @@ export const htmlTemplate = html`
       border-top: 1px solid var(--border-color);
       margin-top: var(--spacing-xl);
     }
+    .resolved-comments-message {
+      color: var(--link-color);
+      cursor: pointer;
+    }
+    .show-resolved-comments {
+      box-shadow: none;
+      padding-left: var(--spacing-m);
+    }
   </style>
   <template is="dom-if" if="[[!hideToggleButtons]]">
     <div class="header">
       <div class="toggleItem">
         <paper-toggle-button
           id="unresolvedToggle"
-          checked="{{_unresolvedOnly}}"
+          checked="{{!unresolvedOnly}}"
           on-tap="_onTapUnresolvedToggle"
-          >Only unresolved threads</paper-toggle-button
+          >All comments</paper-toggle-button
         >
       </div>
       <div
@@ -75,48 +83,63 @@ export const htmlTemplate = html`
           id="draftToggle"
           checked="{{_draftsOnly}}"
           on-tap="_onTapUnresolvedToggle"
-          >Only threads with drafts</paper-toggle-button
+          >Comments with drafts</paper-toggle-button
         >
       </div>
     </div>
   </template>
   <div id="threads">
-    <template is="dom-if" if="[[!threads.length]]">
-      [[emptyThreadMsg]]
+    <template
+      is="dom-if"
+      if="[[_showEmptyThreadsMessage(threads, _displayedThreads, unresolvedOnly)]]"
+    >
+      <div>
+        <span>
+          <template is="dom-if" if="[[_showPartyPopper(threads)]]">
+            <span> \&#x1F389 </span>
+          </template>
+          [[_computeEmptyThreadsMessage(threads, _displayedThreads,
+          unresolvedOnly)]]
+          <template is="dom-if" if="[[_showResolvedCommentsButton(threads, _displayedThreads, unresolvedOnly)]]">
+            <gr-button
+              class="show-resolved-comments"
+              link
+              on-click="_handleResolvedCommentsMessageClick">
+                [[_computeResolvedCommentsMessage(threads, _displayedThreads,
+                unresolvedOnly)]]
+            </gr-button>
+          </template>
+        </span>
+      </div>
     </template>
     <template
       is="dom-repeat"
-      items="[[_sortedThreads]]"
+      items="[[_displayedThreads]]"
       as="thread"
       initial-count="10"
       target-framerate="60"
     >
       <template
         is="dom-if"
-        if="[[_shouldShowThread(thread, _unresolvedOnly, _draftsOnly, onlyShowRobotCommentsWithHumanReply)]]"
+        if="[[_shouldRenderSeparator(_displayedThreads, thread, unresolvedOnly, _draftsOnly, onlyShowRobotCommentsWithHumanReply)]]"
       >
-        <template
-          is="dom-if"
-          if="[[_shouldRenderSeparator(_sortedThreads, thread, _unresolvedOnly, _draftsOnly, onlyShowRobotCommentsWithHumanReply)]]"
-        >
-          <div class="thread-separator"></div>
-        </template>
-        <gr-comment-thread
-          show-file-path=""
-          change-num="[[changeNum]]"
-          comments="[[thread.comments]]"
-          comment-side="[[thread.commentSide]]"
-          show-file-name="[[_isFirstThreadWithFileName(_sortedThreads, thread, _unresolvedOnly, _draftsOnly, onlyShowRobotCommentsWithHumanReply)]]"
-          project-name="[[change.project]]"
-          is-on-parent="[[_isOnParent(thread.commentSide)]]"
-          line-num="[[thread.line]]"
-          patch-num="[[thread.patchNum]]"
-          path="[[thread.path]]"
-          root-id="{{thread.rootId}}"
-          on-thread-changed="_handleCommentsChanged"
-          on-thread-discard="_handleThreadDiscard"
-        ></gr-comment-thread>
+        <div class="thread-separator"></div>
       </template>
+      <gr-comment-thread
+        show-file-path=""
+        change-num="[[changeNum]]"
+        comments="[[thread.comments]]"
+        comment-side="[[thread.commentSide]]"
+        show-file-name="[[_isFirstThreadWithFileName(_displayedThreads, thread, unresolvedOnly, _draftsOnly, onlyShowRobotCommentsWithHumanReply)]]"
+        project-name="[[change.project]]"
+        is-on-parent="[[_isOnParent(thread.commentSide)]]"
+        line-num="[[thread.line]]"
+        patch-num="[[thread.patchNum]]"
+        path="[[thread.path]]"
+        root-id="{{thread.rootId}}"
+        on-thread-changed="_handleCommentsChanged"
+        on-thread-discard="_handleThreadDiscard"
+      ></gr-comment-thread>
     </template>
   </div>
 `;
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
index bad3a99..efc072f 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.js
@@ -18,7 +18,6 @@
 import '../../../test/common-test-setup-karma.js';
 import './gr-thread-list.js';
 import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
-import {NO_THREADS_MSG} from '../../../constants/messages.js';
 import {SpecialFilePath} from '../../../constants/constants.js';
 
 const basicFixture = fixtureFromElement('gr-thread-list');
@@ -287,13 +286,23 @@ suite('gr-thread-list tests', () => {
     assert.equal(getVisibleThreads().length, element.threads.length);
   });
 
+  test('show unresolved threads if unresolvedOnly is set', done => {
+    element.unresolvedOnly = true;
+    flush();
+    const unresolvedThreads = element.threads.filter(t => t.comments.some(
+        c => c.unresolved
+    ));
+    assert.equal(getVisibleThreads().length, unresolvedThreads.length);
+    done();
+  });
+
   test('showing file name takes visible threads into account', () => {
     assert.equal(element._isFirstThreadWithFileName(element._sortedThreads,
-        element._sortedThreads[2], element._unresolvedOnly, element._draftsOnly,
+        element._sortedThreads[2], element.unresolvedOnly, element._draftsOnly,
         element.onlyShowRobotCommentsWithHumanReply), true);
-    element._unresolvedOnly = true;
+    element.unresolvedOnly = true;
     assert.equal(element._isFirstThreadWithFileName(element._sortedThreads,
-        element._sortedThreads[2], element._unresolvedOnly, element._draftsOnly,
+        element._sortedThreads[2], element.unresolvedOnly, element._draftsOnly,
         element.onlyShowRobotCommentsWithHumanReply), false);
   });
 
@@ -539,7 +548,7 @@ suite('gr-thread-list tests', () => {
     });
   });
 
-  test('toggle unresolved only shows unresolved comments', () => {
+  test('toggle unresolved shows all comments', () => {
     MockInteractions.tap(element.shadowRoot.querySelector(
         '#unresolvedToggle'));
     flush();
@@ -617,18 +626,9 @@ suite('gr-thread-list tests', () => {
     });
 
     test('default empty message should show', () => {
-      assert.equal(
-          element.shadowRoot.querySelector('#threads').textContent.trim(),
-          NO_THREADS_MSG
-      );
-    });
-
-    test('can override empty message', () => {
-      element.emptyThreadMsg = 'test';
-      assert.equal(
-          element.shadowRoot.querySelector('#threads').textContent.trim(),
-          'test'
-      );
+      assert.isTrue(
+          element.shadowRoot.querySelector('#threads').textContent.trim()
+              .includes('No comments.'));
     });
   });
 });