Merge "Add hovercard-accounts to avatar stacks."

4 files changed, 143 insertions, 13 deletions

diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack.ts
index 7040285801..8196201184 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack.ts
@@ -4,10 +4,12 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 import './gr-avatar';
+import '../gr-hovercard-account/gr-hovercard-account';
 import {AccountInfo, ServerInfo} from '../../../types/common';
-import {LitElement, css, html} from 'lit';
+import {LitElement, PropertyValues, css, html} from 'lit';
 import {customElement, property, state} from 'lit/decorators.js';
 import {
+  isDetailedAccount,
   uniqueAccountId,
   uniqueDefinedAvatar,
 } from '../../../utils/account-util';
@@ -15,6 +17,9 @@ import {resolve} from '../../../models/dependency';
 import {configModelToken} from '../../../models/config/config-model';
 import {subscribe} from '../../lit/subscription-controller';
 import {getDisplayName} from '../../../utils/display-name-util';
+import {accountsModelToken} from '../../../models/accounts-model/accounts-model';
+import {isDefined} from '../../../types/types';
+import {when} from 'lit/directives/when.js';
 
 /**
  * This elements draws stack of avatars overlapped with each other.
@@ -43,14 +48,22 @@ export class GrAvatarStack extends LitElement {
   imageSize = 16;
 
   /**
+   * Whether a hover-card should be shown for each avatar when hovered
+   */
+  @property({type: Boolean})
+  enableHover = false;
+
+  /**
    * In gr-app, gr-account-chip is in charge of loading a full account, so
    * avatars will be set. However, code-owners will create gr-avatars with a
    * bare account-id. To enable fetching of those avatars, a flag is added to
-   * gr-avatar that will disregard the absence of avatar urls.
+   * gr-avatar-stack that will fetch the accounts on demand
    */
   @property({type: Boolean})
   forceFetch = false;
 
+  private readonly getAccountsModel = resolve(this, accountsModelToken);
+
   @state() config?: ServerInfo;
 
   static override get styles() {
@@ -81,6 +94,24 @@ export class GrAvatarStack extends LitElement {
     );
   }
 
