[flutter_local_notifications] [WIP] Windows implementation of notifications #1473

kennethnym · 2022-02-09T22:07:13Z

This draft PR contains Windows implementation of this plugin. Still a WIP.

azchohfi · 2022-02-10T01:43:56Z

Hey @kennethnym, I've cloned your branch and built the source, and a simple plain notification worked totally fine for me.
The problem with your particular code is the template=ToastGeneric. If you switch that to, for example, ToastText01, then it works fine.
This value needs to be one of the valid TileTemplateType

kennethnym · 2022-02-10T08:59:45Z

I tried using the default templates but it would just show "New notification" instead of the actual body.

kennethnym · 2022-02-10T18:34:44Z

@azchohfi To illustrate, this is the code that I have at the moment:

const auto doc = winrt::Windows::UI::Notifications::ToastNotificationManager::GetTemplateContent(winrt::Windows::UI::Notifications::ToastTemplateType::ToastText01);
const auto nodes = doc.GetElementsByTagName(L"text");
nodes.Item(0).AppendChild(doc.CreateTextNode(winrt::to_hstring(title)));

winrt::Windows::UI::Notifications::ToastNotification notif{ doc };
const auto notifier = winrt::Windows::UI::Notifications::ToastNotificationManager::CreateToastNotifier(L"com.dexterous.example");

notifier.Show(notif);

and this is the toast shown:

title is 'plain title', but it is not shown in the notification.

azchohfi · 2022-02-10T20:52:25Z

Oh, I see. The parameters are being sent from dart to C++, but your GetString method is not querying them properly.
You should wrap the key inside an EncodableValue:

	const auto pair = m->find(flutter::EncodableValue(key));

Also, when you call the GetString method, you should store it not using a reference(&):

		const auto body = Utils::GetString("body", args).value();

You can use Visual Studio 2022 or 2019 to attach to the running exe and debug the C++ code. There is also a solution at "flutter_local_notifications\flutter_local_notifications\example\build\windows\example.sln" that will have the reference to all the files so you can add breakpoints, etc.

I've also found another issue. You need to add the id's to the texts, like this:

		XmlDocument doc;
		doc.LoadXml(L"\
			<toast>\
				<visual>\
					<binding template=\"ToastText01\">\
						<text id=\"1\">abc</text>\
						<text id=\"2\">def</text>\
					</binding>\
				</visual>\
			</toast>");

You can also debug this using cout:

std::cout << winrt::to_string(doc.GetXml()) << std::endl;

I've tested this changes here and now the notifications shows properly:

Let me know if this helps!

kennethnym · 2022-02-10T21:31:26Z

Ahh thanks, that makes sense now. I am not a C++ programmer, so I really appreciate your help!

lzzy12 · 2022-02-14T10:10:03Z

Hi, is it working as expected now after your latest commits?

kennethnym · 2022-02-14T10:18:04Z

Basic title body notification is working, but nothing beyond that.

kennethnym · 2022-02-14T11:43:14Z

@lzzy12 Just an update: I have implemented cancel and cancelAll in the latest commit.

MaikuB · 2022-02-21T09:13:29Z

FYI this reminds me that zonedSchedule currently part of the platform interface but should be so that will happen soon. Note that there's a 10.0.0 branch right now for the 10.0.0 prerelease

Edit: had trouble adding zonedSchedule to platform interface as overriding the method and having a required parameter isn't possible

azchohfi · 2022-03-01T19:08:56Z

@kennethnym any updates on this PR?

kennethnym · 2022-03-01T19:14:31Z

Hi sorry, busy with coursework at the moment, I will continue the work this weekend.

kennethnym · 2022-03-05T14:21:28Z

Just a bit of progress update: I am currently in the process of implementing onSelectNotification callback.

Update: onSelectNotification now works, but only when the program is active.

kennethnym · 2022-03-06T18:36:55Z

At the moment, onSelectNotification is only called once. After a toast is clicked and the callback is invoked, for some reason Windows would not invoke the callback anymore whenever subsequent toasts are clicked on. I am investigating what is going wrong.

kennethnym · 2022-03-23T22:02:20Z

Sorry for the silence, I am currently busy with courseworks. At the moment I am still stuck with the issue above. I would really appreciate it If anyone can help me with it.

azchohfi · 2022-03-28T21:33:05Z

