dustinkredmond / FXTrayIcon

Tray Icon implementation for JavaFX applications. Say goodbye to using AWT's SystemTray icon, instead use a JavaFX Tray Icon.
MIT License
324 stars 25 forks source link

No way to detect mouse button/number of clicks on tray icon #41

Closed dwalluck closed 2 years ago

dwalluck commented 2 years ago

If I understand correctly, the only API made available for attaching an event to the tray is setOnAction() (onAction() with the Builder). This takes an ActionEvent, not a MouseEvent which seems strange. In particular, it interferes with the right click on the tray icon because the event is always fired regardless of which button is used or clicks used. For example, how do you fire something only on a left click and not on the right click/context menu?

dwalluck commented 2 years ago

Looking at the code, there are some private methods that may be useful. In particular, the getPrimaryClickListener() seems to fire on any type of click (if "primary" is supposed to refer to the first mouse button).

schattenlord92 commented 2 years ago

Not sure if this should be a new issue, but it would be great for the MenuItems as well.

dwalluck commented 2 years ago

ActionEvent is supposed to be used for menus, though. Menus normally are supposed to do the same action on left and right click.

The problem with using and ActionEvent in place of a MouseEvent is like I said: right clicking to bring up the context menu will also trigger the icon action, which can't be what is intended.

dustinkredmond commented 2 years ago

@dwalluck It's even more fun when you have to account for the OS that the library is being run on. For example, what equates to a primary mouse click on Windows does not always equate to a primary mouse click on MacOS or some Linux distros. That's AWT to blame rather than the FXTrayIcon implementation. Even more of a surprise was that different versions of the same OS treated events differently.

When I initially wrote FXTrayIcon, I noticed this when comparing Windows and OSX. Rather than try to cater to all OSes that a user might use FXTrayIcon on, I tried to create a good general implementation that would be more or less uniform across the platforms.

We could go down a very deep rabbit hole trying to make this feature (MouseEvent handling) work for all operating system choices. I will leave this Issue open for the time being, but unless someone has a good solution for a fix that will work for most OSes, and in a predictable way, I will probably close after some time.

Unfortunately, we have a lot of limitations imposed by AWT and the way that it behaves on different platforms. I welcome contributions that aim to provide this functionality, but I, myself do not have a good solution in mind that works cross-platform.

dwalluck commented 2 years ago

Help me to understand what the issue is. Naively, without having studied the code, I would have expected you to map a JavaFX action event to AWT action and a JavaFX mouse event to AWT mouse. I don't understand why, if you want to add an ActionEvent to the tray icon, you don't use addActionListener. Instead, setOnAction adds a mouse listener.

Also, I wasn't sure about this code and whether it could somehow interfere with the user-added action because I think it's always present.

// Show parent stage when user clicks the icon
this.trayIcon.addActionListener(stageShowListener);

What I wanted to do was add an action that shows/hides on mouse click. But, right click should not modify the shown state, only bring up the popup menu. I don't think we can use isPopupTrigger() to workaround just that issue since that is only on mouse pressed/released and not mouse clicked.

EasyG0ing1 commented 2 years ago

@dwalluck Hello,

I tried to create another event handler method that assigned a MouseEvent to the awt.TrayIcon object, but when I tried to assign the event using

trayIcon.addMouseListener(getPrimaryClickListener(e))

The IDE said that awt.TrayIcon requires an ActionEvent to be passed into the listener. So it won't actually take a MouseEvent as an argument... this is an AWT limitation.

Or am I missing something?

Edit: Having read further in the discussion, I think I'm understanding what you're getting at. I have very little experience with AWT having cut my teeth on Java purely in an FX context ... I'm going to look into what you suggested as far as mapping an FX to an AWT mouse / action event and see if I can make sense of that ... this could potentially lead to a solution for multiple issues that have been brought up because of AWT limitations ... but I won't know until I can wrap my head around it.

dwalluck commented 2 years ago

The IDE said that awt.TrayIcon requires an ActionEvent to be passed into the listener. So it won't actually take a MouseEvent as an argument... this is an AWT limitation.

I think this is just because getPrimaryClickListener returns ActionEvent, but it could return MouseEvent. The problem I saw is there's no way to construct a new MouseEvent from EventHandler<MouseEvent>, but there must be a way to do it.

dwalluck commented 2 years ago

Edit: Having read further in the discussion, I think I'm understanding what you're getting at.

java.awt.TrayIcon has a method addMouseListener which I think should be used instead of addActionListener. Just as we have a java.awt.event.ActionEvent and a javafx.event.ActionEvent, there is a java.awt.event.MouseEvent and a javafx.scene.input.MouseEvent.

dwalluck commented 2 years ago

Also, I have verified that having this.trayIcon.addActionListener(stageShowListener) always there will interfere with the user's own action. Maybe it needs to be removed if a customer handler is added and the developer is expected to add the functionality themselves in that case.

EasyG0ing1 commented 2 years ago

@dwalluck

and the developer is expected to add the functionality themselves in that case.

This is technically already an option, no? Simply don't engage the library's methods and then create those as needed. Reflection is a wonderful thing ☺

dwalluck commented 2 years ago

This is technically already an option, no? Simply don't engage the library's methods and then create those as needed. Reflection is a wonderful thing ☺

My point is I believe that code is always called. It makes sense as a default, but interferes with the when you want to add a custom handler.

dustinkredmond commented 2 years ago

This should be possible... I will check this and see if there's a way that we can implement this in the next major version release.