+  override updated(changedProperties: PropertyValues) {
+    if (changedProperties.has('accounts')) {
+      if (
+        this.forceFetch &&
+        this.accounts.length > 0 &&
+        this.accounts.some(a => !isDetailedAccount(a))
+      ) {
+        Promise.all(
+          this.accounts.map(account =>
+            this.getAccountsModel().fillDetails(account)
+          )
+        ).then(accounts => {
+          this.accounts = accounts.filter(isDefined);
+        });
+      }
+    }
+  }
+
   override render() {
     const uniqueAvatarAccounts = this.forceFetch
       ? this.accounts.filter(uniqueAccountId)
@@ -98,11 +129,17 @@ export class GrAvatarStack extends LitElement {
     return uniqueAvatarAccounts.map(
       account =>
         html`<gr-avatar
-          .forceFetch=${this.forceFetch}
           .account=${account}
           .imageSize=${this.imageSize}
           aria-label=${getDisplayName(this.config, account)}
         >
+          ${when(
+            this.enableHover,
+            () =>
+              html`<gr-hovercard-account
+                .account=${account}
+              ></gr-hovercard-account>`
+          )}
         </gr-avatar>`
     );
   }
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts
index 08066bf5c2..8c1c5c7597 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar-stack_test.ts
@@ -12,6 +12,7 @@ import {
 import {fixture, html, assert} from '@open-wc/testing';
 import {stubRestApi} from '../../../test/test-utils';
 import {LitElement} from 'lit';
+import {Timestamp} from '../../../api/rest-api';
 
 suite('gr-avatar tests', () => {
   suite('config with avatars', () => {
@@ -82,6 +83,104 @@ suite('gr-avatar tests', () => {
       }
     });
 
+    test('renders avatars and hovercards', async () => {
+      const accounts = [];
+      for (let i = 0; i < 2; ++i) {
+        accounts.push({
+          ...createAccountWithId(i),
+          avatars: [
+            {
+              url: `https://a.b.c/photo${i}.jpg`,
+              height: 32,
+              width: 32,
+            },
+          ],
+        });
+      }
+      accounts.push({
+        ...createAccountWithId(2),
+        avatars: [
+          {
+            // Account with duplicate avatar will be skipped.
+            url: 'https://a.b.c/photo1.jpg',
+            height: 32,
+            width: 32,
+          },
+        ],
+      });
+
+      const element: LitElement = await fixture(
+        html`<gr-avatar-stack
+          .accounts=${accounts}
+          .imageSize=${32}
+          .enableHover=${true}
+        ></gr-avatar-stack>`
+      );
+      await element.updateComplete;
+
+      assert.shadowDom.equal(
+        element,
+        /* HTML */ `<gr-avatar
+            aria-label="0"
+            style='background-image: url("https://a.b.c/photo0.jpg");'
+          >
+            <gr-hovercard-account></gr-hovercard-account>
+          </gr-avatar>
+          <gr-avatar
+            aria-label="1"
+            style='background-image: url("https://a.b.c/photo1.jpg");'
+          >
+            <gr-hovercard-account></gr-hovercard-account>
+          </gr-avatar> `
+      );
+      // Verify that margins are set correctly.
+      const avatars = element.shadowRoot!.querySelectorAll('gr-avatar');
+      assert.strictEqual(avatars.length, 2);
+      assert.strictEqual(window.getComputedStyle(avatars[0]).marginLeft, '0px');
+      for (let i = 1; i < avatars.length; ++i) {
+        assert.strictEqual(
+          window.getComputedStyle(avatars[i]).marginLeft,
+          '-8px'
+        );
+      }
+    });
+
+    test('fetches account details. avatars', async () => {
+      const stub = stubRestApi('getAccountDetails').resolves({
+        ...createAccountWithId(1),
+        avatars: [
+          {
+            url: 'https://a.b.c/photo0.jpg',
+            height: 32,
+            width: 32,
+          },
+        ],
+        registered_on: '1234' as Timestamp,
+      });
+      const element: LitElement = await fixture(
+        html`<gr-avatar-stack
+          .accounts=${[{_account_id: 1}]}
+          .forceFetch=${true}
+          .imageSize=${32}
+        ></gr-avatar-stack>`
+      );
+      await element.updateComplete;
+
+      assert.equal(stub.called, true);
+      assert.shadowDom.equal(
+        element,
+        /* HTML */ `<gr-avatar
+          aria-label="1"
+          style='background-image: url("https://a.b.c/photo0.jpg");'
+        >
+        </gr-avatar>`
+      );
+      // Verify that margins are set correctly.
+      const avatars = element.shadowRoot!.querySelectorAll('gr-avatar');
+      assert.strictEqual(avatars.length, 1);
+      assert.strictEqual(window.getComputedStyle(avatars[0]).marginLeft, '0px');
+    });
+
     test('renders many accounts fallback', async () => {
       const accounts = [];
       for (let i = 0; i < 5; ++i) {
diff --git a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
index 8cfe2d0291..1ea2a64306 100644
--- a/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
+++ b/polygerrit-ui/app/elements/shared/gr-avatar/gr-avatar.ts
@@ -25,14 +25,6 @@ export class GrAvatar extends LitElement {
 
   @state() private hasAvatars = false;
 
-  // In gr-app, gr-account-chip is in charge of loading a full account, so
-  // avatars will be set. However, code-owners will create gr-avatars with a
-  // bare account-id. To enable fetching of those avatars, a flag is added to
-  // gr-avatar that will disregard the absence of avatar urls.
-
-  @property({type: Boolean})
-  forceFetch = false;
-
   private readonly restApiService = getAppContext().restApiService;
 
   private readonly getPluginLoader = resolve(this, pluginLoaderToken);
@@ -98,7 +90,7 @@ export class GrAvatar extends LitElement {
     const avatars = account.avatars || [];
     // if there is no avatar url in account, there is no avatar set on server,
     // and request /avatar?s will be 404.
-    if (avatars.length === 0 && !this.forceFetch) {
+    if (avatars.length === 0) {
       return '';
     }
     for (let i = 0; i < avatars.length; i++) {
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 414548ded3..fc4123b20d 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -310,7 +310,9 @@ export class GrRestApiServiceImpl implements RestApiService, Finalizable {
 
   finalize() {}
 
-  _fetchSharedCacheURL(req: FetchJSONRequest): Promise<ParsedJSON | undefined> {
+  _fetchSharedCacheURL(
+    req: FetchJSONRequest
+  ): Promise<AccountDetailInfo | ParsedJSON | undefined> {
     // Cache is shared across instances
     return this._restApiHelper.fetchCacheURL(req);
   }