Hello @kennethnym. I've tried your branch and I managed to make it work more than once. Cherry-pick this commit from my fork:
azchohfi@d363e5c

sidevesh · 2022-04-03T15:10:41Z

@kennethnym I tried your WIP branch and had some issues with notifications and msix packaging,
@azchohfi found the culprit,
YehudaKremer/msix#118 (comment) something to keep in mind.
Looks like we might have to use the correct CreateToastNotifier method based on if the app is packaged or not.

kennethnym · 2022-04-03T15:56:27Z

Thanks for bring that to attention, I'll investigate how to handle msix apps as well.

kennethnym · 2022-04-13T21:04:11Z

@sidevesh Can you test if the latest commit fixes your issue?

sidevesh · 2022-04-18T16:49:23Z

I get the following error when building an app with the latest commit:

C:\Users\test\Projects\Side-projects\tomatoro\tomatoro-app\windows\flutter\ephemeral\.plugin_symlinks\flutter_local_notifications\windows\flutter_local_notifications_plugin.cpp(224): error C2220: the 
following warning is treated as an error [C:\Users\test\Projects\Side-projects\tomatoro\tomatoro-app\build\windows\plugins\flutter_local_notifications\flutter_local_notifications_plugin.vcxproj]      
C:\Users\test\Projects\Side-projects\tomatoro\tomatoro-app\windows\flutter\ephemeral\.plugin_symlinks\flutter_local_notifications\windows\flutter_local_notifications_plugin.cpp(224): warning C4715: '`anonymous namespace'::FlutterLocalNotificationsPlugin::Initialize': not all control paths return a value [C:\Users\test\Projects\Side-projects\tomatoro\tomatoro-app\build\windows\plugins\flutter_localBuild process failed.
 
#0      WindowsBuild.build (package:msix/src/windowsBuild.dart:33:7)
<asynchronous suspension>
#1      Msix._createMsix (package:msix/msix.dart:73:7)
<asynchronous suspension>
#2      Msix.create (package:msix/msix.dart:33:5)
<asynchronous suspension>
#3      main (file:///C:/Users/test/AppData/Local/Pub/Cache/hosted/pub.dartlang.org/msix-3.3.1/bin/create.dart:4:3)
<asynchronous suspension>
pub finished with exit code 255

kennethnym · 2022-04-19T11:49:26Z

Thanks for reporting, it's an easy fix.

kennethnym · 2022-04-19T11:57:01Z

@sidevesh the issue should be fixed.

sidevesh · 2022-04-25T06:07:00Z

Hey @kennethnym , after the latest commit, I am able to build the app but notification has stopped showing up now.
I had actually made a simple patch earlier replacing the CreateToastNotifier with the correct version and had gotten it to work with the app name showing up correctly, so we should be close to a solution.
I encountered another issue which is that on clicking the notification it opens another copy of the app rather than bring the app to focus or dismiss.
In macOS and Linux, clicking the notification without any on SelectNotification just dismisses the notification and brings the app to focus and I am trying to replicate that on Windows.
Is it possible to implement that behavior ?

kennethnym · 2022-04-26T18:10:42Z

after the latest commit, I am able to build the app but notification has stopped showing up now.

I had actually made a simple patch earlier replacing the CreateToastNotifier with the correct version and had gotten it to work with the app name showing up correctly, so we should be close to a solution.

I encountered another issue which is that on clicking the notification it opens another copy of the app rather than bring the app to focus or dismiss.

Thanks for reporting, I'll look into this.

In macOS and Linux, clicking the notification without any on SelectNotification just dismisses the notification and brings the app to focus and I am trying to replicate that on Windows.

It should be possible to implement this behavior on Windows as well.

kennethnym · 2022-05-04T17:52:41Z

@sidevesh From the issue you linked earlier, @azchohfi mentioned:

For deciding whether to use the overloaded method...
HasIdentity true = CreateToastNotifier()
HasIdentity false = CreateToastNotifier(myAumid)"

I have in fact integrated this into the code in previous commits already:

if (hasIdentity.value())
    toastNotifier = winrt::Windows::UI::Notifications::ToastNotificationManager::CreateToastNotifier();
else
    toastNotifier = winrt::Windows::UI::Notifications::ToastNotificationManager::CreateToastNotifier(winrt::to_hstring(aumid));

Is the app name still not showing up correctly?

As for the other issues, I still need some time to figure out what's going on that is causing them to occur.

