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

Some small fixes and one added method #3

Closed EasyG0ing1 closed 3 years ago

EasyG0ing1 commented 3 years ago

Fixed:

isUnique() method:
    From    if (this.popupMenu.getItem(i).getName().equals(fxItem.getText())) {
    To      if (this.popupMenu.getItem(i).getLabel().equals(fxItem.getText())) {
    Was not matching properly when using getName()

Changes:

isUnique() method:
    Removed check before loop for popupMenu.getItemCount() == 0 // redundant.

show() method:
    Consolidated the creation of miTitle String. (should be a little more efficient)

isShowing() method:
    Made it self contained so that it no longer relies on parent boolean: showing
    Removed boolean variable 'showing' and all references to it

Added:

method clear() making it easy to rebuild the menu on the fly if desired.
dustinkredmond commented 3 years ago

@EasyG0ing1, now that I think about it, isShowing() could probably be implemented like below:

public boolean isShowing() {
  return Arrays.asList(tray.getTrayIcons()).contains(trayIcon);
}
EasyG0ing1 commented 3 years ago

@EasyG0ing1, now that I think about it, isShowing() could probably be implemented like below:

public boolean isShowing() {
  return Arrays.asList(tray.getTrayIcons()).contains(trayIcon);
}

I'll give it a test and see how it works

EasyG0ing1 commented 3 years ago

OK, I pushed a change to the branch, and the isShowing() method now verifies first that it is the correct instance before returning isEnabled()

EasyG0ing1 commented 3 years ago

@dustinkredmond - There is one concern that I have. When using addMenuItem, if those adds get stacked, there is nothing in the code that makes sure that each manuItem is completely added before the next one is added, because the adding of the menuItem happens on the invokeLater thread ... so what CAN happen (and I verified this) is that if you add the same menuItem a second time before that thread has a chance to finish adding the first one, the isUnique will incorrectly come back as true. Now of course, it's up to the developer to decide if he wants the same menu in there twice I suppose, but I just wanted you to be aware of it, since the isUnique method will not always behave as you obviously intended it to.

I have some ideas as to how we could solidify that process ... but only if you think it's important enough to address.

dustinkredmond commented 3 years ago

It's definitely worth investigating. Even if it would only happen rarely, if we can handle, we probably should. Thanks.

EasyG0ing1 commented 3 years ago

@dustinkredmond - in fact, I'm just going to go ahead and tell you how I would deal with the time delta when adding menuItems - making sure one is added before the next add is attempted, because I've dealt with situations like this before and I'm not sure if my solution is the right way to do it so tell me what you think:

I would have a class level LinkedList, with a looping thread that runs every half second and looks at that LinkedList to see if there are any MenuItems to add to popupMenu. If there are, then it would remove the most recent MenuItem from the list, check to see if it is unique, get the current itemCount from popupMenu, add the MenuItem via the invokeLater thread, then wait for the itemCount in popupMenu to increment by one. After it does, it would then start over and check the LinkedList for any more MenuItems ... rinse ... repeat. That would surely guarantee that no duplicates are added to the icon.

Now if there is a cleaner way to go about that ... like some way to start the process in the invokeLater thread then check on it to see when it's done, that would be ideal ... but I'm not sure how or if that's even possible.

dustinkredmond commented 3 years ago

@EasyG0ing1, that sounds like a plan. I will check into it to see if there's any other way, but that sounds like a great implementation. In the meantime, I'm going to merge and close this PR. If you come up with an implementation to handle the other issue, please open a new PR. Thanks for contributing!