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

Builder class correction and some other changes... #27

Closed EasyG0ing1 closed 2 years ago

EasyG0ing1 commented 2 years ago

Re-structured the Builder class so that it now conforms to "proper" Builder style. The Builder class now only accepts options that are desired, then the build() method returns a new instance of FXTrayIcon via a private constructor that only accepts an instance of the Builder class as the argument. That constructor then first calls FXTrayIcons main constructor then it proceeds to configure all of the options that were set in the Builder class, including optional MenuItems and other desired behaviors.

Configured default icon size to be set based on operating system both in Builder and parent constructors - if there is no value passed into the library for icon dimensions.

Added an overloaded constructor where only the parentStage is required. FXTrayIcon will use a default graphic for the trayIcon when this constructor is used. This feature existed already in the Builder class, I simply extended it to the parent constructors.

Also tested every runnable test, updated some to use the builder class and replaced the old purple chain icon with the modern looking icon.

dustinkredmond commented 2 years ago

@EasyG0ing1, is this PR ready for merge? I see that the build is failing due to an error in a test class (TestFXTrayIcon). Travis CI shouldn't be running this test when in a headless environment, so this is no huge issue. I can resolve that with another commit. I'm ready to merge #27 if you're done with it. Thanks!

EasyG0ing1 commented 2 years ago

@dustinkredmond I added this to the POM file which excludes tests in the Maven build process, but I'm not sure if that will affect the project when you try to publish it.

<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-surefire-plugin</artifactId>
    <version>2.22.0</version>
    <configuration>
        <excludes>
            <exclude>**/*.java</exclude>
        </excludes>
    </configuration>
</plugin>

After adding that to the POM file, when I do a mvn test -B this is the result:

[INFO] Scanning for projects...
[INFO] Inspecting build with total of 1 modules...
[INFO] Installing Nexus Staging features:
[INFO]   ... total of 1 executions of maven-deploy-plugin replaced with nexus-staging-maven-plugin
[INFO]
[INFO] --------------< com.dustinredmond.fxtrayicon:FXTrayIcon >---------------
[INFO] Building FXTrayIcon 3.1.1
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ FXTrayIcon ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 9 resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.1:compile (default-compile) @ FXTrayIcon ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ FXTrayIcon ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /Users/michael/IdeaProjects/FXTrayIcon/src/test/resources
[INFO]
[INFO] --- maven-compiler-plugin:3.8.1:testCompile (default-testCompile) @ FXTrayIcon ---
[INFO] Nothing to compile - all classes are up to date
[INFO]
[INFO] --- maven-surefire-plugin:2.22.0:test (default-test) @ FXTrayIcon ---
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.225 s
[INFO] Finished at: 2021-11-03T06:40:11-07:00
[INFO] ------------------------------------------------------------------------

Other than that issue, YES, it's ready to go - well except for an unused import that caused it to fail again, but I just fixed that.

dustinkredmond commented 2 years ago

Good enough for me! I need to include some additional logic to check if the tests are being run in a headless environment, but that shouldn't delay these changes. I'm merging and will try to get a release pushed today.

Thank you for your hard work on this!

EasyG0ing1 commented 2 years ago

@dustinkredmond I've affectionately named your build checker the Java Bridge Troll ... think Montey Python ... ;-)

And you're most welcome, these are fun projects for me.