[RFC] Support tray menus (Windows) #1959

leaanthony · 2022-10-12T10:01:09Z

PR Goals

This PR has been opened to solicit feedback and discussion on system tray support.
The goals of this PR are:

Determine if feature set is complete enough for an initial release
Determine what likely gaps there are with the other platforms, e.g. Template icons for Mac, HiDPI alternative icons.
Get feedback on the experimental programmatic API and where it could be improved
Find bugs

What does this PR provide?

This PR provides the ability to add an icon to the Windows system tray area, with the option of adding a menu. The system tray has callbacks for both left and right buttons. By default, both these buttons will show the associated menu. Currently, this capability is provided by an experimental API that builds on what is currently available but provides a more hierarchical model for Wails applications.

An example application is provided in v2/examples/systray. It starts with the main window hidden. This can be made visible by left clicking the tray icon. Right clicking the tray icon shows the menu. The menu is a "kitchen sink" example of what is possible. There are plenty of comments in the example which detail more about what is possible with the system tray.

It should be noted that there is other code in here related to Mac trays. This is a WIP and should not be considered as part of this "PR".

How can I provide feedback?

Please comment on this PR with any feedback. Thank you! 🙏

Related Links

Support better tray menu API Move object creation to main thread

…port

# Conflicts: # v2/internal/frontend/desktop/windows/win32/consts.go

# Conflicts: # .gitignore # v2/internal/frontend/desktop/windows/win32/consts.go # v2/internal/frontend/devserver/devserver.go # website/docs/introduction.mdx

the option in the docs is the wrong flag you have to add a d at the end

…ctor assethandler/assetserver signatures.

Refactor ShellNotifyIcon. Application shutdown once.

teddywing

This looks cool!

I tried out and read over the example code in v2/examples/systray/main.go. The API looks straightforward.

There’s one UI behaviour that I was confused by. If I:

Open the menu by right-clicking the tray icon
Left-click the tray icon
The menu disappears and the window opens

Should the window be opening? I would have thought that the menu should close, and the window would only open if the menu wasn’t already open.

