bisq-network / bisq2

GNU Affero General Public License v3.0
171 stars 64 forks source link

Check if OS notifications work correctly on Windows #1411

Closed HenrikJannsen closed 8 months ago

HenrikJannsen commented 10 months ago

On Windows we use AwtNotifications. We should figure out if there is a native solution which works well on all commonly used Windows versions. If not feasible we need to check if AwtNotifications works sufficiently well.

mrblue313 commented 10 months ago

I can look into this.

HenrikJannsen commented 9 months ago

Depends on https://github.com/bisq-network/bisq2/issues/1383

HenrikJannsen commented 9 months ago

@mrblue313 Any update on that task?

mrblue313 commented 9 months ago

I am working on it. Will give an update in the following days.

mrblue313 commented 9 months ago

Giving an update on the current status of this.

Testing the current solution

I have been testing using Clear-net and Windows 11.

The following matrix was considered:

These were my findings:

image

For instance, in the case of the offerbook, this is what it displays (not very indicative):

image

image

Same when you click on the three dots icon:

image

And in the system settings:

image

image

image

... and even the notification bell lights up!

image

Other things I have noticed (outside the notification framework layer, but still find worth mentioning):

To compare, this is an example of how notifications show in Element. They have the user image, longer text, etc.

image

Investigation

Suggested next steps

I suggest I test again when the binary is ready to be generated and revaluate the situation. But if we are able to set the app name correctly + image, and with some enhancements on the title/message that we pass adhering to the constraints seen, I would continue using AWT.

Another thing I am concerned about is that Windows Toast Notifications is only available from Windows 10, so if we want to ensure compatibility with any Windows version, that would still not be a good solution.

HenrikJannsen commented 9 months ago

Events > Conferences

Yes agree.

I would like to test this with the binary as well.

It should work now to build a binary for windows. ./gradlew :desktop:desktop-app-launcher:generateInstallers Then the Bisq icon and "Bisq" should be shown.

The body/message gets truncated very early.

Yes we should increase maxLenght

When a message is edited, users get notified about it again. Is this by design?

Not by design. Probably better to not send a notification in that case, but low prio to fix IMO.

If you choose "Ignore user", all their messages disappear everywhere. Seems like this option should be given to the user a bit more carefully, possibly involving two clicks and explaining what this means and how to undo. While testing, I was just expecting not to see this user's notifications.

We could show a popup (with dont show again) with more info. When un-ignore all msg are shown again. Its just a local filtering.

..user image...

That would be nice but I think thats a challenging task to get on all OS working. You could add an issue as a future feature request.

Thanks for all the testing. Can you try if you can get a windows binary running to see if icon / name works as expected? And can you make a PR for the minor changes with lenngth and nav path improvements? A util for getting a display string from the nav path would be good to be reused in other app-scope areas (and supported by translations). We have all the nav paths, so its just to combine those where we want to show the parent path.

mrblue313 commented 9 months ago

Sure, I'll get back with this.

mrblue313 commented 9 months ago

I have tested with the binary. Presenting the findings.

After generating the binary, I am taking the installer from: bisq2\apps\desktop\desktop-app-launcher\build\packaging\jpackage\temp_exe\images

It looks like the content in the folders inside images is swapped? in the win-exe.image is the .msi and in win-msi.image is the .exe.

I have tried both of them. The Bisq 2.exe seems to be a portable version. However, non of them show the image correctly in the notification. Besides that, as application name when receiving the notification, description is being picked up instead of name: https://github.com/bisq-network/bisq2/blob/0d05a7e74ab081f7a353d2854511398769db42a6/build-logic/packaging/src/main/kotlin/bisq/gradle/packaging/jpackage/PackageFactory.kt#L44 This only happens in the notification panel, the rest of places get "Bisq2".

image

I have also noticed that we are using a different (old) version of the logo in the task bar. This looks inconsistent with the one appearing in the process window, which is the newer one. I can update those icons in a PR soon.

image

Not sure if related to our *.msi in particular, but the installation using the Windows installer works really bad. I am not able to close the window, it seems to be hung up and need to kill the process once the app has been installed.

One more thing to consider, and that I have noticed, (independent from the build itself) is the flushing of messages when the user connects/starts up the app. At the moment, the user needs to wait/dismiss for every single notification. If the messages are more than 3 o 4, that might not feel like a good experience. I have created an issue for this #1546.

HenrikJannsen commented 9 months ago

@mrblue313 I assume you have build the installer with ./gradlew :desktop:desktop-app-launcher:generateInstallers, right?

Can you try to fix the issues with the image and title? If no solution found @alvasw might have an idea.

mrblue313 commented 9 months ago

I assume you have build the installer with ./gradlew :desktop:desktop-app-launcher:generateInstallers, right?

Yes, that's what I used.

Can you try to fix the issues with the image and title?

Sure, I will continue looking into it.

alvasw commented 9 months ago

Do you know when did it worked last (with the installed binary)? That would help to identify the commit that broke the notifications.

mrblue313 commented 9 months ago

Do you know when did it worked last (with the installed binary)? That would help to identify the commit that broke the notifications.

I have never seen it working before. The first time I tested it was around the 16th of Dec.

alvasw commented 9 months ago

Do you know when did it worked last (with the installed binary)? That would help to identify the commit that broke the notifications.

I have never seen it working before. The first time I tested it was around the 16th of Dec.

Oh my bad, I thought it worked before we updated the packaging module. @HenrikJannsen Have the notifications ever worked on Windows? I haven't read or worked on notification code before.

HenrikJannsen commented 9 months ago

I never managed to build a binary for Windows. So I could not test it with a binary to see if the icon and app name is correct. When running from source the notifications itself worked but showed the java icon.