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

Add support for notification overrides #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jeffbargmann
Copy link

Host may wish to silence notifications, or hide the body of notifications for privacy.

Host may wish to silence notifications, or hide the body of
notifications for privacy.
Host app may want to know if notification was clicked, in order to
activate it's main window/etc
// Apply overrides
options = Object.assign({}, options);
if (settings.forceSilent) options.silent = true;
if (settings.bodyOverride) options.body = settings.bodyOverride;
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure what's actually happening here. From the comment it seems that options should override settings, but the original values of settings is what seems to be used? Is the returned settings meant to be changed by whoever uses the library, and can be changed at runtime?

Copy link
Owner

Choose a reason for hiding this comment

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

About the bodyOverride: is it to override the notifications when notifications are handled natively? If so, maybe it could be nice to override the icon as well? Hmm

I like the forceSilent. Would there be a benefit to killing the notification all together when that happens, as compared to setting the silent flag?

@seriema
Copy link
Owner

seriema commented Apr 30, 2016

Looks very interesting! Could you help out with some documentation on the Readme.md? If you have some example it would be great to include it in https://github.com/seriema/electron-notification-shim-demos :D

@seriema
Copy link
Owner

seriema commented May 17, 2016

@JBLatenight ping

@seriema
Copy link
Owner

seriema commented Aug 21, 2016

@JBLatenight I'm using your code as an inspiraton for a new feature. It's on the branch notification-onclick-override. I still have questions about your code so I might not take it as-is.

@khaled-su
Copy link

khaled-su commented Oct 7, 2016

@seriema Any progress here? Is the branch ready? Thanks.

@seriema
Copy link
Owner

seriema commented Oct 16, 2016

@khaledabbas Sadly, no. You can read my thoughts on it on the feature request #4 as to why I'm a bit stuck atm. Any help is welcome.

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.

None yet

3 participants