DEV: Refactor composer-/topic-presence-display (#29262)

A followup to f05b984208

* modifiers to keep track of components' lifecycles, instead of did-insert/did-update/willDestroy
* proper glimmer-friendly tracking in related models
* caching
* `@outletArgs`
* gjs

app/assets/javascripts/discourse/app/models/composer.js

@@ -187,6 +187,11 @@ export default class Composer extends RestModel {
 
   @service dialog;
 
+  @tracked topic;
+  @tracked post;
+  @tracked reply;
+  @tracked whisper;
+
   unlistTopic = false;
   noBump = false;
   draftSaving = false;
@@ -200,7 +205,6 @@ export default class Composer extends RestModel {
   @not("creatingPrivateMessage") notCreatingPrivateMessage;
   @not("privateMessage") notPrivateMessage;
   @or("creatingTopic", "editingFirstPost") topicFirstPost;
-  @equal("action", REPLY) replyingToTopic;
   @equal("composeState", OPEN) viewOpen;
   @equal("composeState", DRAFT) viewDraft;
   @equal("composeState", FULLSCREEN) viewFullscreen;
@@ -263,6 +267,14 @@ export default class Composer extends RestModel {
     return categoryId ? Category.findById(categoryId) : null;
   }
 
+  get replyingToTopic() {
+    return this.get("action") === REPLY;
+  }
+
+  get editingPost() {
+    return isEdit(this.get("action"));
+  }
+
   @discourseComputed("category.minimumRequiredTags")
   minimumRequiredTags(minimumRequiredTags) {
     return minimumRequiredTags || 0;
@@ -286,11 +298,6 @@ export default class Composer extends RestModel {
     );
   }
 
-  @discourseComputed("action")
-  editingPost(action) {
-    return isEdit(action);
-  }
-
   @observes("composeState")
   composeStateChanged() {
     const oldOpen = this.composerOpened;

app/assets/javascripts/discourse/app/services/presence.js

@@ -1,4 +1,5 @@
 import EmberObject, { computed } from "@ember/object";
+import { dependentKeyCompat } from "@ember/object/compat";
 import Evented from "@ember/object/evented";
 import { cancel, debounce, next, once, throttle } from "@ember/runloop";
 import Service, { service } from "@ember/service";
@@ -108,12 +109,11 @@ class PresenceChannel extends EmberObject.extend(Evented) {
     this.trigger("change", this);
   }
 
-  @computed("_presenceState.users", "subscribed")
+  @dependentKeyCompat
   get users() {
-    if (!this.subscribed) {
+    if (this.get("subscribed")) {
-      return;
+      return this.get("_presenceState.users");
     }
-    return this._presenceState?.users;
   }
 
   @computed("_presenceState.count", "subscribed")

app/assets/javascripts/discourse/tests/helpers/presence-pretender.js

@@ -34,10 +34,7 @@ export default function (helper) {
 }
 
 export function getChannelInfo(name) {
-  return (
+  return (channels[name] ||= { count: 0, users: [], last_message_id: 0 });
-    channels[name] ||
-    (channels[name] = { count: 0, users: [], last_message_id: 0 })
-  );
 }
 
 export async function joinChannel(name, user) {

plugins/discourse-presence/assets/javascripts/discourse/components/composer-presence-display.gjs

@@ -0,0 +1,149 @@
+import Component from "@glimmer/component";
+import { cached, tracked } from "@glimmer/tracking";
+import { service } from "@ember/service";
+import { modifier } from "ember-modifier";
+import { gt } from "truth-helpers";
+import UserLink from "discourse/components/user-link";
+import avatar from "discourse/helpers/avatar";
+import i18n from "discourse-common/helpers/i18n";
+
+export default class ComposerPresenceDisplay extends Component {
+  @service presence;
+  @service composerPresenceManager;
+  @service currentUser;
+  @service siteSettings;
+
+  @tracked replyChannel;
+  @tracked whisperChannel;
+  @tracked editChannel;
+
+  setupReplyChannel = modifier(() => {
+    const topic = this.args.model.topic;
+    if (!topic || !this.isReply) {
+      return;
+    }
+
+    const replyChannel = this.presence.getChannel(
+      `/discourse-presence/reply/${topic.id}`
+    );
+    replyChannel.subscribe();
+    this.replyChannel = replyChannel;
+
+    return () => replyChannel.unsubscribe();
+  });
+
+  setupWhisperChannel = modifier(() => {
+    if (
+      !this.args.model.topic ||
+      !this.isReply ||
+      !this.currentUser.staff ||
+      !this.currentUser.whisperer
+    ) {
+      return;
+    }
+
+    const whisperChannel = this.presence.getChannel(
+      `/discourse-presence/whisper/${this.args.model.topic.id}`
+    );
+    whisperChannel.subscribe();
+    this.whisperChannel = whisperChannel;
+
+    return () => whisperChannel.unsubscribe();
+  });
+
+  setupEditChannel = modifier(() => {
+    if (!this.args.model.post || !this.isEdit) {
+      return;
+    }
+
+    const editChannel = this.presence.getChannel(
+      `/discourse-presence/edit/${this.args.model.post.id}`
+    );
+    editChannel.subscribe();
+    this.editChannel = editChannel;
+
+    return () => editChannel.unsubscribe();
+  });
+
+  notifyState = modifier(() => {
+    const { topic, post, reply } = this.args.model;
+    const raw = this.isEdit ? post?.raw || "" : "";
+    const entity = this.isEdit ? post : topic;
+
+    if (reply !== raw) {
+      this.composerPresenceManager.notifyState(this.state, entity?.id);
+    }
+
+    return () => this.composerPresenceManager.leave();
+  });
+
+  get isReply() {
+    return this.state === "reply" || this.state === "whisper";
+  }
+
+  get isEdit() {
+    return this.state === "edit";
+  }
+
+  get state() {
+    if (this.args.model.editingPost) {
+      return "edit";
+    } else if (this.args.model.whisper) {
+      return "whisper";
+    } else if (this.args.model.replyingToTopic) {
+      return "reply";
+    }
+  }
+
+  @cached
+  get users() {
+    let users;
+    if (this.isEdit) {
+      users = this.editChannel?.users || [];
+    } else {
+      const replyUsers = this.replyChannel?.users || [];
+      const whisperUsers = this.whisperChannel?.users || [];
+      users = [...replyUsers, ...whisperUsers];
+    }
+
+    return users
+      .filter((u) => u.id !== this.currentUser.id)
+      .slice(0, this.siteSettings.presence_max_users_shown);
+  }
+
+  <template>
+    <div
+      {{this.setupReplyChannel}}
+      {{this.setupWhisperChannel}}
+      {{this.setupEditChannel}}
+      {{this.notifyState}}
+    >
+      {{#if (gt this.users.length 0)}}
+        <div class="presence-users">
+          <div class="presence-avatars">
+            {{#each this.users as |user|}}
+              <UserLink @user={{user}}>
+                {{avatar user imageSize="small"}}
+              </UserLink>
+            {{/each}}
+          </div>
+
+          <span class="presence-text">
+            <span class="description">
+              {{~#if this.isReply~}}
+                {{i18n "presence.replying" count=this.users.length}}
+              {{~else~}}
+                {{i18n "presence.editing" count=this.users.length}}
+              {{~/if~}}
+            </span>
+            <span class="wave">
+              <span class="dot">.</span>
+              <span class="dot">.</span>
+              <span class="dot">.</span>
+            </span>
+          </span>
+        </div>
+      {{/if}}
+    </div>
+  </template>
+}

plugins/discourse-presence/assets/javascripts/discourse/components/composer-presence-display.hbs

@@ -1,30 +0,0 @@
-<div
-  {{did-insert this.setupChannels}}
-  {{did-update this.setupChannels @model.reply @model.whisper this.state}}
->
-  {{#if this.shouldDisplay}}
-    <div class="presence-users">
-      <div class="presence-avatars">
-        {{#each this.users as |user|}}
-          <UserLink @user={{user}}>
-            {{avatar user imageSize="small"}}
-          </UserLink>
-        {{/each}}
-      </div>
-      <span class="presence-text">
-        <span class="description">
-          {{~#if this.isReply~}}
-            {{i18n "presence.replying" count=this.users.length}}
-          {{~else~}}
-            {{i18n "presence.editing" count=this.users.length}}
-          {{~/if~}}
-        </span>
-        <span class="wave">
-          <span class="dot">.</span>
-          <span class="dot">.</span>
-          <span class="dot">.</span>
-        </span>
-      </span>
-    </div>
-  {{/if}}
-</div>

plugins/discourse-presence/assets/javascripts/discourse/components/composer-presence-display.js

@@ -1,137 +0,0 @@
-import Component from "@glimmer/component";
-import { tracked } from "@glimmer/tracking";
-import { action } from "@ember/object";
-import { service } from "@ember/service";
-
-export default class ComposerPresenceDisplayComponent extends Component {
-  @service presence;
-  @service composerPresenceManager;
-  @service currentUser;
-  @service siteSettings;
-
-  @tracked replyChannel;
-  @tracked whisperChannel;
-  @tracked editChannel;
-
-  get isReply() {
-    return this.state === "reply" || this.state === "whisper";
-  }
-
-  get isEdit() {
-    return this.state === "edit";
-  }
-
-  get state() {
-    const { editingPost, whisper, replyingToTopic } = this.args.model;
-
-    if (editingPost) {
-      return "edit";
-    } else if (whisper) {
-      return "whisper";
-    } else if (replyingToTopic) {
-      return "reply";
-    }
-  }
-
-  get replyChannelName() {
-    const topicId = this.args.model?.topic?.id;
-    if (topicId && this.isReply) {
-      return `/discourse-presence/reply/${topicId}`;
-    }
-  }
-
-  get whisperChannelName() {
-    const topicId = this.args.model?.topic?.id;
-    if (topicId && this.isReply && this.currentUser.whisperer) {
-      return `/discourse-presence/whisper/${topicId}`;
-    }
-  }
-
-  get editChannelName() {
-    const postId = this.args.model?.post?.id;
-    if (postId && this.isEdit) {
-      return `/discourse-presence/edit/${postId}`;
-    }
-  }
-
-  get replyUsers() {
-    return this.replyChannel?.users || [];
-  }
-
-  get whisperUsers() {
-    return this.whisperChannel?.users || [];
-  }
-
-  get replyingUsers() {
-    return [...this.replyUsers, ...this.whisperUsers];
-  }
-
-  get editingUsers() {
-    return this.editChannel?.users || [];
-  }
-
-  get users() {
-    const users = this.isEdit ? this.editingUsers : this.replyingUsers;
-    return users
-      .filter((u) => u.id !== this.currentUser.id)
-      .slice(0, this.siteSettings.presence_max_users_shown);
-  }
-
-  get shouldDisplay() {
-    return this.users.length > 0;
-  }
-
-  @action
-  setupChannels() {
-    this.setupReplyChannel();
-    this.setupWhisperChannel();
-    this.setupEditChannel();
-    this.notifyState();
-  }
-
-  setupReplyChannel() {
-    this.setupChannel("replyChannel", this.replyChannelName);
-  }
-
-  setupWhisperChannel() {
-    if (this.currentUser.staff) {
-      this.setupChannel("whisperChannel", this.whisperChannelName);
-    }
-  }
-
-  setupEditChannel() {
-    this.setupChannel("editChannel", this.editChannelName);
-  }
-
-  setupChannel(key, name) {
-    if (this[key]?.name !== name) {
-      this[key]?.unsubscribe();
-      if (name) {
-        this[key] = this.presence.getChannel(name);
-        this[key].subscribe();
-      }
-    }
-  }
-
-  notifyState() {
-    const { reply, post, topic } = this.args.model;
-    const raw = this.isEdit ? post?.raw || "" : "";
-    const entity = this.isEdit ? post : topic;
-
-    if (reply !== raw) {
-      this.composerPresenceManager.notifyState(this.state, entity?.id);
-    }
-  }
-
-  willDestroy() {
-    super.willDestroy(...arguments);
-    this.unsubscribeFromChannels();
-    this.composerPresenceManager.leave();
-  }
-
-  unsubscribeFromChannels() {
-    this.replyChannel?.unsubscribe();
-    this.whisperChannel?.unsubscribe();
-    this.editChannel?.unsubscribe();
-  }
-}

plugins/discourse-presence/assets/javascripts/discourse/components/topic-presence-display.gjs

@@ -0,0 +1,77 @@
+import Component from "@glimmer/component";
+import { cached, tracked } from "@glimmer/tracking";
+import { service } from "@ember/service";
+import { modifier } from "ember-modifier";
+import { gt } from "truth-helpers";
+import UserLink from "discourse/components/user-link";
+import avatar from "discourse/helpers/avatar";
+import i18n from "discourse-common/helpers/i18n";
+
+export default class TopicPresenceDisplay extends Component {
+  @service presence;
+  @service currentUser;
+
+  @tracked replyChannel;
+  @tracked whisperChannel;
+
+  setupReplyChannel = modifier(() => {
+    const replyChannel = this.presence.getChannel(
+      `/discourse-presence/reply/${this.args.topic.id}`
+    );
+    replyChannel.subscribe();
+    this.replyChannel = replyChannel;
+
+    return () => replyChannel.unsubscribe();
+  });
+
+  setupWhisperChannels = modifier(() => {
+    if (!this.currentUser.staff) {
+      return;
+    }
+
+    const whisperChannel = this.presence.getChannel(
+      `/discourse-presence/whisper/${this.args.topic.id}`
+    );
+    whisperChannel.subscribe();
+    this.whisperChannel = whisperChannel;
+
+    return () => whisperChannel.unsubscribe();
+  });
+
+  @cached
+  get users() {
+    const replyUsers = this.replyChannel?.users || [];
+    const whisperUsers = this.whisperChannel?.users || [];
+
+    return [...replyUsers, ...whisperUsers].filter(
+      (u) => u.id !== this.currentUser.id
+    );
+  }
+
+  <template>
+    <div {{this.setupReplyChannel}} {{this.setupWhisperChannels}}>
+      {{#if (gt this.users.length 0)}}
+        <div class="presence-users">
+          <div class="presence-avatars">
+            {{#each this.users as |user|}}
+              <UserLink @user={{user}}>
+                {{avatar user imageSize="small"}}
+              </UserLink>
+            {{/each}}
+          </div>
+
+          <span class="presence-text">
+            <span class="description">
+              {{i18n "presence.replying_to_topic" count=this.users.length}}
+            </span>
+            <span class="wave">
+              <span class="dot">.</span>
+              <span class="dot">.</span>
+              <span class="dot">.</span>
+            </span>
+          </span>
+        </div>
+      {{/if}}
+    </div>
+  </template>
+}

plugins/discourse-presence/assets/javascripts/discourse/components/topic-presence-display.hbs

@@ -1,26 +0,0 @@
-<div
-  {{did-insert this.setupChannels}}
-  {{did-update this.setupChannels @topic.id}}
->
-  {{#if this.shouldDisplay}}
-    <div class="presence-users">
-      <div class="presence-avatars">
-        {{#each this.users as |user|}}
-          <UserLink @user={{user}}>
-            {{avatar user imageSize="small"}}
-          </UserLink>
-        {{/each}}
-      </div>
-      <span class="presence-text">
-        <span class="description">
-          {{i18n "presence.replying_to_topic" count=this.users.length}}
-        </span>
-        <span class="wave">
-          <span class="dot">.</span>
-          <span class="dot">.</span>
-          <span class="dot">.</span>
-        </span>
-      </span>
-    </div>
-  {{/if}}
-</div>

plugins/discourse-presence/assets/javascripts/discourse/components/topic-presence-display.js

@@ -1,72 +0,0 @@
-import Component from "@glimmer/component";
-import { tracked } from "@glimmer/tracking";
-import { action } from "@ember/object";
-import { service } from "@ember/service";
-
-export default class TopicPresenceDisplayComponent extends Component {
-  @service presence;
-  @service currentUser;
-
-  @tracked replyChannel;
-  @tracked whisperChannel;
-
-  get replyChannelName() {
-    return `/discourse-presence/reply/${this.args.topic.id}`;
-  }
-
-  get whisperChannelName() {
-    return `/discourse-presence/whisper/${this.args.topic.id}`;
-  }
-
-  get replyUsers() {
-    return this.replyChannel?.users || [];
-  }
-
-  get whisperUsers() {
-    return this.whisperChannel?.users || [];
-  }
-
-  get users() {
-    return [...this.replyUsers, ...this.whisperUsers].filter(
-      (u) => u.id !== this.currentUser.id
-    );
-  }
-
-  get shouldDisplay() {
-    return this.users.length > 0;
-  }
-
-  @action
-  setupChannels() {
-    this.setupReplyChannel();
-    this.setupWhisperChannel();
-  }
-
-  willDestroy() {
-    super.willDestroy(...arguments);
-    this.unsubscribeFromChannels();
-  }
-
-  unsubscribeFromChannels() {
-    this.replyChannel?.unsubscribe();
-    this.whisperChannel?.unsubscribe();
-  }
-
-  setupReplyChannel() {
-    if (this.replyChannel?.name !== this.replyChannelName) {
-      this.replyChannel?.unsubscribe();
-      this.replyChannel = this.presence.getChannel(this.replyChannelName);
-      this.replyChannel.subscribe();
-    }
-  }
-
-  setupWhisperChannel() {
-    if (this.currentUser.staff) {
-      if (this.whisperChannel?.name !== this.whisperChannelName) {
-        this.whisperChannel?.unsubscribe();
-        this.whisperChannel = this.presence.getChannel(this.whisperChannelName);
-        this.whisperChannel.subscribe();
-      }
-    }
-  }
-}

plugins/discourse-presence/assets/javascripts/discourse/connectors/before-composer-controls/presence.hbs

@@ -1 +1 @@
-<ComposerPresenceDisplay @model={{this.model}} />
+<ComposerPresenceDisplay @model={{@outletArgs.model}} />

plugins/discourse-presence/assets/javascripts/discourse/connectors/topic-above-footer-buttons/presence.hbs

@@ -1,2 +1,2 @@
 {{! Note: the topic-above-footer-buttons outlet is only rendered for logged-in users }}
-<TopicPresenceDisplay @topic={{this.model}} />
+<TopicPresenceDisplay @topic={{@outletArgs.model}} />

plugins/discourse-presence/test/javascripts/acceptance/discourse-presence-test.js

@@ -6,12 +6,7 @@ import {
   leaveChannel,
   presentUserIds,
 } from "discourse/tests/helpers/presence-pretender";
-import {
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
-  acceptance,
-  count,
-  exists,
-  query,
-} from "discourse/tests/helpers/qunit-helpers";
 import selectKit from "discourse/tests/helpers/select-kit-helper";
 
 acceptance("Discourse Presence Plugin", function (needs) {
@@ -33,7 +28,7 @@ acceptance("Discourse Presence Plugin", function (needs) {
     assert.strictEqual(
       currentURL(),
       "/t/internationalization-localization/280",
-      "it transitions to the newly created topic URL"
+      "transitions to the newly created topic URL"
     );
   });
 
@@ -41,7 +36,7 @@ acceptance("Discourse Presence Plugin", function (needs) {
     await visit("/t/internationalization-localization/280");
 
     await click("#topic-footer-buttons .btn.create");
-    assert.ok(exists(".d-editor-input"), "the composer input is visible");
+    assert.dom(".d-editor-input").exists("the composer input is visible");
 
     assert.deepEqual(
       presentUserIds("/discourse-presence/reply/280"),
@@ -70,7 +65,7 @@ acceptance("Discourse Presence Plugin", function (needs) {
     await visit("/t/internationalization-localization/280");
 
     await click("#topic-footer-buttons .btn.create");
-    assert.ok(exists(".d-editor-input"), "the composer input is visible");
+    assert.dom(".d-editor-input").exists("the composer input is visible");
 
     await fillIn(".d-editor-input", "this is the content of my reply");
 
@@ -84,11 +79,9 @@ acceptance("Discourse Presence Plugin", function (needs) {
     await menu.expand();
     await menu.selectRowByName("toggle-whisper");
 
-    assert.strictEqual(
+    assert
-      count(".composer-actions svg.d-icon-far-eye-slash"),
+      .dom(".composer-actions svg.d-icon-far-eye-slash")
-      1,
+      .exists("sets the post type to whisper");
-      "it sets the post type to whisper"
-    );
 
     assert.deepEqual(
       presentUserIds("/discourse-presence/reply/280"),
@@ -117,11 +110,13 @@ acceptance("Discourse Presence Plugin", function (needs) {
     await click(".topic-post:nth-of-type(1) button.show-more-actions");
     await click(".topic-post:nth-of-type(1) button.edit");
 
-    assert.strictEqual(
+    assert
-      query(".d-editor-input").value,
+      .dom(".d-editor-input")
-      query(".topic-post:nth-of-type(1) .cooked > p").innerText,
+      .hasValue(
-      "composer has contents of post to be edited"
+        document.querySelector(".topic-post:nth-of-type(1) .cooked > p")
-    );
+          .innerText,
+        "composer has contents of post to be edited"
+      );
 
     assert.deepEqual(
       presentUserIds("/discourse-presence/edit/398"),
@@ -158,7 +153,7 @@ acceptance("Discourse Presence Plugin", function (needs) {
     assert
       .dom(".topic-above-footer-buttons-outlet.presence")
       .exists("includes the presence component");
-    assert.strictEqual(count(avatarSelector), 0, "no avatars displayed");
+    assert.dom(avatarSelector).doesNotExist("no avatars displayed");
 
     await joinChannel("/discourse-presence/reply/280", {
       id: 123,
@@ -166,7 +161,7 @@ acceptance("Discourse Presence Plugin", function (needs) {
       username: "my-username",
     });
 
-    assert.strictEqual(count(avatarSelector), 1, "avatar displayed");
+    assert.dom(avatarSelector).exists({ count: 1 }, "avatar displayed");
 
     await joinChannel("/discourse-presence/whisper/280", {
       id: 124,
@@ -174,28 +169,28 @@ acceptance("Discourse Presence Plugin", function (needs) {
       username: "my-username2",
     });
 
-    assert.strictEqual(count(avatarSelector), 2, "whisper avatar displayed");
+    assert.dom(avatarSelector).exists({ count: 2 }, "whisper avatar displayed");
 
     await leaveChannel("/discourse-presence/reply/280", {
       id: 123,
     });
 
-    assert.strictEqual(count(avatarSelector), 1, "reply avatar removed");
+    assert.dom(avatarSelector).exists({ count: 1 }, "reply avatar removed");
 
     await leaveChannel("/discourse-presence/whisper/280", {
       id: 124,
     });
 
-    assert.strictEqual(count(avatarSelector), 0, "whisper avatar removed");
+    assert.dom(avatarSelector).doesNotExist("whisper avatar removed");
   });
 
   test("Displays replying and whispering presence in composer", async function (assert) {
     await visit("/t/internationalization-localization/280");
     await click("#topic-footer-buttons .btn.create");
-    assert.ok(exists(".d-editor-input"), "the composer input is visible");
+    assert.dom(".d-editor-input").exists("the composer input is visible");
 
     const avatarSelector = ".reply-to .presence-avatars .avatar";
-    assert.strictEqual(count(avatarSelector), 0, "no avatars displayed");
+    assert.dom(avatarSelector).doesNotExist("no avatars displayed");
 
     await joinChannel("/discourse-presence/reply/280", {
       id: 123,
@@ -203,7 +198,7 @@ acceptance("Discourse Presence Plugin", function (needs) {
       username: "my-username",
     });
 
-    assert.strictEqual(count(avatarSelector), 1, "avatar displayed");
+    assert.dom(avatarSelector).exists({ count: 1 }, "avatar displayed");
 
     await joinChannel("/discourse-presence/whisper/280", {
       id: 124,
@@ -211,18 +206,18 @@ acceptance("Discourse Presence Plugin", function (needs) {
       username: "my-username2",
     });
 
-    assert.strictEqual(count(avatarSelector), 2, "whisper avatar displayed");
+    assert.dom(avatarSelector).exists({ count: 2 }, "whisper avatar displayed");
 
     await leaveChannel("/discourse-presence/reply/280", {
       id: 123,
     });
 
-    assert.strictEqual(count(avatarSelector), 1, "reply avatar removed");
+    assert.dom(avatarSelector).exists({ count: 1 }, "reply avatar removed");
 
     await leaveChannel("/discourse-presence/whisper/280", {
       id: 124,
     });
 
-    assert.strictEqual(count(avatarSelector), 0, "whisper avatar removed");
+    assert.dom(avatarSelector).doesNotExist("whisper avatar removed");
   });
 });