firecat53 / networkmanager-dmenu

Control NetworkManager via dmenu
MIT License
783 stars 74 forks source link

Improve notifications #96

Closed Kasper24 closed 2 years ago

Kasper24 commented 2 years ago

It will be a better idea to adjust the message to have a different title per notification than just a generic 'Network' but this is still better than having 'notify-send' (or 'networkmanager-dmenu' with this PR) show up as the notification title.

firecat53 commented 2 years ago

Using dunst, which I think would be pretty common amongst people using this tool, the notification with this PR reads "Network Activated wg0" instead of just "Activated wg0". I find the current implementation perfectly sufficient given that I'm typically doing something to trigger the notification, so I don't need additional words telling me what system the notification belongs to.

I think if you want to try a more flexible approach so that ultimately I can still just see "Activated wg0" when using dunst, I'd be okay with that.

Thoughts? Sorry about the delayed response, by the way! I appreciate your interest in the project.

Scott

Kasper24 commented 2 years ago

Using dunst, which I think would be pretty common amongst people using this tool, the notification with this PR reads "Network Activated wg0" instead of just "Activated wg0". I find the current implementation perfectly sufficient given that I'm typically doing something to trigger the notification, so I don't need additional words telling me what system the notification belongs to.

I think if you want to try a more flexible approach so that ultimately I can still just see "Activated wg0" when using dunst, I'd be okay with that.

Thoughts? Sorry about the delayed response, by the way! I appreciate your interest in the project.

Scott

It's just really passing a parameter to the title flag, I think it makes more sense than leaving it empty which will make it show notify-send instead.

I'm using Awesomewm which has it's own notification libary, so not sure how customizable dunst is, but it's a matter of separating the title text from message text if you don't want it show like that.

If you are still against that, maybe merge the app name parameter only? That makes it possible to know that the notification is coming from your app

firecat53 commented 2 years ago

I went ahead and merged the app name change. You planted a seed though, and I may mull it over and end up adding the title as well and adjusting my dunst configuration :) Thanks again for the ideas!