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

Syntax sugar for notifier.notify #45

Closed
ariririos opened this issue Feb 20, 2015 · 7 comments
Closed

Syntax sugar for notifier.notify #45

ariririos opened this issue Feb 20, 2015 · 7 comments

Comments

@ariririos
Copy link

Instead of:
notifier.notify({message: 'Hi'})
I think we should be able to use:
notifier.notify('Hi').
The title should be the default - 'Node Notification' or whatever it is on each system.

It's cleaner and gets the point across better. Just my thoughts.

@mikaelbr
Copy link
Owner

Good idea. This is something I do on gulp-notify. I accept patches if you have some time to spare. 👍

@ariririos
Copy link
Author

@mikaelbr I submitted a PR to fix this issue, but I left WindowsBalloon alone- I explained why here.

@kurisubrooks
Copy link
Collaborator

Was this ever resolved? @rioc0719 @mikaelbr

@kurisubrooks
Copy link
Collaborator

I was thinking we could try something like this. In @rioc0719 's pull request, he only states the message (notifier.notify('message');), but I was thinking maybe we could read the program's package.json and get the title of the app, and make that the notification's title, aswell as setting the sound = true, amongst other options that can be used.

I'd send a pull request, but i'm afraid i'll break something.

@mikaelbr
Copy link
Owner

mikaelbr commented Dec 7, 2015

We don't need this to be included in the next release, so you can easily just do a PR and I'll review it for you.

@kurisubrooks
Copy link
Collaborator

I'll give it a try and let you know.

kurisubrooks added a commit to kurisubrooks/node-notifier that referenced this issue Dec 7, 2015
mikaelbr added a commit that referenced this issue Jan 26, 2016
@mikaelbr
Copy link
Owner

Added in #45.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants