fix: toast notification #382
Conversation
sekwah41
force-pushed
the
fix/toast-notification
branch
2 times, most recently
from
a90e05b
to
b535a66
Compare
|
I'll approve it myself once I've checked it on Windows, though id like to also check on the other platforms we have just to do a full check. So id like @roldanjr to also review :) |
|
This fixes #367 |
sekwah41
force-pushed
the
fix/toast-notification
branch
from
b535a66
to
017cbd7
Compare
|
I can only check the AppImage package once this fix is published. I only have a Linux machine right now and don't have access to Mac and Windows. |
the portable version at least doesn't show the wrong name, but no icon shows
|
app/main/src/main.ts
Outdated
| title: "READY TO BE INSTALLED", | ||
| message: "Update has been successfully downloaded.", | ||
| actions: ["Quit and Install", "Install it Later"], | ||
| callback: (err, response) => { | ||
| if (!err) { | ||
| if (response === "quit and install") { |
There was a problem hiding this comment.
For some reason this callback seems to be undefined. Though the update installs anyway.
So the icons show fine but something is odd with the response.
When you dismiss it it does seem to respond with dismissed. Ill need to check the events on windows and mac as well as any other callback logic functions as expected.
|
Need to re-visit this to fix it once I get a bit more time. |
|
Maybe we can release the new version of the app after you've figured out the fix for this @sekwah41. Thank you for your effort on this one 🙇🏼 |
|
I am not back on windows as my main OS. I will take a look at this today. |
Co-authored-by: Joseph Musser <me@jnm2.com>
sekwah41
force-pushed
the
fix/toast-notification
branch
from
17b22f8
to
e02a8cc
Compare
|
|
|
Ok so looks like there is an issue atm at least with node-notifier and how it uses snoretoast for multiple actions with custom appId's atm it only returns "undefined" with the notify response callback. So for now I'm commenting out the "Install it Later" option as users can just ignore the notifications on different operating systems. @roldanjr as long as you are ok with that the rest should be fine to merge. |
|
See mikaelbr/node-notifier#332 about the snoretoast issues. |
sekwah41
force-pushed
the
fix/toast-notification
branch
from
192710b
to
b036f3b
Compare
Need to test if this works or not. Based on the documentation it seems like whatever installer/auto updater we have may be what is needed here. Though hopefully not.