search

h4llow3En / mac-notification-sys

✉️ A simple wrapper to deliver or schedule macOS Notifications in Rust
98 stars 28 forks source link

feat: allow playing default notification sound #50

Closed amrbashir closed 1 year ago

amrbashir commented 1 year ago

ref: https://github.com/tauri-apps/tauri/pull/7269

lucasfernog commented 1 year ago

I've been testing this but I don't know how to change the default sound on macOS lol

Note that with this change if you provide an unknown sound, the default one will be played.

amrbashir commented 1 year ago

I've been testing this but I don't know how to change the default sound on macOS lol

I think for testing purposes, it is enough to know that a sound is played.

Note that with this change if you provide an unknown sound, the default one will be played.

It is kinda considered a breaking change but I think for the better, no? I mean if you provide a sound, you want a sound to be played even if it doesn't exist, otherwise don't pass a sound at all to make it silent, plus if this is the default macOS sound, I think we should follow it.

Pandawan commented 1 year ago

We might still want to allow explicitly specifying “no sound” when using the sound option. So maybe bring back “_mute” (or similar)?

Otherwise looks p good to me

amrbashir commented 1 year ago

If you want to specify a "no sound" option, then just pass an empty string "", "_mute" was never really exposed to the user.

Pandawan commented 1 year ago

oh I missed that, sorry was checking this rather quickly

hoodie commented 1 year ago

thanks for the contribution @amrbashir, sorry for the late response, if you could go ahead and apply these quick changes I'd go ahead and merge this.

hoodie commented 1 year ago

I found a way to add the enum based API without a breaking change. Thanks for your contribution @amrbashir

casperstorm commented 1 year ago

Great choice to go with the enum approach. Well done on this!