UX: makes avatar non interactive in thread participants list (#23847)

It was slightly surprising to have a user card show when click on a thread item list.

More over this commit does:
- moves chat/user-avatar to chat-user-avatar and converts it to gjs
- moves chat/thread/participants to chat-thread-participants
- rewrite the `toggleCheckIfPossible` modifier to only be applied when selecting messages, it prevents the click event to collide with the click of avatars in regular messages

plugins/chat/assets/javascripts/discourse/components/chat-channel-title.hbs

@@ -7,7 +7,7 @@
             {{@channel.chatable.users.length}}
           </span>
         {{else}}
-          <Chat::UserAvatar @user={{get @channel.chatable.users "0"}} />
+          <ChatUserAvatar @user={{get @channel.chatable.users "0"}} />
         {{/if}}
       </div>
     {{/if}}

plugins/chat/assets/javascripts/discourse/components/chat-composer-message-details.hbs

@@ -5,7 +5,7 @@
 >
   <div class="chat-reply">
     {{d-icon (if @message.editing "pencil-alt" "reply")}}
-    <Chat::UserAvatar @user={{@message.user}} />
+    <ChatUserAvatar @user={{@message.user}} />
     <span class="chat-reply__username">{{@message.user.username}}</span>
     <span class="chat-reply__excerpt">
       {{replace-emoji (html-safe @message.excerpt)}}

plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.gjs

@@ -10,7 +10,7 @@ import didInsert from "@ember/render-modifiers/modifiers/did-insert";
 import concatClass from "discourse/helpers/concat-class";
 import DButton from "discourse/components/d-button";
 import { on } from "@ember/modifier";
-import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat/user-avatar";
+import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat-user-avatar";
 import ChatMessageReaction from "discourse/plugins/chat/discourse/components/chat-message-reaction";
 import { fn } from "@ember/helper";
 import or from "truth-helpers/helpers/or";

plugins/chat/assets/javascripts/discourse/components/chat-message-in-reply-to-indicator.hbs

@@ -9,7 +9,7 @@
     {{#if @message.inReplyTo.chatWebhookEvent.emoji}}
       <ChatEmojiAvatar @emoji={{@message.inReplyTo.chatWebhookEvent.emoji}} />
     {{else}}
-      <Chat::UserAvatar @user={{@message.inReplyTo.user}} />
+      <ChatUserAvatar @user={{@message.inReplyTo.user}} />
     {{/if}}
 
     <span class="chat-reply__excerpt">

plugins/chat/assets/javascripts/discourse/components/chat-message-thread-indicator.hbs

@@ -10,7 +10,7 @@
 >
 
   <div class="chat-message-thread-indicator__last-reply-avatar">
-    <Chat::UserAvatar
+    <ChatUserAvatar
       @user={{@message.thread.preview.lastReplyUser}}
       @avatarSize="small"
     />
@@ -27,7 +27,7 @@
   <div class="chat-message-thread-indicator__replies-count">
     {{i18n "chat.thread.replies" count=@message.thread.preview.replyCount}}
   </div>
-  <Chat::Thread::Participants @thread={{@message.thread}} />
+  <ChatThreadParticipants @thread={{@message.thread}} />
   <div class="chat-message-thread-indicator__last-reply-excerpt">
     {{replace-emoji (html-safe @message.thread.preview.lastReplyExcerpt)}}
   </div>

plugins/chat/assets/javascripts/discourse/components/chat-message.gjs

@@ -32,6 +32,7 @@ import didInsert from "@ember/render-modifiers/modifiers/did-insert";
 import didUpdate from "@ember/render-modifiers/modifiers/did-update";
 import willDestroy from "@ember/render-modifiers/modifiers/will-destroy";
 import ChatOnLongPress from "discourse/plugins/chat/discourse/modifiers/chat/on-long-press";
+import { modifier } from "ember-modifier";
 
 let _chatMessageDecorators = [];
 
@@ -84,7 +85,7 @@ export default class ChatMessage extends Component {
         {{on "mouseenter" this.onMouseEnter passive=true}}
         {{on "mouseleave" this.onMouseLeave passive=true}}
         {{on "mousemove" this.onMouseMove passive=true}}
-        {{on "click" this.toggleCheckIfPossible passive=true}}
+        {{this.toggleCheckIfPossible}}
         {{ChatOnLongPress
           this.onLongPressStart
           this.onLongPressEnd
@@ -200,6 +201,34 @@ export default class ChatMessage extends Component {
 
   @optionalService adminTools;
 
+  toggleCheckIfPossible = modifier((element) => {
+    let addedListener = false;
+
+    const handler = () => {
+      if (!this.pane.selectingMessages) {
+        return;
+      }
+
+      if (event.shiftKey) {
+        this.messageInteractor.bulkSelect(!this.args.message.selected);
+        return;
+      }
+
+      this.messageInteractor.select(!this.args.message.selected);
+    };
+
+    if (this.pane.selectingMessages) {
+      element.addEventListener("click", handler, { passive: true });
+      addedListener = true;
+    }
+
+    return () => {
+      if (addedListener) {
+        element.removeEventListener("click", handler);
+      }
+    };
+  });
+
   constructor() {
     super(...arguments);
     this.initMentionedUsers();
@@ -280,20 +309,6 @@ export default class ChatMessage extends Component {
     recursiveExpand(this.args.message);
   }
 
-  @action
-  toggleCheckIfPossible(event) {
-    if (!this.pane.selectingMessages) {
-      return;
-    }
-
-    if (event.shiftKey) {
-      this.messageInteractor.bulkSelect(!this.args.message.selected);
-      return;
-    }
-
-    this.messageInteractor.select(!this.args.message.selected);
-  }
-
   @action
   toggleChecked(event) {
     event.stopPropagation();

plugins/chat/assets/javascripts/discourse/components/chat-thread-participants.gjs

@@ -1,5 +1,5 @@
 import Component from "@glimmer/component";
-import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat/user-avatar";
+import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat-user-avatar";
 import I18n from "I18n";
 
 export default class ChatThreadParticipants extends Component {
@@ -12,6 +12,7 @@ export default class ChatThreadParticipants extends Component {
               @user={{user}}
               @avatarSize="tiny"
               @showPresence={{false}}
+              @interactive={{false}}
             />
           {{/each}}
         </div>

plugins/chat/assets/javascripts/discourse/components/chat-user-avatar.gjs

@@ -0,0 +1,58 @@
+import Component from "@glimmer/component";
+import { inject as service } from "@ember/service";
+import { renderAvatar } from "discourse/helpers/user-avatar";
+import { htmlSafe } from "@ember/template";
+import concatClass from "discourse/helpers/concat-class";
+
+export default class ChatUserAvatar extends Component {
+  <template>
+    <div
+      class={{concatClass "chat-user-avatar" (if this.isOnline "is-online")}}
+      data-username={{@user.username}}
+    >
+      {{#if this.interactive}}
+        <div
+          role="button"
+          class="chat-user-avatar__container clickable"
+          data-user-card={{@user.username}}
+        >
+          {{this.avatar}}
+        </div>
+      {{else}}
+        {{this.avatar}}
+      {{/if}}
+    </div>
+  </template>
+
+  @service chat;
+
+  get avatar() {
+    return htmlSafe(
+      renderAvatar(this.args.user, { imageSize: this.avatarSize })
+    );
+  }
+
+  get interactive() {
+    return this.args.interactive ?? true;
+  }
+
+  get avatarSize() {
+    return this.args.avatarSize || "tiny";
+  }
+
+  get showPresence() {
+    return this.args.showPresence ?? true;
+  }
+
+  get isOnline() {
+    const users = (this.args.chat || this.chat).presenceChannel?.users;
+
+    return (
+      this.showPresence &&
+      !!users?.find(
+        ({ id, username }) =>
+          this.args.user?.id === id || this.args.user?.username === username
+      )
+    );
+  }
+}

plugins/chat/assets/javascripts/discourse/components/chat-user-info.gjs

@@ -1,6 +1,6 @@
 import Component from "@glimmer/component";
 import { userPath } from "discourse/lib/url";
-import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat/user-avatar";
+import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat-user-avatar";
 import ChatUserDisplayName from "discourse/plugins/chat/discourse/components/chat-user-display-name";
 
 export default class ChatUserInfo extends Component {

plugins/chat/assets/javascripts/discourse/components/chat/message-creator/user-row.hbs

@@ -1,4 +1,4 @@
-<Chat::UserAvatar @user={{@content.model}} />
+<ChatUserAvatar @user={{@content.model}} />
 <ChatUserDisplayName @user={{@content.model}} />
 
 {{#if (gt @content.tracking.unreadCount 0)}}

plugins/chat/assets/javascripts/discourse/components/chat/message-creator/user-selection.hbs

@@ -1,4 +1,4 @@
-<Chat::UserAvatar @user={{@selection.model}} @showPresence={{false}} />
+<ChatUserAvatar @user={{@selection.model}} @showPresence={{false}} />
 
 <span class="chat-message-creator__selection-item__username">
   {{@selection.model.username}}

plugins/chat/assets/javascripts/discourse/components/chat/message/avatar.hbs

@@ -2,6 +2,6 @@
   {{#if @message.chatWebhookEvent.emoji}}
     <ChatEmojiAvatar @emoji={{@message.chatWebhookEvent.emoji}} />
   {{else}}
-    <Chat::UserAvatar @user={{@message.user}} @avatarSize="medium" />
+    <ChatUserAvatar @user={{@message.user}} @avatarSize="medium" />
   {{/if}}
 </div>

plugins/chat/assets/javascripts/discourse/components/chat/thread-list/item.hbs

@@ -29,11 +29,12 @@
       <div class="chat-thread-list-item__metadata">
 
         <div class="chat-thread-list-item__members">
-          <Chat::UserAvatar
+          <ChatUserAvatar
             @user={{@thread.originalMessage.user}}
             @showPresence={{false}}
+            @interactive={{false}}
           />
-          <Chat::Thread::Participants
+          <ChatThreadParticipants
             @thread={{@thread}}
             @includeOriginalMessageUser={{false}}
             class="chat-thread-list-item__participants"

plugins/chat/assets/javascripts/discourse/components/chat/user-avatar.hbs

@@ -1,9 +0,0 @@
-<div class="chat-user-avatar {{if this.isOnline 'is-online'}}">
-  <div
-    role="button"
-    class="chat-user-avatar__container clickable"
-    data-user-card={{@user.username}}
-  >
-    {{avatar @user imageSize=this.avatarSize}}
-  </div>
-</div>

plugins/chat/assets/javascripts/discourse/components/chat/user-avatar.js

@@ -1,26 +0,0 @@
-import Component from "@glimmer/component";
-import { inject as service } from "@ember/service";
-
-export default class ChatUserAvatar extends Component {
-  @service chat;
-
-  get avatarSize() {
-    return this.args.avatarSize || "tiny";
-  }
-
-  get showPresence() {
-    return this.args.showPresence ?? true;
-  }
-
-  get isOnline() {
-    const users = (this.args.chat || this.chat).presenceChannel?.users;
-
-    return (
-      this.showPresence &&
-      !!users?.find(
-        ({ id, username }) =>
-          this.args.user?.id === id || this.args.user?.username === username
-      )
-    );
-  }
-}

plugins/chat/assets/stylesheets/common/chat-thread-list-item.scss

@@ -75,6 +75,8 @@
     align-items: center;
     gap: 0.5rem;
     .chat-user-avatar {
+      cursor: pointer;
+
       &__container {
         padding: 0;
       }

plugins/chat/spec/fabricators/chat_fabricator.rb

@@ -63,7 +63,7 @@ end
 Fabricator(:chat_message_without_service, class_name: "Chat::Message") do
   user
   chat_channel
-  message { Faker::Lorem.paragraph_by_chars(number: 500).gsub("...", "…") }
+  message { Faker::Lorem.paragraph_by_chars(number: 500).gsub("...", "…").gsub("..", "…") }
 
   after_build { |message, attrs| message.cook }
   after_create { |message, attrs| message.create_mentions }
@@ -90,7 +90,8 @@ Fabricator(:chat_message_with_service, class_name: "Chat::CreateMessage") do
       chat_channel_id: channel.id,
       guardian: user.guardian,
       message:
-        transients[:message] || Faker::Lorem.paragraph_by_chars(number: 500).gsub("...", "…"),
+        transients[:message] ||
+          Faker::Lorem.paragraph_by_chars(number: 500).gsub("...", "…").gsub("..", "…"),
       thread_id: transients[:thread]&.id,
       in_reply_to_id: transients[:in_reply_to]&.id,
       upload_ids: transients[:upload_ids],

plugins/chat/spec/system/page_objects/chat/components/thread_indicator.rb

@@ -33,7 +33,7 @@ module PageObjects
 
         def has_participant?(user)
           find(@context).has_css?(
-            ".chat-thread-participants__avatar-group .chat-user-avatar .chat-user-avatar__container[data-user-card=\"#{user.username}\"] img",
+            ".chat-thread-participants__avatar-group .chat-user-avatar[data-username=\"#{user.username}\"] img",
           )
         end
 

plugins/chat/test/javascripts/components/chat-thread-participants-test.js

@@ -5,18 +5,18 @@ import { render } from "@ember/test-helpers";
 import { module, test } from "qunit";
 
 module(
-  "Discourse Chat | Component | <Chat::Thread::Participants />",
+  "Discourse Chat | Component | <ChatThreadParticipants />",
   function (hooks) {
     setupRenderingTest(hooks);
 
     test("no participants", async function (assert) {
       this.thread = fabricators.thread();
-      await render(hbs`<Chat::Thread::Participants @thread={{this.thread}} />`);
+      await render(hbs`<ChatThreadParticipants @thread={{this.thread}} />`);
 
       assert.dom(".chat-thread-participants").doesNotExist();
     });
 
-    test("includeOriginalMessageUser=true", async function (assert) {
+    test("@includeOriginalMessageUser=true", async function (assert) {
       const orignalMessageUser = fabricators.user({ username: "bob" });
       this.thread = fabricators.thread({
         original_message: fabricators.message({ user: orignalMessageUser }),
@@ -29,13 +29,12 @@ module(
         }),
       });
 
-      await render(hbs`<Chat::Thread::Participants @thread={{this.thread}} />`);
+      await render(hbs`<ChatThreadParticipants @thread={{this.thread}} />`);
 
-      assert.dom('.chat-user-avatar [data-user-card="bob"]').exists();
+      assert.dom(".chat-user-avatar[data-username]").exists({ count: 2 });
-      assert.dom('.chat-user-avatar [data-user-card="alice"]').exists();
     });
 
-    test("includeOriginalMessageUser=false", async function (assert) {
+    test("@includeOriginalMessageUser=false", async function (assert) {
       const orignalMessageUser = fabricators.user({ username: "bob" });
       this.thread = fabricators.thread({
         original_message: fabricators.message({ user: orignalMessageUser }),
@@ -49,11 +48,11 @@ module(
       });
 
       await render(
-        hbs`<Chat::Thread::Participants @thread={{this.thread}} @includeOriginalMessageUser={{false}} />`
+        hbs`<ChatThreadParticipants @thread={{this.thread}} @includeOriginalMessageUser={{false}} />`
       );
 
-      assert.dom('.chat-user-avatar [data-user-card="bob"]').doesNotExist();
+      assert.dom('.chat-user-avatar[data-username="bob"]').doesNotExist();
-      assert.dom('.chat-user-avatar [data-user-card="alice"]').exists();
+      assert.dom('.chat-user-avatar[data-username="alice"]').exists();
     });
   }
 );

plugins/chat/test/javascripts/components/chat-user-avatar-test.js

@@ -13,15 +13,15 @@ function containerSelector(user, options = {}) {
   return `.chat-user-avatar${onlineSelector} .chat-user-avatar__container[data-user-card=${user.username}] .avatar[title=${user.username}]`;
 }
 
-module("Discourse Chat | Component | <Chat::UserAvatar />", function (hooks) {
+module("Discourse Chat | Component | <ChatUserAvatar />", function (hooks) {
   setupRenderingTest(hooks);
 
-  test("user is not online", async function (assert) {
+  test("when user is not online", async function (assert) {
     this.user = fabricators.user();
     this.chat = { presenceChannel: { users: [] } };
 
     await render(
-      hbs`<Chat::UserAvatar @chat={{this.chat}} @user={{this.user}} />`
+      hbs`<ChatUserAvatar @chat={{this.chat}} @user={{this.user}} />`
     );
 
     assert.dom(containerSelector(this.user, { online: false })).exists();
@@ -34,22 +34,32 @@ module("Discourse Chat | Component | <Chat::UserAvatar />", function (hooks) {
     };
 
     await render(
-      hbs`<Chat::UserAvatar @chat={{this.chat}} @user={{this.user}} />`
+      hbs`<ChatUserAvatar @chat={{this.chat}} @user={{this.user}} />`
     );
 
     assert.dom(containerSelector(this.user, { online: true })).exists();
   });
 
-  test("showPresence=false", async function (assert) {
+  test("@showPresence=false", async function (assert) {
     this.user = fabricators.user();
     this.chat = {
       presenceChannel: { users: [{ id: this.user.id }] },
     };
 
     await render(
-      hbs`<Chat::UserAvatar @showPresence={{false}} @chat={{this.chat}} @user={{this.user}} />`
+      hbs`<ChatUserAvatar @showPresence={{false}} @chat={{this.chat}} @user={{this.user}} />`
     );
 
     assert.dom(containerSelector(this.user, { online: false })).exists();
   });
+
+  test("@interactive=true", async function (assert) {
+    this.user = fabricators.user();
+
+    await render(
+      hbs`<ChatUserAvatar @interactive={{false}}  @user={{this.user}} />`
+    );
+
+    assert.dom(".clickable").doesNotExist();
+  });
 });