-
Notifications
You must be signed in to change notification settings - Fork 10.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NEW] Allow sounds when conversation is focused #9312
Conversation
I wasn't sure how you do internationalization, so only English is implemented. Let me know if you need any work on my part to add other languages. |
@@ -239,7 +243,7 @@ Template.accountPreferences.events({ | |||
} | |||
if (audio) { | |||
const $audio = $(`audio#${ audio }`); | |||
return $audio && $audio[0] && $audio.play(); | |||
return $audio && $audio[0] && $audio[0].play(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here was the audio preview bug. It was calling "play" on an array instead of the first HTML5 audio object.
@@ -42,10 +43,14 @@ Meteor.startup(function() { | |||
KonchatNotification.newMessage(notification.payload.rid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not clear on why embeds only fire notifications when the room is focused, instead of unfocused. I'm 99% sure this is an existing bug, but I've left it as is for now. (L40-41 above)
Any chance to get this merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for this!
@RocketChat/core
Closes #9302
Default is "True", so it's opt-in.
Also fixed a bug in accountPreferences.js where changing notification sound was throwing an error instead of playing the preview.