sidevesh · 2022-05-14T13:06:16Z

hey @kennethnym , tried it again and same behavior, notifications have stopped showing up,
so can't really see if the app name in notification is fixed or not.

sidevesh · 2022-05-14T13:08:08Z

Like I mentioned previously I had simply replaced the CreateToastNotifier with the correct version and managed to fix the app name,
although that would mess up non msix apps so your fix with the check built in is the proper solution here.

kennethnym · 2022-05-14T13:11:33Z

I am struggling to understand why it would stop working altogether. Exams are coming up for me; I'll start working on it again after exams.

sidevesh · 2022-05-14T13:13:50Z

Also, any pointers about the issue: on clicking the notification it opens another copy of the app rather than bring the app to focus or dismiss ?

kennethnym · 2022-05-14T13:15:00Z

I have no idea what is causing that issue either. It wasn't happening before, and I never changed the code related to that.

sidevesh · 2022-05-14T13:19:49Z

I have no idea what is causing that issue either. It wasn't happening before, and I never changed the code related to that.

Okay, I will try testing this in a non msix build to see if msix packaging is causing the difference in behavior, might help us debug this issue.

DomingoMG · 2022-06-12T13:14:56Z

I'm looking forward to using this on Windows, Is it a long way from launch?

kennethnym · 2022-07-04T07:59:27Z

Sorry for the silence. I've been very busy lately, so I was not able to squeeze time to work on this. I will give this another look over the weekend.

keanallen · 2022-09-14T08:35:40Z

Hello, guys

lightrabbit

I found some parts in the code that could be improved, could you please take a look?

lightrabbit · 2022-09-14T13:12:18Z

flutter_local_notifications/windows/flutter_local_notifications_plugin.cpp

