search

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

Added method addMenuItems #21

Closed EasyG0ing1 closed 2 years ago

EasyG0ing1 commented 2 years ago

I added a method named addMenuItems, which allows the developer to add an unlimited number of MenuItems in a single method call. Also, if they create a MenuItem with the text 'separator' attached to it, it will add a separator, which conveniently allows the creation of a somewhat complex menu with one line of code:

For example, this line of code

icon.addMenuItems(menu1, menu2, separator, menu3, menu4, separator, exitMenu);

Creates this menu

Screen Shot 2021-09-08 at 4 54 20 AM

Also, the check for duplicate menuItems still works as well.

I also added a test Class called MultipleItemsOneLine

dustinkredmond commented 2 years ago

@EasyG0ing1 The only change that I would like to see would maybe be to the text "separator" to add the separator. I would rather check that javafx.scene.control.SeparatorMenuItem is passed instead. Let me look at this before I decide to merge. Thank you for your contribution!

EasyG0ing1 commented 2 years ago

@dustinkredmond

I agree that it's not ideal, but I couldn't think of a way to be able to pass in as many MenuItems as desired and still have separators on the same line.

I will update the branch and remove the separator option. It should be good enough to just use the insertSeparator(index) method.

:-)

EasyG0ing1 commented 2 years ago

@dustinkredmond

Hey Dustin,

Don't do anything with this pull request just yet ... I have something else that I want to propose in the form of a different pull request that you might like better.

Mike

dustinkredmond commented 2 years ago

10-4. Thank you sir, awaiting your new pull.

EasyG0ing1 commented 2 years ago

@dustinkredmond - I'm making sure that my i's are dotted and my t's are crossed ... it is by far the largest pull request I think I've ever done... just a heads up :-)

... no changes in any existing functionality at all ... in case you're concerned about that. In fact, it's all-new no changes to any existing code.

dustinkredmond commented 2 years ago

Hey @EasyG0ing1, did I still need to merge this one, or did your other pull take care of it? I released and deployed 3.1.0 this morning with that pull.

EasyG0ing1 commented 2 years ago

Hi @dustinkredmond - SO... It looks like I added ONLY the Builder class to the pull I did after this one ... I guess I should have added this method to that branch in hindsight. I guess I would say that merging this pull is certainly of no ill consequence and is minor enough that it can wait until your next release whenever that might be.

Ultimately ... it's your call ... as always you won't hurt my feelings one way or the other. :-)

Mike

dustinkredmond commented 2 years ago

@EasyG0ing1 merging now, I'll put out a minor release hopefully by this afternoon. Thank you!