Windows Notifications
Closed this issue · 17 comments
We have a cuddly little PR over in #161 and already made decent progress. Here's what's left to do:
- Set the body/text of the toast message
- Set image of the toast
- Verify multiple templates
- Handle callbacks
- Document the shortcut requirement on Windows 8
- Handle
close()
As for the Win8 shortcut requirement, IMO installing a shortcut doesn't belong here as the electron app should be responsible for creating a start menu shortcut when it first gets installed. Found legit looking code (from MSDN) to check for a shortcut (and, install it if we go that route).
👎 on creating shortcuts automatically as well
Seems solid, I'll take it out. Thanks for the feedback!
Images are now working 🎆
Callbacks are working, too 🔥
If you ignored close you'd probably be fine, I believe Linux does the same
@jpoon just wanted a few hours to bang his head against it - but I agree, getting what we have right now in would already be awesome.
@paulcbetts would you mind taking a short look if you agree with the stuff we did here? I'd ❤️ if you could take a peek.
Hide/Close is now implemented. Should have parity with Linux/Mac implementations.
For the shortcut requirement, I'm curious if that is actually the case. According to MSDN in order to send a toast, we need:
- For a desktop app to display a toast, the app must have a shortcut on the Start screen.
- The shortcut must have an AppUserModelID.
On Win10, I had neither and was still able to send a toast. I have a gut feeling that the documentation may be out-of-date and the shortcut is not necessary for Win8. I'll spin up a VM tomorrow and report my findings.
On Win10, I had neither and was still able to send a toast. I have a gut feeling that the documentation may be out-of-date and the shortcut is not necessary for Win8. I'll spin up a VM tomorrow and report my findings.
It is probably changed on Windows 10, on my Windows 8 machine I have to create a shortcut in start screen with an AppUserModelID to be able to send toast notification.
In the current state the code is not going to run on Windows 7. You are linking WindowsCreateString, WindowsDeleteString, RoGetActivationFactory and RoInitialize statically. It has to be loaded dynamically using LoadLibrary / GetProcAddress. I have commented on that already in #161.
Also I would suggest using the DesktopToastFailedEventHandler to raise https://developer.mozilla.org/en-US/docs/Web/API/Notification/onerror. Currently, when you pass a http/https icon URL, the notification does not display on Windows 8 and the DesktopToastFailedEventHandler gets triggered, but it's not being handled. Therefore nothing happens.
Super appreciate all the feedback @miniak. We'll get another PR out there shortly to address Win7 issue.
@felixrieseberg I didn't notice that you are using delayload. That's definitely more elegant. In this case the whole DLL is unavailable on Win7. You can only use it per library, not per symbol, am I right?
Yeah, it does have a few constraints.
I'll close this issue for now - we have the work sketched out here done (except for the excellent addition of the DesktopToastFailedEventHandler, which @miniak already kindly made a PR for).