From d1e4e7cd6faa4f1146a81a29b315b2d58c22ae5a Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 9 May 2023 13:25:33 +0200 Subject: [PATCH] FIX: more precise chat-replying-indicator (#21451) - correctly subscribe/unsubscribe channel - instantly changes users list - adds a test for testing channel change - rewrites tests to be less verbose - ensures users is always an array --- .../components/chat-replying-indicator.hbs | 5 +- .../components/chat-replying-indicator.js | 47 +++---- .../chat-replying-indicator-test.js | 129 ++++++------------ 3 files changed, 61 insertions(+), 120 deletions(-) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.hbs index 1ae711500fd..2f70c795b0e 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.hbs @@ -5,9 +5,8 @@ (if this.presenceChannel.subscribed "is-subscribed") }} {{did-insert this.subscribe}} - {{did-update this.handleDraft @channel.isDraft}} - {{did-update this.subscribe this.channelName}} - {{will-destroy this.teardown}} + {{did-update this.updateSubscription @channel.id}} + {{will-destroy this.unsubscribe}} > {{#if this.shouldRender}} {{this.text}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.js b/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.js index 014ba799321..cfd21404eae 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-replying-indicator.js @@ -1,4 +1,4 @@ -import { isBlank, isPresent } from "@ember/utils"; +import { isPresent } from "@ember/utils"; import Component from "@glimmer/component"; import { inject as service } from "@ember/service"; import I18n from "I18n"; @@ -13,16 +13,27 @@ export default class ChatReplyingIndicator extends Component { @tracked users = []; @action - async subscribe() { - this.presenceChannel = this.presence.getChannel(this.channelName); - this.presenceChannel.on("change", this.handlePresenceChange); - await this.presenceChannel.subscribe(); + async updateSubscription() { + await this.unsubscribe(); + await this.subscribe(); } @action - async resubscribe() { - await this.teardown(); - await this.subscribe(); + async subscribe() { + this.presenceChannel = this.presence.getChannel(this.channelName); + await this.presenceChannel.subscribe(); + this.users = this.presenceChannel.users; + this.presenceChannel.on("change", this.handlePresenceChange); + } + + @action + async unsubscribe() { + this.users = []; + + if (this.presenceChannel.subscribed) { + this.presenceChannel.off("change", this.handlePresenceChange); + await this.presenceChannel.unsubscribe(); + } } @action @@ -30,22 +41,6 @@ export default class ChatReplyingIndicator extends Component { this.users = presenceChannel.users || []; } - @action - async handleDraft() { - if (this.args.channel.isDraft) { - await this.teardown(); - } else { - await this.resubscribe(); - } - } - - @action - async teardown() { - if (this.presenceChannel) { - await this.presenceChannel.unsubscribe(); - } - } - get usernames() { return this.users .filter((u) => u.id !== this.currentUser.id) @@ -53,10 +48,6 @@ export default class ChatReplyingIndicator extends Component { } get text() { - if (isBlank(this.usernames)) { - return; - } - if (this.usernames.length === 1) { return I18n.t("chat.replying_indicator.single_user", { username: this.usernames[0], diff --git a/plugins/chat/test/javascripts/components/chat-replying-indicator-test.js b/plugins/chat/test/javascripts/components/chat-replying-indicator-test.js index a0bb90994b4..8fd4e68204c 100644 --- a/plugins/chat/test/javascripts/components/chat-replying-indicator-test.js +++ b/plugins/chat/test/javascripts/components/chat-replying-indicator-test.js @@ -1,11 +1,19 @@ import { setupRenderingTest } from "discourse/tests/helpers/component-test"; -import { exists, query } from "discourse/tests/helpers/qunit-helpers"; +import { query } from "discourse/tests/helpers/qunit-helpers"; import hbs from "htmlbars-inline-precompile"; import fabricators from "../helpers/fabricators"; import { module, test } from "qunit"; -import { render, settled } from "@ember/test-helpers"; +import { render } from "@ember/test-helpers"; import { joinChannel } from "discourse/tests/helpers/presence-pretender"; +async function addUserToChannel(channelId, id, username) { + await joinChannel(`/chat-reply/${channelId}`, { + id, + avatar_template: "/images/avatar.png", + username, + }); +} + module( "Discourse Chat | Component | chat-replying-indicator", function (hooks) { @@ -16,7 +24,7 @@ module( await render(hbs``); - assert.false(exists(".chat-replying-indicator__text")); + assert.dom(".chat-replying-indicator__text").doesNotExist(); }); test("displays indicator when user is replying", async function (assert) { @@ -24,11 +32,7 @@ module( await render(hbs``); - await joinChannel("/chat-reply/1", { - id: 1, - avatar_template: "/images/avatar.png", - username: "sam", - }); + await addUserToChannel(1, 1, "sam"); assert.strictEqual( query(".chat-replying-indicator__text").innerText, @@ -41,22 +45,12 @@ module( await render(hbs``); - await joinChannel("/chat-reply/1", { - id: 1, - avatar_template: "/images/avatar.png", - username: "sam", - }); + await addUserToChannel(1, 1, "sam"); + await addUserToChannel(1, 2, "mark"); - await joinChannel("/chat-reply/1", { - id: 2, - avatar_template: "/images/avatar.png", - username: "mark", - }); - - assert.strictEqual( - query(".chat-replying-indicator__text").innerText, - `sam and mark are typing` - ); + assert + .dom(".chat-replying-indicator__text") + .hasText("sam and mark are typing"); }); test("displays indicator when 3 users are replying", async function (assert) { @@ -64,28 +58,13 @@ module( await render(hbs``); - await joinChannel("/chat-reply/1", { - id: 1, - avatar_template: "/images/avatar.png", - username: "sam", - }); + await addUserToChannel(1, 1, "sam"); + await addUserToChannel(1, 2, "mark"); + await addUserToChannel(1, 3, "joffrey"); - await joinChannel("/chat-reply/1", { - id: 2, - avatar_template: "/images/avatar.png", - username: "mark", - }); - - await joinChannel("/chat-reply/1", { - id: 3, - avatar_template: "/images/avatar.png", - username: "joffrey", - }); - - assert.strictEqual( - query(".chat-replying-indicator__text").innerText, - `sam, mark and joffrey are typing` - ); + assert + .dom(".chat-replying-indicator__text") + .hasText("sam, mark and joffrey are typing"); }); test("displays indicator when more than 3 users are replying", async function (assert) { @@ -93,34 +72,14 @@ module( await render(hbs``); - await joinChannel("/chat-reply/1", { - id: 1, - avatar_template: "/images/avatar.png", - username: "sam", - }); + await addUserToChannel(1, 1, "sam"); + await addUserToChannel(1, 2, "mark"); + await addUserToChannel(1, 3, "joffrey"); + await addUserToChannel(1, 4, "taylor"); - await joinChannel("/chat-reply/1", { - id: 2, - avatar_template: "/images/avatar.png", - username: "mark", - }); - - await joinChannel("/chat-reply/1", { - id: 3, - avatar_template: "/images/avatar.png", - username: "joffrey", - }); - - await joinChannel("/chat-reply/1", { - id: 4, - avatar_template: "/images/avatar.png", - username: "taylor", - }); - - assert.strictEqual( - query(".chat-replying-indicator__text").innerText, - `sam, mark and 2 others are typing` - ); + assert + .dom(".chat-replying-indicator__text") + .hasText("sam, mark and 2 others are typing"); }); test("filters current user from list of repliers", async function (assert) { @@ -128,32 +87,24 @@ module( await render(hbs``); - await joinChannel("/chat-reply/1", { - id: 1, - avatar_template: "/images/avatar.png", - username: "sam", - }); + await addUserToChannel(1, 1, "sam"); + await addUserToChannel(1, this.currentUser.id, this.currentUser.username); - await joinChannel("/chat-reply/1", this.currentUser); - - assert.strictEqual( - query(".chat-replying-indicator__text").innerText, - `sam is typing` - ); + assert.dom(".chat-replying-indicator__text").hasText("sam is typing"); }); - test("resets presence when channel is draft", async function (assert) { - this.channel = fabricators.chatChannel(); + test("resets presence when channel changes", async function (assert) { + this.set("channel", fabricators.chatChannel()); + + await addUserToChannel(1, 1, "sam"); await render(hbs``); - assert.dom(".chat-replying-indicator.is-subscribed").exists(); + assert.dom(".chat-replying-indicator__text").hasText("sam is typing"); - this.channel.isDraft = true; + this.set("channel", fabricators.chatChannel({ id: 2 })); - await settled(); - - assert.dom(".chat-replying-indicator.is-subscribed").doesNotExist(); + assert.dom(".chat-replying-indicator__text").doesNotExist(); }); } );