Provide default implementation of showNotification for Electron #165
Conversation
…simply delegates to Electron. This still requires a polyfill to be provided (example in app.js) for Windows 7
Codecov Report
@@ Coverage Diff @@
## master #165 +/- ##
==========================================
+ Coverage 93.66% 93.69% +0.02%
==========================================
Files 15 15
Lines 1294 1300 +6
Branches 215 218 +3
==========================================
+ Hits 1212 1218 +6
Misses 82 82
|
| throw new TypeError("showNotification requires an implementation."); | ||
| const Notification = this.electron.Notification; | ||
| const notify = new Notification(Object.assign(options || {}, { title: title })); | ||
| if (options["onClick"]) { |
There was a problem hiding this comment.
if both onClick and notification are set, won't this result in two subscriptions?
There was a problem hiding this comment.
Yes, but I felt that if you went out of your way to work that out then you want both subscribed. Note that the notification object is provided on the default w3c polyfill that redirects to showNotification which is in place for windows 7. With windows 10 the polyfill should be turned off and I'll be working on surfacing this better in the future.
There was a problem hiding this comment.
Also, on a similar topic, won't having show and once called cause an issue?
There was a problem hiding this comment.
The 'show' event can be fired multiple times so I only wanted it attached once.
Implementation simply delegates to Electron. This still requires a polyfill to be provided (example in app.js) for Windows 7