You asked about gaps with other platforms. In addition to the template icons you mentioned, NSStatusItems have a behavior property (https://developer.apple.com/documentation/appkit/nsstatusitembehavior?language=objc). It might be nice to add behaviours to provide additional interaction options for removing the NSStatusItem.

v2/pkg/options/systemtray.go

v2/internal/platform/systray/windows.go

teddywing · 2022-10-21T22:39:15Z

One more idea: Is there any way to animate the tray icon in order to indicate, for instance, that the application is making a network request?

leaanthony · 2022-10-22T09:39:18Z

Thanks @teddywing for taking the time to test this out and provide feedback 👍

There’s one UI behaviour that I was confused by. If I:

Open the menu by right-clicking the tray icon
Left-click the tray icon
The menu disappears and the window opens
Should the window be opening? I would have thought that the menu should close, and the window would only open if the menu wasn’t already open.

I'm not sure we can be 💯 sure this is the desired behaviour for all system trays. We do have the left and right click callbacks. Do you think this would be sufficient for a developer to produce the outcome you suggested?

You asked about gaps with other platforms. In addition to the template icons you mentioned, NSStatusItems have a behavior property (https://developer.apple.com/documentation/appkit/nsstatusitembehavior?language=objc). It might be nice to add behaviours to provide additional interaction options for removing the NSStatusItem.

👍

One more idea: Is there any way to animate the tray icon in order to indicate, for instance, that the application is making a network request?

You should be able to call SetIcon() from a Goroutine with a timer (I haven't tried it but it totally should work).

teddywing · 2022-10-22T15:41:06Z

I'm not sure we can be 💯 sure this is the desired behaviour for all system trays. We do have the left and right click callbacks. Do you think this would be sufficient for a developer to produce the outcome you suggested?

Yes you’re right, it makes more sense for these behaviours to be defined by the app developer. Are there APIs to know when the menu is visible and to hide & show it?

You should be able to call SetIcon() from a Goroutine with a timer (I haven't tried it but it totally should work).

Cool, that sounds good.

…events. Updated example.

leaanthony · 2022-10-26T08:48:12Z

Yes you’re right, it makes more sense for these behaviours to be defined by the app developer. Are there APIs to know when the menu is visible and to hide & show it?

There are now 👍

leaanthony · 2022-10-26T08:53:20Z

Considering it's been 2 weeks since this PR was opened, I propose we lock this API down for now and move on to Mac support.

stffabi

This looks awesome to me, awesome stuff 🚀.

I think the API is fine as it is, so from my side we can lock that down.

Furthermore I added some comments regarding Windows message loop specific things. It seems like we are getting a bunch of duplicate code between winc and the new platform/win32 package. Maybe we should someday take the opportunity to clean that up, maybe the best time when we do multi-window support.

stffabi · 2022-10-26T08:45:26Z

v2/internal/frontend/desktop/windows/win32/consts.go

-	procGetWindowRect        = moduser32.NewProc("GetWindowRect")
-	procGetMonitorInfo       = moduser32.NewProc("GetMonitorInfoW")
-	procMonitorFromWindow    = moduser32.NewProc("MonitorFromWindow")
+	moduser32                       = syscall.NewLazyDLL("user32.dll")


Should we start using windows.NewLazySystemDLL in order the prevent preloading attacks as mentioned in the docs of syscall.NewLazyDll?

IIRC there are a log of places with the old NewLazyDLL, maybe we could do this in a separate PR and go through the complete code base?

Yeah, agree. I think refactors should be their own PRs 👍 Why not open a ticket for it and we can work on that?

Yeah, will do that.

stffabi · 2022-10-26T08:47:12Z

v2/internal/frontend/desktop/windows/win32/consts.go

@@ -41,3 +51,217 @@ func IsWindowsVersionAtLeast(major, minor, buildNumber int) bool {
 		windowsVersion.Minor >= minor &&
 		windowsVersion.Build >= buildNumber
 }
+
+// http://msdn.microsoft.com/en-us/library/windows/desktop/aa373931.aspx
+type GUID struct {


Should we use windows.GUID instead?

Are they 1:1?

Yeah, it seems: https://cs.opensource.google/go/x/sys/+/refs/tags/v0.1.0:windows/types_windows.go;l=1698

stffabi · 2022-10-26T08:59:55Z

v2/internal/platform/systray/menu.go

+	"errors"
+	"fmt"
+	platformMenu "github.com/wailsapp/wails/v2/internal/platform/menu"
+	"github.com/wailsapp/wails/v2/internal/platform/win32"


Doesn't this break builds for non Windows targets currently, without any build constraint?

stffabi · 2022-10-26T09:03:35Z

v2/internal/platform/systray/windows.go

+	return result
+}
+
+func (p *Systray) Run() error {


Should we really have several MessageLoops running in the same application? Windows and e.g. .NET almost always runs only one MessageLoop on the main thread. I thank that would make it a lot easier to reason about on which thread one can access GUIs and not.

I did ask a win32 guy about this and he said that a message loop per thread is fine. What would moving this to a main loop look like? What if this is a tray only application with no window?

Yes, it is not a problem per se. I'm just used to having only one MessageLoop from my old .NET days 😄. A drawback would be to have several dedicated OS threads that are being run for every MessageLoop.

AFAIK there's no problem of running a message loop if there's no window, it should run without any window. Let's consider the following case, which I think would also be easier to reason about with just one message loop. One has an application with just a tray, for one click another window is created (in the multi-window case).

So the callback of the click is done in the dedicated systray os thread. If we now create a new window, we would need to span a go routine, create the window and start another message loop for that. Otherwise we would implicitly use the message loop of the systray.

Let's consider the same case with just one message loop on the main-thread which is dedicated to the UI. In that case the systray is run in that message loop, a click callback is also run in the main-thread, one can simply create a new window without needing to span a go routine. Show the window and the window is also run on the same main-thread, so do all the callbacks. That would probably also make it easier to interact with WebView2 if there are several windows, since all the COM initialisation would only have to be done once.

stffabi · 2022-10-26T09:04:31Z

v2/internal/platform/systray/windows.go

+}
+
+func (p *Systray) Run() error {
+	var msg win32.MSG


We might suffer here from the same problem as we had with our main message loop here: #1083

Oooph that is a monumental catch 👍

stffabi · 2022-10-26T09:06:56Z

v2/internal/platform/systray/windows.go

+
+func (p *Systray) Run() error {
+	var msg win32.MSG
+	for {


Don't we need to lock down the OS thread in here? There's no guarantee that if run in a Go routine, all calls will be executed on the same OS thread. And we might then pull messages from the wrong OS thread or miss some events if it's not run on the message loop that created the systray.

stffabi · 2022-10-26T09:09:05Z

v2/internal/platform/win32/structs.go

+	HBalloonIcon     HICON
+}
+
+type GUID struct {


IIRC this is a duplicate definition in another package, maybe also using windows.GUID?

stffabi · 2022-10-26T09:10:25Z

v2/internal/platform/win32/structs.go

@@ -0,0 +1,54 @@
+package win32
+
+type NOTIFYICONDATA struct {


IIRC this is a duplicate definition with a struct in another package, should we unify this?

stffabi · 2022-10-26T09:15:09Z

v2/internal/platform/win32/winres.go

+
+import "unsafe"
+
+func MakeIntResource(id uint16) *uint16 {


I think using this one will make problems with the race detector as seen in: #1554

stffabi · 2022-10-26T09:18:59Z

v2/pkg/application/systray.go

+	}
+}
+
+func (t *SystemTray) run() {


Don't we have to lock down the OS thread here? Since Windows will send the messages of the systray to the message loop of the OS thread that created the window. So we would have to make sure all of the creation of the SysTray will be done on the same OS thread as t.impl.Run() is run.

This is OS specific for windows, so we might need to put that into platform specific code behind a build tag.

leaanthony · 2022-10-26T09:48:20Z

The winc/win32 thing I should have made clearer: everything in platform is the target from after we merge this PR. Eventually, as we migrate from frontend we move everything we need into platform/win32 and drop both windows/win32 and the winc directory. I'm looking at frontend/win32 exposing a slightly higher interface than just a 1:1 api onto win32 so I wanted to "port" that over as needed. I hate the duplication but in my head they are different things 😅

stffabi · 2022-10-26T10:12:05Z

The winc/win32 thing I should have made clearer: everything in platform is the target from after we merge this PR. Eventually, as we migrate from frontend we move everything we need into platform/win32 and drop both windows/win32 and the winc directory. I'm looking at frontend/win32 exposing a slightly higher interface than just a 1:1 api onto win32 so I wanted to "port" that over as needed. I hate the duplication but in my head they are different things 😅

Ah great, thanks for clarifying 🙏

teddywing · 2022-10-26T20:40:44Z

Yes you’re right, it makes more sense for these behaviours to be defined by the app developer. Are there APIs to know when the menu is visible and to hide & show it?

There are now 👍

Nice, thanks!

leaanthony · 2022-12-05T21:29:36Z

Closing in favour of #2181 - Thanks for the feedback 👍

leaanthony and others added 30 commits April 30, 2022 12:00

Initial Tray support for Mac

3f82cea

Support activation policy

ee50170

Better Checked implementation

1830c64

Merge branch 'master' into exp

c1d4aeb

Move TrayMenu from config to runtime

ae0933c

Support better tray menu API Move object creation to main thread

Support retina images

58b6e4c

Support sizing tray menu

13fb535

Hugely improved + darkmode bitmap support. Dynamic scaling factor sup…

1947a9a

…port

Add SetLabel, minor api updates

ee5ca1a

WIP

368733b

Support loading images into Windows tray

319d592

Merge branch 'master' into exp

11cbfb6

# Conflicts: # v2/internal/frontend/desktop/windows/win32/consts.go

Merge master

a11655c

Application refactor

823c365

Merge branch 'exp' into feature/1521_Support_Tray_Menus

e5a1ddb

# Conflicts: # .gitignore # v2/internal/frontend/desktop/windows/win32/consts.go # v2/internal/frontend/devserver/devserver.go # website/docs/introduction.mdx

Add issue translator (#1891)

26c6881

chore: update the label name in the issue template (#1893)

022fd9a

obfuscated instead of obfuscate in the docs (#1895)

2940a9e

the option in the docs is the wrong flag you have to add a d at the end

[assetHandler] Remove redundant log prefix (#1896)

cfb55f7

[windows] refactor out main loop. Create new application struct. Refa…

311dde4

…ctor assethandler/assetserver signatures.

Refactor darwin app

d319d51

Refactor app for linux

03dd514

Initial Windows systray support

bf088ea

Refactor light/dark icons

a967ce0

Move systray startup into application

c63f816

Support menu.Menu

68bdc7c

Support traymenu update

abe83f2

Fix GetMessage bug

0664c51

Fix GetMessage bug.

249dc81

Refactor ShellNotifyIcon. Application shutdown once.

Support submenus

181ba42

leaanthony added 2 commits October 12, 2022 21:04

Merge branch 'master' into feature/1521_Support_Tray_Menus

62c19f1

Merge branch 'master' into feature/1521_Support_Tray_Menus

312f1a6

leaanthony requested review from stffabi and removed request for stffabi October 13, 2022 09:10

leaanthony added the Ready For Testing A fix is available and needs testing label Oct 13, 2022

leaanthony added 2 commits October 13, 2022 20:11

Merge branch 'master' into feature/1521_Support_Tray_Menus

97ca838

Merge branch 'master' into feature/1521_Support_Tray_Menus

d7db4c1

teddywing reviewed Oct 21, 2022

View reviewed changes

v2/pkg/options/systemtray.go Outdated Show resolved Hide resolved

v2/internal/platform/systray/windows.go Outdated Show resolved Hide resolved

Merge branch 'master' into feature/1521_Support_Tray_Menus

7969419

Merge branch 'master' into feature/1521_Support_Tray_Menus

58be270

leaanthony added 2 commits October 23, 2022 10:11

Merge branch 'master' into feature/1521_Support_Tray_Menus

6dac029

Add support for double click mouse button events and menu open/close …

ca07910

…events. Updated example.

Merge branch 'master' into feature/1521_Support_Tray_Menus

78e3132

stffabi reviewed Oct 26, 2022

View reviewed changes

leaanthony mentioned this pull request Oct 29, 2022

Support Tray Menus (Windows) #1521

Closed

leaanthony added 2 commits November 25, 2022 07:17

Merge branch 'master' into feature/1521_Support_Tray_Menus

f469d3e

Address some issues in Code Review

4b11bed

asabya mentioned this pull request Nov 25, 2022

Add tray menu fairDataSociety/fairdrive-desktop-app#56

Open

leaanthony mentioned this pull request Dec 5, 2022

Windows tray menus #2181

Merged

leaanthony closed this Dec 5, 2022

W1M0R mentioned this pull request Sep 18, 2023

How to use system tray #2821

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Support tray menus (Windows) #1959

[RFC] Support tray menus (Windows) #1959

leaanthony commented Oct 12, 2022 •

edited

Loading

teddywing left a comment

teddywing commented Oct 21, 2022

leaanthony commented Oct 22, 2022 •

edited

Loading

teddywing commented Oct 22, 2022

leaanthony commented Oct 26, 2022

leaanthony commented Oct 26, 2022

stffabi left a comment

stffabi Oct 26, 2022

leaanthony Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

leaanthony Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

leaanthony Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

leaanthony Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

stffabi Oct 26, 2022

leaanthony commented Oct 26, 2022

stffabi commented Oct 26, 2022

teddywing commented Oct 26, 2022

leaanthony commented Dec 5, 2022

[RFC] Support tray menus (Windows) #1959

[RFC] Support tray menus (Windows) #1959

Conversation

leaanthony commented Oct 12, 2022 • edited Loading

PR Goals

What does this PR provide?

How can I provide feedback?

Related Links

teddywing left a comment

Choose a reason for hiding this comment

teddywing commented Oct 21, 2022

leaanthony commented Oct 22, 2022 • edited Loading

teddywing commented Oct 22, 2022

leaanthony commented Oct 26, 2022

leaanthony commented Oct 26, 2022

stffabi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leaanthony commented Oct 26, 2022

stffabi commented Oct 26, 2022

teddywing commented Oct 26, 2022

leaanthony commented Dec 5, 2022

leaanthony commented Oct 12, 2022 •

edited

Loading

leaanthony commented Oct 22, 2022 •

edited

Loading