+
+		const auto bindingNode = doc.SelectSingleNode(L"//binding[1]");
+
+		if (title.has_value()) {


According to this link: https://docs.microsoft.com/en-us/windows/apps/design/shell/tiles-and-notifications/adaptive-interactive-toasts?tabs=xml
Maybe we can pass the XML text directly to the plugin and then combine the title and content etc into XML text on the Dart code side?
In this way, more Toast styles can be easily supported in the future.

That is something that I have thought about actually - a Windows specific method that allows users to pass in XML of the notification toast directly.

lightrabbit · 2022-09-14T13:15:41Z

flutter_local_notifications/windows/registration.cpp

+/// <summary>
+/// String representation of the callback GUID.
+/// </summary>
+const std::string CALLBACK_GUID_STR = "{68d0c89d-760f-4f79-a067-ae8d4220ccc1}";


According the resources doc in UpdateRegistry. It looks like every application should have a unique GUID, so the GUID here should be configurable. Maybe change the GUID to be passed from the initialization parameter instead?

Gotcha, will fix.

lightrabbit · 2022-09-14T13:21:36Z

flutter_local_notifications/windows/flutter_local_notifications_plugin.cpp

+
+		UINT32 length;
+		auto err = GetCurrentPackageFullName(&length, nullptr);
+		if (err != ERROR_INSUFFICIENT_BUFFER) {


When I run the example here, it returns the error code 87(ERROR_INVALID_PARAMETER) which causes the initialization to fail. But when I still return false(not std:nullopt) here, the plugin still works fine.

I found the cause of the problem. The variable length needs to be initialized.
Replace UINT32 length; to UINT32 length = 0; could resolve this issue.

Wow! Can't wait! Thank you guys for working on this.

kennethnym · 2022-09-15T12:27:10Z

Quick update - I recently switched to working on a MacBook fully as I will be away from my main computer for an extended period. Parallel seems to be a good option for running Windows on a MacBook. As soon as I get that up and running I should be able to start working on this (longrunning) PR, addressing @lightrabbit 's helpful comments as well.

tenraneko · 2022-11-17T14:09:09Z

Hello everybody!
Sorry, I still do not understand how to fix the error with the wrong app name in notifications 😢

kennethnym · 2022-11-18T09:14:47Z

I think this is a known issue at the moment. Unfortunately I don't have the time at the moment to look into this, but when I do I will see what's going I'll see what I can do to fix it!

xBLACKICEx · 2024-05-21T14:12:56Z

hi @kennethnym is there any news for support on windows ？

kennethnym added 3 commits February 9, 2022 10:41

Start impl of Windows notifications

278f9b3

Remove dependency override

ac1cd51

Merge branch 'master' into windows-impl

962ca96

kennethnym added 7 commits February 10, 2022 21:40

Fix notification content not showing

e0370c0

Implement initialize method

7b5dfa3

WIP impl of cancelling notifications

1fd635e

Add support for optional title/body

1fad545

Impl Registry logic for notification handling

8c53f57

Tidy up code

b8beedd

Remove dependency overrides

34e187c

Implement cancel/cancelAll

5400c0b

EdGeraghty mentioned this pull request Feb 25, 2022

Desktop builds don't show notifications syphon-org/syphon#402

Open

WIP: Impl onSelectNotification

0c21c9d

Fix read access violation

d2f66e9

sidevesh mentioned this pull request Mar 21, 2022

Notifications have weird app name and possibly other issues when app is packaged as msix YehudaKremer/msix#118

Closed

kennethnym added 2 commits March 31, 2022 12:24

Pass payload to Dart on notification selected

57ecbf0

Remove melos files

0baa7db

Attempting to handle msix

5ad6464

Forgot to check for optional value

effca5a

Fix Initialize not returning value

31b1ee7

kennethnym mentioned this pull request May 28, 2022

Desktop support (Linux, Windows) #746

Open

2 tasks

lightrabbit reviewed Sep 14, 2022

View reviewed changes

lightrabbit mentioned this pull request Sep 27, 2022

merge to latest version & fix issues in code review kennethnym/flutter_local_notifications#1

Open


		const auto bindingNode = doc.SelectSingleNode(L"//binding[1]");

		if (title.has_value()) {

[flutter_local_notifications] [WIP] Windows implementation of notifications #1473

Are you sure you want to change the base?

[flutter_local_notifications] [WIP] Windows implementation of notifications #1473

Conversation

kennethnym commented Feb 9, 2022 • edited

azchohfi commented Feb 10, 2022

kennethnym commented Feb 10, 2022 • edited

kennethnym commented Feb 10, 2022

azchohfi commented Feb 10, 2022

kennethnym commented Feb 10, 2022

lzzy12 commented Feb 14, 2022

kennethnym commented Feb 14, 2022

kennethnym commented Feb 14, 2022

MaikuB commented Feb 21, 2022 • edited

azchohfi commented Mar 1, 2022

kennethnym commented Mar 1, 2022

kennethnym commented Mar 5, 2022 • edited

kennethnym commented Mar 6, 2022 • edited

kennethnym commented Mar 23, 2022

azchohfi commented Mar 28, 2022

sidevesh commented Apr 3, 2022

kennethnym commented Apr 3, 2022

kennethnym commented Apr 13, 2022

sidevesh commented Apr 18, 2022

kennethnym commented Apr 19, 2022

kennethnym commented Apr 19, 2022

sidevesh commented Apr 25, 2022

kennethnym commented Apr 26, 2022

kennethnym commented May 4, 2022

sidevesh commented May 14, 2022

sidevesh commented May 14, 2022

kennethnym commented May 14, 2022

sidevesh commented May 14, 2022

kennethnym commented May 14, 2022

sidevesh commented May 14, 2022

DomingoMG commented Jun 12, 2022 • edited

kennethnym commented Jul 4, 2022

keanallen commented Sep 14, 2022

lightrabbit left a comment

Choose a reason for hiding this comment

lightrabbit Sep 14, 2022

Choose a reason for hiding this comment

kennethnym Sep 15, 2022

Choose a reason for hiding this comment

lightrabbit Sep 14, 2022

Choose a reason for hiding this comment

kennethnym Sep 15, 2022

Choose a reason for hiding this comment

lightrabbit Sep 14, 2022

Choose a reason for hiding this comment

lightrabbit Sep 27, 2022

Choose a reason for hiding this comment

keanallen Sep 27, 2022

Choose a reason for hiding this comment

kennethnym commented Sep 15, 2022

tenraneko commented Nov 17, 2022

kennethnym commented Nov 18, 2022

xBLACKICEx commented May 21, 2024

kennethnym commented Feb 9, 2022 •

edited

kennethnym commented Feb 10, 2022 •

edited

MaikuB commented Feb 21, 2022 •

edited

kennethnym commented Mar 5, 2022 •

edited

kennethnym commented Mar 6, 2022 •

edited

DomingoMG commented Jun 12, 2022 •

edited