From c8beefc1eedccd02d4f543a6462eea1efdb16532 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Fri, 11 Nov 2022 13:11:41 +0100 Subject: [PATCH] FIX: reimplements chat audio into a service (#18983) This implementation attempts to be more resilient to background tab. Notes: - adds support for immediate arg in @debounce decorators - fixes a bug in discourseDebounce which was not supporting immediate arg in tests - chat-audio-manager has no tests as audio requires real user interaction and is hard to test reliably --- .../discourse-common/addon/lib/debounce.js | 10 ++- .../addon/utils/decorators.js | 10 ++- .../tests/unit/utils/decorators-test.js | 16 +++++ .../discourse/controllers/preferences-chat.js | 8 ++- .../discourse/initializers/chat-audio.js | 29 ++++++++ .../initializers/chat-notification-sounds.js | 47 ------------- .../discourse/services/chat-audio-manager.js | 68 +++++++++++++++++++ 7 files changed, 135 insertions(+), 53 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/initializers/chat-audio.js delete mode 100644 plugins/chat/assets/javascripts/discourse/initializers/chat-notification-sounds.js create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-audio-manager.js diff --git a/app/assets/javascripts/discourse-common/addon/lib/debounce.js b/app/assets/javascripts/discourse-common/addon/lib/debounce.js index 2bcd387724a..37171684b15 100644 --- a/app/assets/javascripts/discourse-common/addon/lib/debounce.js +++ b/app/assets/javascripts/discourse-common/addon/lib/debounce.js @@ -9,10 +9,18 @@ import { isTesting } from "discourse-common/config/environment"; export default function () { if (isTesting()) { + const lastArgument = arguments[arguments.length - 1]; + const hasImmediateArgument = typeof lastArgument === "boolean"; + + let args = [].slice.call(arguments, 0, hasImmediateArgument ? -2 : -1); + // Replace the time argument with 10ms - let args = [].slice.call(arguments, 0, -1); args.push(10); + if (hasImmediateArgument) { + args.push(lastArgument); + } + return debounce.apply(undefined, args); } else { return debounce(...arguments); diff --git a/app/assets/javascripts/discourse-common/addon/utils/decorators.js b/app/assets/javascripts/discourse-common/addon/utils/decorators.js index f9544d9176a..1320285db11 100644 --- a/app/assets/javascripts/discourse-common/addon/utils/decorators.js +++ b/app/assets/javascripts/discourse-common/addon/utils/decorators.js @@ -88,7 +88,7 @@ export function readOnly(target, name, desc) { }; } -export function debounce(delay) { +export function debounce(delay, immediate = false) { return function (target, name, descriptor) { return { enumerable: descriptor.enumerable, @@ -97,7 +97,13 @@ export function debounce(delay) { initializer() { const originalFunction = descriptor.value; const debounced = function (...args) { - return discourseDebounce(this, originalFunction, ...args, delay); + return discourseDebounce( + this, + originalFunction, + ...args, + delay, + immediate + ); }; return debounced; diff --git a/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js b/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js index dc7598055eb..1c07f7dcf52 100644 --- a/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js +++ b/app/assets/javascripts/discourse/tests/unit/utils/decorators-test.js @@ -57,12 +57,18 @@ class NativeComponent extends Component { const TestStub = EmberObject.extend({ counter: 0, otherCounter: 0, + state: null, @debounce(50) increment(value) { this.counter += value; }, + @debounce(50, true) + setState(state) { + this.state = state; + }, + // Note: it only works in this particular order: // `@observes()` first, then `@debounce()` @observes("prop") @@ -149,6 +155,16 @@ module("Unit | Utils | decorators", function (hooks) { assert.strictEqual(stub.counter, 6); }); + test("immediate debounce", async function (assert) { + const stub = TestStub.create(); + + stub.setState("foo"); + stub.setState("bar"); + await settled(); + + assert.strictEqual(stub.state, "foo"); + }); + test("debounce works with @observe", async function (assert) { const stub = TestStub.create(); diff --git a/plugins/chat/assets/javascripts/discourse/controllers/preferences-chat.js b/plugins/chat/assets/javascripts/discourse/controllers/preferences-chat.js index 392dc8f7f91..b89baf2ccdc 100644 --- a/plugins/chat/assets/javascripts/discourse/controllers/preferences-chat.js +++ b/plugins/chat/assets/javascripts/discourse/controllers/preferences-chat.js @@ -4,7 +4,8 @@ import discourseComputed from "discourse-common/utils/decorators"; import I18n from "I18n"; import { action } from "@ember/object"; import { popupAjaxError } from "discourse/lib/ajax-error"; -import { CHAT_SOUNDS } from "discourse/plugins/chat/discourse/initializers/chat-notification-sounds"; +import { CHAT_SOUNDS } from "discourse/plugins/chat/discourse/services/chat-audio-manager"; +import { inject as service } from "@ember/service"; const CHAT_ATTRS = [ "chat_enabled", @@ -20,6 +21,8 @@ const EMAIL_FREQUENCY_OPTIONS = [ ]; export default class PreferencesChatController extends Controller { + @service chatAudioManager; + emailFrequencyOptions = EMAIL_FREQUENCY_OPTIONS; @discourseComputed @@ -32,8 +35,7 @@ export default class PreferencesChatController extends Controller { @action onChangeChatSound(sound) { if (sound && !isTesting()) { - const audio = new Audio(CHAT_SOUNDS[sound]); - audio.play(); + this.chatAudioManager.playImmediately(sound); } this.model.set("user_option.chat_sound", sound); } diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-audio.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-audio.js new file mode 100644 index 00000000000..448d31d13da --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-audio.js @@ -0,0 +1,29 @@ +import { withPluginApi } from "discourse/lib/plugin-api"; + +const MENTION = 29; +const MESSAGE = 30; +const CHAT_NOTIFICATION_TYPES = [MENTION, MESSAGE]; + +export default { + name: "chat-audio", + + initialize(container) { + const currentUser = container.lookup("service:current-user"); + const chatService = container.lookup("service:chat"); + + if (!chatService.userCanChat || !currentUser?.chat_sound) { + return; + } + + const chatAudioManager = container.lookup("service:chat-audio-manager"); + chatAudioManager.setup(); + + withPluginApi("0.12.1", (api) => { + api.registerDesktopNotificationHandler((data, siteSettings, user) => { + if (CHAT_NOTIFICATION_TYPES.includes(data.notification_type)) { + chatAudioManager.play(user.chat_sound); + } + }); + }); + }, +}; diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-notification-sounds.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-notification-sounds.js deleted file mode 100644 index 3a9548c739c..00000000000 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-notification-sounds.js +++ /dev/null @@ -1,47 +0,0 @@ -import { withPluginApi } from "discourse/lib/plugin-api"; -import discourseDebounce from "discourse-common/lib/debounce"; - -export const CHAT_SOUNDS = { - bell: "/plugins/chat/audio/bell.mp3", - ding: "/plugins/chat/audio/ding.mp3", -}; - -const MENTION = 29; -const MESSAGE = 30; -const CHAT_NOTIFICATION_TYPES = [MENTION, MESSAGE]; - -const AUDIO_DEBOUNCE_TIMEOUT = 3000; - -export default { - name: "chat-notification-sounds", - initialize(container) { - const currentUser = container.lookup("service:current-user"); - const chatService = container.lookup("service:chat"); - - if (!chatService.userCanChat || !currentUser?.chat_sound) { - return; - } - - function playAudio(user) { - const audio = new Audio(CHAT_SOUNDS[user.chat_sound]); - audio.play().catch(() => { - // eslint-disable-next-line no-console - console.info( - "User needs to interact with DOM before we can play notification sounds" - ); - }); - } - - function playAudioWithDebounce(user) { - discourseDebounce(this, playAudio, user, AUDIO_DEBOUNCE_TIMEOUT, true); - } - - withPluginApi("0.12.1", (api) => { - api.registerDesktopNotificationHandler((data, siteSettings, user) => { - if (CHAT_NOTIFICATION_TYPES.includes(data.notification_type)) { - playAudioWithDebounce(user); - } - }); - }); - }, -}; diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-audio-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-audio-manager.js new file mode 100644 index 00000000000..8a2d339267f --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-audio-manager.js @@ -0,0 +1,68 @@ +import Service from "@ember/service"; +import { debounce } from "discourse-common/utils/decorators"; + +const AUDIO_DEBOUNCE_DELAY = 3000; + +export const CHAT_SOUNDS = { + bell: [{ src: "/plugins/chat/audio/bell.mp3", type: "audio/mpeg" }], + ding: [{ src: "/plugins/chat/audio/ding.mp3", type: "audio/mpeg" }], +}; + +const DEFAULT_SOUND_NAME = "bell"; + +const createAudioCache = (sources) => { + const audio = new Audio(); + sources.forEach(({ type, src }) => { + const source = document.createElement("source"); + source.type = type; + source.src = src; + audio.appendChild(source); + }); + return audio; +}; + +export default class ChatAudioManager extends Service { + _audioCache = {}; + + setup() { + Object.keys(CHAT_SOUNDS).forEach((soundName) => { + this._audioCache[soundName] = createAudioCache(CHAT_SOUNDS[soundName]); + }); + } + + willDestroy() { + this._super(...arguments); + + this._audioCache = {}; + } + + playImmediately(soundName) { + return this._play(soundName); + } + + @debounce(AUDIO_DEBOUNCE_DELAY, true) + play(soundName) { + return this._play(soundName); + } + + _play(soundName) { + const audio = + this._audioCache[soundName] || this._audioCache[DEFAULT_SOUND_NAME]; + + if (!audio.paused) { + audio.pause(); + if (typeof audio.fastSeek === "function") { + audio.fastSeek(0); + } else { + audio.currentTime = 0; + } + } + + return audio.play().catch(() => { + // eslint-disable-next-line no-console + console.info( + "[chat] User needs to interact with DOM before we can play notification sounds." + ); + }); + } +}