commit cbee28a79efc1b88eed18dcb0ee499149d074d14
Author: Ben Rohlfs <brohlfs@google.com>
Date:   Wed Sep 30 10:37:54 2020 +0200

    Add comment icon to CR column of the dashboard
    
    Users would really like information about unresolved comments on the
    dashboard, but a dedicated column would take too much space. So let's
    just show a comment icon in the CR column, and don't even show the
    number of unresolved comments, just on hover.
    
    Also replaces the checkmark and x characters by icons.
    
    Screenshots: https://imgur.com/a/vcprgU9
    Change-Id: Ib1b8584190b967b00ef3294b2c0daaa4a7a035b9

diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index 5d898bd..dec8cd8 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -49,13 +49,24 @@ import {
 } from '../../../types/common';
 import {hasOwnProperty} from '../../../utils/common-util';
 
-enum CHANGE_SIZE {
+enum ChangeSize {
   XS = 10,
   SMALL = 50,
   MEDIUM = 250,
   LARGE = 1000,
 }
 
+// export for testing
+export enum LabelCategory {
+  NOT_APPLICABLE = 'NOT_APPLICABLE',
+  APPROVED = 'APPROVED',
+  POSITIVE = 'POSITIVE',
+  NEUTRAL = 'NEUTRAL',
+  UNRESOLVED_COMMENTS = 'UNRESOLVED_COMMENTS',
+  NEGATIVE = 'NEGATIVE',
+  REJECTED = 'REJECTED',
+}
+
 // How many reviewers should be shown with an account-label?
 const PRIMARY_REVIEWERS_COUNT = 2;
 
@@ -131,9 +142,15 @@ class GrChangeListItem extends ChangeTableMixin(
 
   _computeLabelTitle(change: ChangeInfo | undefined, labelName: string) {
     const label: QuickLabelInfo | undefined = change?.labels?.[labelName];
-    if (!label) {
+    const category = this._computeLabelCategory(change, labelName);
+    if (!label || category === LabelCategory.NOT_APPLICABLE) {
       return 'Label not applicable';
     }
+    if (category === LabelCategory.UNRESOLVED_COMMENTS) {
+      const num = change?.unresolved_comment_count ?? 0;
+      const plural = num > 1 ? 's' : '';
+      return `${num} unresolved comment${plural}`;
+    }
     const significantLabel =
       label.rejected || label.approved || label.disliked || label.recommended;
     if (significantLabel && significantLabel.name) {
@@ -143,58 +160,90 @@ class GrChangeListItem extends ChangeTableMixin(
   }
 
   _computeLabelClass(change: ChangeInfo | undefined, labelName: string) {
-    const label: QuickLabelInfo | undefined = change?.labels?.[labelName];
-    // Mimic a Set.
-    // TODO(TS): replace with `u_green` to remove the quotes and brackets
-    const classes: {
-      cell: boolean;
-      label: boolean;
-      ['u-green']?: boolean;
-      ['u-monospace']?: boolean;
-      ['u-red']?: boolean;
-      ['u-gray-background']?: boolean;
-    } = {
-      cell: true,
-      label: true,
-    };
-    if (label) {
-      if (label.approved) {
-        classes['u-green'] = true;
-      }
-      if (label.value === 1) {
-        classes['u-monospace'] = true;
-        classes['u-green'] = true;
-      } else if (label.value === -1) {
-        classes['u-monospace'] = true;
-        classes['u-red'] = true;
-      }
-      if (label.rejected) {
-        classes['u-red'] = true;
-      }
-    } else {
-      classes['u-gray-background'] = true;
+    const category = this._computeLabelCategory(change, labelName);
+    const classes = ['cell', 'label'];
+    switch (category) {
+      case LabelCategory.NOT_APPLICABLE:
+        classes.push('u-gray-background');
+        break;
+      case LabelCategory.APPROVED:
+        classes.push('u-green');
+        break;
+      case LabelCategory.POSITIVE:
+        classes.push('u-monospace');
+        classes.push('u-green');
+        break;
+      case LabelCategory.NEGATIVE:
+        classes.push('u-monospace');
+        classes.push('u-red');
+        break;
+      case LabelCategory.REJECTED:
+        classes.push('u-red');
+        break;
     }
-    return Object.keys(classes).sort().join(' ');
+    return classes.sort().join(' ');
   }
 
-  _computeLabelValue(change: ChangeInfo | undefined, labelName: string) {
+  _computeHasLabelIcon(change: ChangeInfo | undefined, labelName: string) {
+    return this._computeLabelIcon(change, labelName) !== '';
+  }
+
+  _computeLabelIcon(change: ChangeInfo | undefined, labelName: string): string {
+    const category = this._computeLabelCategory(change, labelName);
+    switch (category) {
+      case LabelCategory.APPROVED:
+        return 'gr-icons:check';
+      case LabelCategory.UNRESOLVED_COMMENTS:
+        return 'gr-icons:comment';
+      case LabelCategory.REJECTED:
+        return 'gr-icons:close';
+      default:
+        return '';
+    }
+  }
+
+  _computeLabelCategory(change: ChangeInfo | undefined, labelName: string) {
     const label: QuickLabelInfo | undefined = change?.labels?.[labelName];
     if (!label) {
-      return '';
-    }
-    if (label.approved) {
-      return '✓';
+      return LabelCategory.NOT_APPLICABLE;
     }
     if (label.rejected) {
-      return '✕';
+      return LabelCategory.REJECTED;
+    }
+    if (label.value && label.value < 0) {
+      return LabelCategory.NEGATIVE;
+    }
+    if (change?.unresolved_comment_count && labelName === 'Code-Review') {
+      return LabelCategory.UNRESOLVED_COMMENTS;
+    }
+    if (label.approved) {
+      return LabelCategory.APPROVED;
     }
     if (label.value && label.value > 0) {
-      return `+${label.value}`;
+      return LabelCategory.POSITIVE;
     }
-    if (label.value && label.value < 0) {
-      return label.value;
+    return LabelCategory.NEUTRAL;
+  }
+
+  _computeLabelValue(change: ChangeInfo | undefined, labelName: string) {
+    const label: QuickLabelInfo | undefined = change?.labels?.[labelName];
+    const category = this._computeLabelCategory(change, labelName);
+    switch (category) {
+      case LabelCategory.NOT_APPLICABLE:
+        return '';
+      case LabelCategory.APPROVED:
+        return '\u2713'; // ✓
+      case LabelCategory.POSITIVE:
+        return `+${label?.value}`;
+      case LabelCategory.NEUTRAL:
+        return '';
+      case LabelCategory.UNRESOLVED_COMMENTS:
+        return 'u';
+      case LabelCategory.NEGATIVE:
+        return `${label?.value}`;
+      case LabelCategory.REJECTED:
+        return '\u2715'; // ✕
     }
-    return '';
   }
 
   _computeRepoUrl(change?: ChangeInfo) {
@@ -319,13 +368,13 @@ class GrChangeListItem extends ChangeTableMixin(
     if (isNaN(delta) || delta === 0) {
       return null; // Unknown
     }
-    if (delta < CHANGE_SIZE.XS) {
+    if (delta < ChangeSize.XS) {
       return 'XS';
-    } else if (delta < CHANGE_SIZE.SMALL) {
+    } else if (delta < ChangeSize.SMALL) {
       return 'S';
-    } else if (delta < CHANGE_SIZE.MEDIUM) {
+    } else if (delta < ChangeSize.MEDIUM) {
       return 'M';
-    } else if (delta < CHANGE_SIZE.LARGE) {
+    } else if (delta < ChangeSize.LARGE) {
       return 'L';
     } else {
       return 'XL';
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
index e0c9ac3..5316fe5 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_html.ts
@@ -92,10 +92,12 @@ export const htmlTemplate = html`
       font-size: var(--font-size-mono);
       line-height: var(--line-height-mono);
     }
-    .u-green {
+    .u-green,
+    .u-green iron-icon {
       color: var(--positive-green-text-color);
     }
-    .u-red {
+    .u-red,
+    .u-red iron-icon {
       color: var(--negative-red-text-color);
     }
     .u-gray-background {
@@ -108,6 +110,9 @@ export const htmlTemplate = html`
     .cell.label {
       font-weight: var(--font-weight-normal);
     }
+    .cell.label iron-icon {
+      vertical-align: top;
+    }
     .lastChildHidden:last-of-type {
       display: none;
     }
@@ -278,7 +283,12 @@ export const htmlTemplate = html`
       title$="[[_computeLabelTitle(change, labelName)]]"
       class$="[[_computeLabelClass(change, labelName)]]"
     >
-      [[_computeLabelValue(change, labelName)]]
+      <template is="dom-if" if="[[_computeHasLabelIcon(change, labelName)]]">
+        <iron-icon icon="[[_computeLabelIcon(change, labelName)]]"></iron-icon>
+      </template>
+      <template is="dom-if" if="[[!_computeHasLabelIcon(change, labelName)]]">
+        <span>[[_computeLabelValue(change, labelName)]]</span>
+      </template>
     </td>
   </template>
   <template
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
index 3dfdb60..d3274f3 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.js
@@ -18,6 +18,7 @@
 import '../../../test/common-test-setup-karma.js';
 import './gr-change-list-item.js';
 import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
+import {LabelCategory} from './gr-change-list-item.js';
 
 const basicFixture = fixtureFromElement('gr-change-list-item');
 
@@ -32,17 +33,45 @@ suite('gr-change-list-item tests', () => {
     element = basicFixture.instantiate();
   });
 
-  test('computed fields', () => {
+  test('_computeLabelCategory', () => {
+    assert.equal(element._computeLabelCategory({labels: {}}),
+        LabelCategory.NOT_APPLICABLE);
+    assert.equal(element._computeLabelCategory(
+        {labels: {}}, 'Verified'), LabelCategory.NOT_APPLICABLE);
+    assert.equal(element._computeLabelCategory(
+        {labels: {Verified: {approved: true, value: 1}}}, 'Verified'),
+    LabelCategory.APPROVED);
+    assert.equal(element._computeLabelCategory(
+        {labels: {Verified: {rejected: true, value: -1}}}, 'Verified'),
+    LabelCategory.REJECTED);
+    assert.equal(element._computeLabelCategory(
+        {
+          labels: {'Code-Review': {approved: true, value: 1}},
+          unresolved_comment_count: 1,
+        }, 'Code-Review'),
+    LabelCategory.UNRESOLVED_COMMENTS);
+    assert.equal(element._computeLabelCategory(
+        {labels: {'Code-Review': {value: 1}}}, 'Code-Review'),
+    LabelCategory.POSITIVE);
+    assert.equal(element._computeLabelCategory(
+        {labels: {'Code-Review': {value: -1}}}, 'Code-Review'),
+    LabelCategory.NEGATIVE);
+    assert.equal(element._computeLabelCategory(
+        {labels: {'Code-Review': {value: -1}}}, 'Verified'),
+    LabelCategory.NOT_APPLICABLE);
+  });
+
+  test('_computeLabelClass', () => {
     assert.equal(element._computeLabelClass({labels: {}}),
         'cell label u-gray-background');
     assert.equal(element._computeLabelClass(
         {labels: {}}, 'Verified'), 'cell label u-gray-background');
     assert.equal(element._computeLabelClass(
         {labels: {Verified: {approved: true, value: 1}}}, 'Verified'),
-    'cell label u-green u-monospace');
+    'cell label u-green');
     assert.equal(element._computeLabelClass(
         {labels: {Verified: {rejected: true, value: -1}}}, 'Verified'),
-    'cell label u-monospace u-red');
+    'cell label u-red');
     assert.equal(element._computeLabelClass(
         {labels: {'Code-Review': {value: 1}}}, 'Code-Review'),
     'cell label u-green u-monospace');
@@ -52,7 +81,9 @@ suite('gr-change-list-item tests', () => {
     assert.equal(element._computeLabelClass(
         {labels: {'Code-Review': {value: -1}}}, 'Verified'),
     'cell label u-gray-background');
+  });
 
+  test('_computeLabelTitle', () => {
     assert.equal(element._computeLabelTitle({labels: {}}, 'Verified'),
         'Label not applicable');
     assert.equal(element._computeLabelTitle(
@@ -86,7 +117,34 @@ suite('gr-change-list-item tests', () => {
         {labels: {'Code-Review': {approved: {name: 'Diffy'},
           disliked: {name: 'Admin'}, value: -1}}}, 'Code-Review'),
     'Code-Review\nby Diffy');
+    assert.equal(element._computeLabelTitle(
+        {
+          labels: {'Code-Review': {approved: true, value: 1}},
+          unresolved_comment_count: 1,
+        }, 'Code-Review'),
+    '1 unresolved comment');
+    assert.equal(element._computeLabelTitle(
+        {
+          labels: {'Code-Review': {approved: true, value: 1}},
+          unresolved_comment_count: 2,
+        }, 'Code-Review'),
+    '2 unresolved comments');
+  });
+
+  test('_computeLabelIcon', () => {
+    assert.equal(element._computeLabelIcon({labels: {}}), '');
+    assert.equal(element._computeLabelIcon(
+        {labels: {Verified: {approved: true, value: 1}}}, 'Verified'),
+    'gr-icons:check');
+    assert.equal(element._computeLabelIcon(
+        {
+          labels: {'Code-Review': {approved: true, value: 1}},
+          unresolved_comment_count: 1,
+        }, 'Code-Review'),
+    'gr-icons:comment');
+  });
 
+  test('_computeLabelValue', () => {
     assert.equal(element._computeLabelValue({labels: {}}), '');
     assert.equal(element._computeLabelValue({labels: {}}, 'Verified'), '');
     assert.equal(element._computeLabelValue(