mohabouje / WinToast

WinToast is a lightly library written in C++ which brings a complete integration of the modern toast notifications of Windows 8 & Windows 10. Toast notifications allows your app to inform the users about relevant information and timely events that they should see and take action upon inside your app, such as a new instant message, a new friend request, breaking news, or a calendar event.
MIT License
688 stars 127 forks source link

[Request For Comments] Work around problem with the first call #14

Closed dscho closed 6 years ago

dscho commented 6 years ago

This PR is more of a test balloon and a request for help than a polished PR intended to be merged as-is.

In my tests, I could get the toasts to show reliably only when launching the executable after the shortcut was created. If the same process that created the shortcut then tried to show the toast, it was simply not shown (but no return value indicated any failure).

The work-around with which I came up is to wait three seconds after the shortcut is created, then spawn a new process (essentially the same executable with the same command-line parameters). If no shortcut had to be created, this work-around is not necessary.

Of course, this is an ugly work-around to the problem. I do not even understand why this problem exists, otherwise a more elegant solution would probably be obvious.

Any ideas?

This PR builds on top of https://github.com/mohabouje/WinToast/pull/13.

dscho commented 6 years ago

@mohabouje so merging this to the new develop branch makes me think that you also have the feeling that this is not quite the way to go.

Any ideas how to go about it?

dscho commented 6 years ago

Actually, I have more information. After extensive testing, it would seem that my console application has to sleep for at least 3 seconds (sometimes even 5) after the shortcut was created to make the toast visible.

It would appear that this sleep is also necessary if the AppID of the shortcut had to be changed.

And it also is necessary in my tests if I run the DesktopToastsSample and click the button very quickly after the window is shown for the first time...

In real world scenarios, WinToast would be initialized at startup (and create the shortcut if necessary), and then notify the user much later, so it would not matter. It would only matter that this happens when 1) running the first time, and 2) trying to show a toast right away.

mohabouje commented 6 years ago

Hi,

Nice work! I love the idea to have actions, I started a few weeks ago a template to add text inputs, buttons, etc. I dint have time to finish it, but you made all-most all the work.

I accepted the pull request in the develop branch to clean together possible bugs and to kept the master branch clean, this is a good candidate for a new release. So, let’s use master branch only for new stable release.

I'm not worry about the problem with the link creation, actually the toast should be launched when the app is ready, and normally it takes more than a few seconds.

The problem is that the actions, the new feature, are not supported in Windows 8 or 8.1, and this library should be compatible with this older version.

So we need to ensure that everything works fine in older version and add the required flags to check the OS Version. I will commit some changes during this week. It's good to know that there are people interested in good contributions to this tinny but useful library,

Best regards =)

dscho commented 6 years ago

Awesome!

BTW we could also simply develop together on my PR branch (I always mark my PRs with "allow maintainer to edit", meaning that you can push into my branch in my repository as long as the PR is open).

As to Windows 8 or 8.1: good point, I managed to forget that I wanted to test that (read: to test what happens on those platforms, as they clearly do not support actions).

BTW I want to use this console example as a building block for Git for Windows' updater... And it would be awesome if I could integrate it soon (even if it is just a preliminary version, not the final stable release). My hope is that I can release a new Git for Windows with an updater that displays toast notifications whenever possible (falling back to the modal dialog it shows right now) on Monday, Nov 27th (or soon thereafter).

dscho commented 6 years ago

Oh, and I had another idea how to go about working around this shortcut business: instead of what I did in this PR, I want to introduce an option to the command-line example, say, -only-create-shortcut, that does what it says: it will not show any toast (and will actually error out if you provide any image or text) but ensure that the shortcut exists. Its exit code should even indicate whether there already was a shortcut (smallest yet-unused positive exit code) or whether it did create/adjust one (exit code: 0).

What do you think of that idea?

mohabouje commented 6 years ago

Sounds great!

I like the idea, and it could be a good approach right now, specially if you want it soon for the new release. We can just force the user to launch the exe file one time to create the shortcut.

I will test the actions system right now in Windows 8 & Windows 8.1 to analyse the behaviour. I will come back with some feedback!

mohabouje commented 6 years ago

I will work in your branch =)

mohabouje commented 6 years ago

Sorry, I forgot to clone your repo and work in your branch! Actually, I think that I dont know how to do it. In any case, the branch develop is ready to be tested and posible merged with master. It takes me a while to find the way to get the current Windows versión without manifest.

In any case, the app will launch the actions only if the app runs in Windows 10, so same code will work in all platforms. I add some print_help functionality and I change the command style to a generic way in the example application. Dont hesitate to change something. See the readme in the folder for details.

Please sync your repo and merge develop with yours, if you find any problema, just do a pull!

Have a nice day, and best regards!

dscho commented 6 years ago

Oops, I saw this only now... Will have a look!

dscho commented 6 years ago

It takes me a while to find the way to get the current Windows versión without manifest.

I think the canonical way is to use GetVersion(). Sure, it uses the version from the manifest, but you can declare multiple versions with which the executable is compatible. See e.g.

https://github.com/git-for-windows/git/blob/master/compat/win32/git.manifest#L11-L24

dscho commented 6 years ago

I see that you included Windows 8.1 and 10 in the manifest, but somehow it did not work?

Overall, I like the direction where you went in develop; I would just use interactive rebase extensively to "tell a story with the commits".

How do you want to proceed from here? Do you want me to reimplement some stuff from develop, on top of master? Or would you prefer to do it yourself?

mohabouje commented 6 years ago

It's done! There is no need to use manifest with the alternative one =)

It seems to work well in Windows 7, 8.1 & 10, with the same code. The actions are showed only in Windows 10, so, please if you have the chance to test it =)

Now, the master branch has all your changes & mine. =)

dscho commented 6 years ago

Will do! Thanks!

dscho commented 6 years ago

Will do! Thanks!

Yes, works as advertised! Perfect!