-
Notifications
You must be signed in to change notification settings - Fork 322
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
fix: fixes mapping on snoretoast activate event, fixes #291 #347
Conversation
Brilliant. This should fix #291 finally. Good work. Unsure what to do with this as a version update though. Think this might be a major version bump, even though it technically was a bug as the API has changed. |
Looking forward to this fix! It sounds like a major version update to me @mikaelbr since it has a breaking change |
see mikaelbr/node-notifier#347 wait for node-notifier release
This PR doesn't actually fix #291 in Windows, and it appears to have no observable effect at all. Here's the sequence of events:
A simple workaround is to default the activation type to resultantData = resultantData.activationType || 'activate'; // lib/utils.js#L264 |
I'm seeing the same thing as @tony19: this doesn't do anything, at least on Windows 10 20H2. The pipe never gets written to upon activation of the notification, which I believe is a bug in With this change, everything suddenly starts working, both plain activate events and actions passed as buttons. I'm not sure how it's working at all for some people? |
@DuBistKomisch could you publish a PR with your changes to their gitlab ? Or alternatively, you could compile your changes, and make a PR here with the new binaries as for your question
toaster with actions seems to work for me only if |
@Araxeus I've got a fork of I had another look at the snoretoast code and there seems to be another code path for a "background callback" which writes to the pipe properly. However it depends on writing the callback to the Windows registry when snoretoast "installs" a shortcut, so this wouldn't work for a normal shortcut created by a normal Windows installer with just the userAppModelID set (such as for our app). This highlights another bug in Anyway, I can put up some PRs on Monday, try my fork in the meantime :) |
@DuBistKomisch Thanks alot!! shame the we can't yet just use |
I've submitted a PR to snoretoast at KDE/snoretoast#15, and rebuilt it for node-notifier at #375. Turns out this doesn't really have much to do with this PR or #291 after all since it only happens with |
The mapper-function for the resultantData did not map "activate" to "click", so fixed the mapper to handle this.
This introduces a breaking change.