DEV: Minor cleanup of user-card code (#27436)

app/assets/javascripts/discourse/app/components/group-card-contents.hbs

@@ -16,8 +16,8 @@
       <div class="card-row first-row">
         <div class="group-card-avatar">
           <a
+            {{on "click" this.handleShowGroup}}
             href={{this.groupPath}}
-            {{on "click" (fn this.handleShowGroup this.group)}}
             class="card-huge-avatar"
           >
             <AvatarFlair
@@ -32,8 +32,8 @@
           <span>
             <h1 class={{this.group.name}}>
               <a
+                {{on "click" this.handleShowGroup}}
                 href={{this.groupPath}}
-                {{on "click" (fn this.handleShowGroup this.group)}}
                 class="group-page-link"
               >{{this.group.name}}</a>
             </h1>
@@ -84,8 +84,8 @@
             {{/each}}
             {{#if this.showMoreMembers}}
               <a
+                {{on "click" this.handleShowGroup}}
                 href={{this.groupPath}}
-                {{on "click" (fn this.handleShowGroup this.group)}}
                 class="more-members-link"
               >
                 <span class="more-members-count">+{{this.moreMembersCount}}

app/assets/javascripts/discourse/app/components/group-card-contents.js

@@ -2,7 +2,6 @@ import Component from "@ember/component";
 import { action } from "@ember/object";
 import { alias, gt } from "@ember/object/computed";
 import { service } from "@ember/service";
-import { Promise } from "rsvp";
 import { setting } from "discourse/lib/computed";
 import { wantsNewWindow } from "discourse/lib/intercept-click";
 import { groupPath } from "discourse/lib/url";
@@ -51,24 +50,26 @@ export default Component.extend(CardContentsBase, CleansUp, {
     return groupPath(group.name);
   },
 
-  _showCallback(username, $target) {
+  async _showCallback(username, $target) {
     this._positionCard($target);
     this.setProperties({ visible: true, loading: true });
 
-    return this.store
-      .find("group", username)
-      .then((group) => {
-        this.setProperties({ group });
-        if (!group.flair_url && !group.flair_bg_color) {
-          group.set("flair_url", "fa-users");
-        }
-        return group.can_see_members &&
-          group.members.length < maxMembersToDisplay
-          ? group.reloadMembers({ limit: maxMembersToDisplay }, true)
-          : Promise.resolve();
-      })
-      .catch(() => this._close())
-      .finally(() => this.set("loading", null));
+    try {
+      const group = await this.store.find("group", username);
+      this.setProperties({ group });
+
+      if (!group.flair_url && !group.flair_bg_color) {
+        group.set("flair_url", "fa-users");
+      }
+
+      if (group.can_see_members && group.members.length < maxMembersToDisplay) {
+        return group.reloadMembers({ limit: maxMembersToDisplay }, true);
+      }
+    } catch {
+      this._close();
+    } finally {
+      this.set("loading", null);
+    }
   },
 
   _close() {
@@ -88,7 +89,7 @@ export default Component.extend(CardContentsBase, CleansUp, {
   },
 
   @action
-  handleShowGroup(group, event) {
+  handleShowGroup(event) {
     if (wantsNewWindow(event)) {
       return;
     }
@@ -96,15 +97,14 @@ export default Component.extend(CardContentsBase, CleansUp, {
     event.preventDefault();
     // Invokes `showGroup` argument. Convert to `this.args.showGroup` when
     // refactoring this to a glimmer component.
-    this.showGroup(group);
+    this.showGroup(this.group);
     this._close();
   },
 
   actions: {
     cancelFilter() {
-      const postStream = this.postStream;
-      postStream.cancelFilter();
-      postStream.refresh();
+      this.postStream.cancelFilter();
+      this.postStream.refresh();
       this._close();
     },
 
@@ -114,9 +114,5 @@ export default Component.extend(CardContentsBase, CleansUp, {
         hasGroups: true,
       });
     },
-
-    showGroup(group) {
-      this.handleShowGroup(group);
-    },
   },
 });

app/assets/javascripts/discourse/app/components/user-card-contents.hbs

@@ -35,8 +35,8 @@
               }}</span>
           {{else}}
             <a
+              {{on "click" this.handleShowUser}}
               href={{this.user.path}}
-              {{on "click" (fn this.handleShowUser this.user)}}
               class="card-huge-avatar"
             >{{bound-avatar this.user "huge"}}</a>
           {{/if}}
@@ -70,8 +70,8 @@
               </span>
             {{else}}
               <a
+                {{on "click" this.handleShowUser}}
                 href={{this.user.path}}
-                {{on "click" (fn this.handleShowUser this.user)}}
                 class="user-profile-link"
               >
                 <span
@@ -91,10 +91,7 @@
           <PluginOutlet
             @name="user-card-after-username"
             @connectorTagName="div"
-            @outletArgs={{hash
-              user=this.user
-              showUser=(fn this.handleShowUser this.user)
-            }}
+            @outletArgs={{hash user=this.user showUser=this.handleShowUser}}
           />
           {{#if this.nameFirst}}
             <h2 class="username">{{this.user.username}}</h2>

app/assets/javascripts/discourse/app/components/user-card-contents.js

@@ -170,14 +170,13 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
       return;
     }
 
-    const thisElem = this.element;
-    if (!thisElem) {
+    if (!this.element) {
       return;
     }
 
     const url = this.get("user.card_background_upload_url");
     const bg = isEmpty(url) ? "" : `url(${getURLWithCDN(url)})`;
-    thisElem.style.backgroundImage = bg;
+    this.element.style.backgroundImage = bg;
   },
 
   @discourseComputed("user.primary_group_name")
@@ -190,7 +189,7 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
     return profileHidden || inactive;
   },
 
-  _showCallback(username, $target) {
+  async _showCallback(username, $target) {
     this._positionCard($target);
     this.setProperties({ visible: true, loading: true });
 
@@ -199,26 +198,28 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
       include_post_count_for: this.get("topic.id"),
     };
 
-    return User.findByUsername(username, args)
-      .then((user) => {
-        if (user.topic_post_count) {
-          this.set(
-            "topicPostCount",
-            user.topic_post_count[args.include_post_count_for]
-          );
-        }
-        this.setProperties({ user });
-        this.user.statusManager.trackStatus();
-        return user;
-      })
-      .catch(() => this._close())
-      .finally(() => this.set("loading", null));
+    try {
+      const user = await User.findByUsername(username, args);
+
+      if (user.topic_post_count) {
+        this.set(
+          "topicPostCount",
+          user.topic_post_count[args.include_post_count_for]
+        );
+      }
+      this.setProperties({ user });
+      this.user.statusManager.trackStatus();
+
+      return user;
+    } catch {
+      this._close();
+    } finally {
+      this.set("loading", null);
+    }
   },
 
   _close() {
-    if (this.user) {
-      this.user.statusManager.stopTrackingStatus();
-    }
+    this.user?.statusManager.stopTrackingStatus();
 
     this.setProperties({
       user: null,
@@ -233,7 +234,7 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
   },
 
   @action
-  handleShowUser(user, event) {
+  handleShowUser(event) {
     if (wantsNewWindow(event)) {
       return;
     }
@@ -241,7 +242,7 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
     event.preventDefault();
     // Invokes `showUser` argument. Convert to `this.args.showUser` when
     // refactoring this to a glimmer component.
-    this.showUser(user);
+    this.showUser(this.user);
     this._close();
   },
 
@@ -256,9 +257,8 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
     },
 
     cancelFilter() {
-      const postStream = this.postStream;
-      postStream.cancelFilter();
-      postStream.refresh();
+      this.postStream.cancelFilter();
+      this.postStream.refresh();
       this._close();
     },
 
@@ -272,10 +272,6 @@ export default Component.extend(CardContentsBase, CanCheckEmails, CleansUp, {
       this._close();
     },
 
-    showUser(user) {
-      this.handleShowUser(user);
-    },
-
     checkEmail(user) {
       user.checkEmail();
     },

app/assets/javascripts/discourse/app/mixins/card-contents-base.js

@@ -11,6 +11,8 @@ import discourseLater from "discourse-common/lib/later";
 import { bind } from "discourse-common/utils/decorators";
 
 const DEFAULT_SELECTOR = "#main-outlet";
+const AVATAR_OVERFLOW_SIZE = 44;
+const MOBILE_SCROLL_EVENT = "scroll.mobile-card-cloak";
 
 let _cardClickListenerSelectors = [DEFAULT_SELECTOR];
 
@@ -23,8 +25,12 @@ export function resetCardClickListenerSelector() {
 }
 
 export default Mixin.create({
-  router: service(),
+  appEvents: service(),
+  currentUser: service(),
   menu: service(),
+  router: service(),
+  site: service(),
+  siteSettings: service(),
 
   elementId: null, //click detection added for data-{elementId}
   triggeringLinkClass: null, //the <a> classname where this card should appear
@@ -100,14 +106,11 @@ export default Mixin.create({
     this._super(...arguments);
 
     const id = this.elementId;
-    const triggeringLinkClass = this.triggeringLinkClass;
-    const previewClickEvent = `click.discourse-preview-${id}-${triggeringLinkClass}`;
-    const mobileScrollEvent = "scroll.mobile-card-cloak";
+    const previewClickEvent = `click.discourse-preview-${id}-${this.triggeringLinkClass}`;
 
     this.setProperties({
       boundCardClickHandler: this._cardClickHandler,
       previewClickEvent,
-      mobileScrollEvent,
     });
 
     document.addEventListener("mousedown", this._clickOutsideHandler);
@@ -177,18 +180,15 @@ export default Mixin.create({
   },
 
   _bindMobileScroll() {
-    const mobileScrollEvent = this.mobileScrollEvent;
     const onScroll = () => {
       throttle(this, this._close, 1000);
     };
 
-    $(window).on(mobileScrollEvent, onScroll);
+    $(window).on(MOBILE_SCROLL_EVENT, onScroll);
   },
 
   _unbindMobileScroll() {
-    const mobileScrollEvent = this.mobileScrollEvent;
-
-    $(window).off(mobileScrollEvent);
+    $(window).off(MOBILE_SCROLL_EVENT);
   },
 
   _previewClick($target) {
@@ -201,14 +201,13 @@ export default Mixin.create({
         return;
       }
 
-      const avatarOverflowSize = 44;
       if (this.site.desktopView) {
         this._menuInstance = await this.menu.show(target[0], {
           content: this.element,
           autoUpdate: false,
           identifier: "card",
           padding: {
-            top: 10 + avatarOverflowSize + headerOffset(),
+            top: 10 + AVATAR_OVERFLOW_SIZE + headerOffset(),
             right: 10,
             bottom: 10,
             left: 10,
@@ -222,7 +221,7 @@ export default Mixin.create({
           computePosition: (content) => {
             content.style.left = "10px";
             content.style.right = "10px";
-            content.style.top = 10 + avatarOverflowSize + "px";
+            content.style.top = 10 + AVATAR_OVERFLOW_SIZE + "px";
           },
         });
       }
@@ -299,21 +298,18 @@ export default Mixin.create({
 
   @bind
   _clickOutsideHandler(event) {
-    if (this.visible) {
-      if (
-        event.target
-          .closest(`[data-${this.elementId}]`)
-          ?.getAttribute(`data-${this.elementId}`) ||
-        event.target.closest(`a.${this.triggeringLinkClass}`) ||
-        event.target.closest(`#${this.elementId}`)
-      ) {
-        return;
-      }
-
-      this._close();
+    if (
+      !this.visible ||
+      event.target
+        .closest(`[data-${this.elementId}]`)
+        ?.getAttribute(`data-${this.elementId}`) ||
+      event.target.closest(`a.${this.triggeringLinkClass}`) ||
+      event.target.closest(`#${this.elementId}`)
+    ) {
+      return;
     }
 
-    return true;
+    this._close();
   },
 
   @bind

app/assets/javascripts/discourse/tests/acceptance/user-card-test.js

@@ -2,11 +2,7 @@ import { getOwner } from "@ember/owner";
 import { click, visit } from "@ember/test-helpers";
 import { test } from "qunit";
 import userFixtures from "discourse/tests/fixtures/user-fixtures";
-import {
-  acceptance,
-  exists,
-  query,
-} from "discourse/tests/helpers/qunit-helpers";
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
 import { cloneJSON } from "discourse-common/lib/object";
 import I18n from "discourse-i18n";
 
@@ -21,10 +17,11 @@ acceptance("User Card - Show Local Time", function (needs) {
     await visit("/t/internationalization-localization/280");
     await click('a[data-user-card="charlie"]');
 
-    assert.notOk(
-      exists(".user-card .local-time"),
-      "it does not show the local time if the user card returns a null/undefined timezone for another user"
-    );
+    assert
+      .dom(".user-card .local-time")
+      .doesNotExist(
+        "it does not show the local time if the user card returns a null/undefined timezone for another user"
+      );
   });
 });
 
@@ -42,11 +39,10 @@ acceptance(
       await visit("/t/this-is-a-test-topic/9");
       await click('a[data-user-card="eviltrout"]');
 
-      assert.equal(
-        query(".user-card h1.username .name-username-wrapper").innerText,
-        "eviltrout"
-      );
-      assert.equal(query(".user-card h2.full-name").innerText, "Robin Ward");
+      assert
+        .dom(".user-card h1.username .name-username-wrapper")
+        .hasText("eviltrout");
+      assert.dom(".user-card h2.full-name").hasText("Robin Ward");
     });
   }
 );
@@ -65,11 +61,10 @@ acceptance(
       await visit("/t/this-is-a-test-topic/9");
       await click('a[data-user-card="eviltrout"]');
 
-      assert.equal(
-        query(".user-card h1.full-name .name-username-wrapper").innerText,
-        "Robin Ward"
-      );
-      assert.equal(query(".user-card h2.username").innerText, "eviltrout");
+      assert
+        .dom(".user-card h1.full-name .name-username-wrapper")
+        .hasText("Robin Ward");
+      assert.dom(".user-card h2.username").hasText("eviltrout");
     });
   }
 );
@@ -88,7 +83,7 @@ acceptance("User Card - User Status", function (needs) {
     await visit("/t/internationalization-localization/280");
     await click('a[data-user-card="charlie"]');
 
-    assert.ok(exists(".user-card h3.user-status"));
+    assert.dom(".user-card h3.user-status").exists();
   });
 
   test("doesn't show user status if disabled", async function (assert) {
@@ -97,7 +92,7 @@ acceptance("User Card - User Status", function (needs) {
     await visit("/t/internationalization-localization/280");
     await click('a[data-user-card="charlie"]');
 
-    assert.notOk(exists(".user-card h3.user-status"));
+    assert.dom(".user-card h3.user-status").doesNotExist();
   });
 });
 
@@ -123,14 +118,10 @@ acceptance("User Card - Hidden Profile", function (needs) {
     await visit("/t/this-is-a-test-topic/9");
     await click('a[data-user-card="eviltrout"]');
 
-    assert.equal(
-      query(".user-card .name-username-wrapper").innerText,
-      "eviltrout"
-    );
-    assert.equal(
-      query(".user-card .profile-hidden").innerText,
-      I18n.t("user.profile_hidden")
-    );
+    assert.dom(".user-card .name-username-wrapper").hasText("eviltrout");
+    assert
+      .dom(".user-card .profile-hidden")
+      .hasText(I18n.t("user.profile_hidden"));
   });
 });
 
@@ -154,14 +145,9 @@ acceptance("User Card - Inactive user", function (needs) {
     await visit("/t/this-is-a-test-topic/9");
     await click('a[data-user-card="eviltrout"]');
 
-    assert.equal(
-      query(".user-card .name-username-wrapper").innerText,
-      "eviltrout"
-    );
-
-    assert.equal(
-      query(".user-card .inactive-user").innerText,
-      I18n.t("user.inactive_user")
-    );
+    assert.dom(".user-card .name-username-wrapper").hasText("eviltrout");
+    assert
+      .dom(".user-card .inactive-user")
+      .hasText(I18n.t("user.inactive_user"));
   });
 });