Skip to content
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

Play different sound when notification is urgent #85

Merged

Conversation

meisenzahl
Copy link
Member

@meisenzahl meisenzahl commented Aug 22, 2020

Related to #75

Works in conjunction with elementary/sound-theme#11

Test with io.elementary.notifications.demo and set Priority to Urgent.

@meisenzahl
Copy link
Member Author

@marbetschar, @danrabbit and @cassidyjames I would appreciate your feedback

@marbetschar
Copy link
Member

@meisenzahl your PR does not play a sound at all on my end for URGENT notifications - but it does for regular ones. Is this the intended behaviour?

Tested it on elementary Daily (Pre-Release of Odin) using Time Limit which sends a notification with NotificationPriority.URGENT once the time is up.

@meisenzahl
Copy link
Member Author

meisenzahl commented Oct 23, 2020

@marbetschar I'm sorry I forgot to mention that this feature only works in combination with elementary/sound-theme#11 🙈

elementary/sound-theme#11 provides a new sound for urgent notifications.

@marbetschar
Copy link
Member

@meisenzahl installed your sound-theme branch, but still got no sound on elementary Daily. I tried logout/login and a reboot - but in both cases despite the Time Limit notification is shown, no sound is played.

@meisenzahl
Copy link
Member Author

@marbetschar thanks for testing it again.

I just created a demo application to make testing easier. Please give this a try.

@meisenzahl meisenzahl closed this Oct 24, 2020
@meisenzahl meisenzahl reopened this Oct 24, 2020
@meisenzahl meisenzahl mentioned this pull request Oct 24, 2020
@meisenzahl
Copy link
Member Author

meisenzahl commented Oct 24, 2020

Screenshot of the demo application to test the notifications server:

Screenshot von 2020-10-25 07 19 54

@marbetschar
Copy link
Member

marbetschar commented Oct 26, 2020

@meisenzahl thanks, but unfortunately still no success. Neither for Time Limit nor for the new notifications demo application. For both cases I can't hear any sound for the notification priority "URGENT". For all other priorities, the default sound is still played. Even a reboot did not help. Also, I double checked and the file /usr/share/sounds/elementary/stereo/dialog-urgent.ogg exists on my laptop and I'm able to play it using the Music app - so something else seems to be wrong here.

Maybe @danrabbit can help out?

@meisenzahl meisenzahl force-pushed the play-different-sound-based-on-priority branch from 926a611 to 0229f10 Compare October 28, 2020 18:37
@meisenzahl
Copy link
Member Author

@marbetschar after a long time I tried it again after a new installation. Here it works. Does it work for you now?

Copy link
Member

@marbetschar marbetschar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me, so its a 👍 from my end. Maybe someone from @elementary/ux wants review this as well before we merge it.

@danirabbit danirabbit merged commit 18601a8 into elementary:master Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants