codecentric / NSMenuFX

Other
127 stars 26 forks source link

Remove dependency on 'javafx.web' module. #30

Closed msgilligan closed 3 years ago

msgilligan commented 5 years ago

On JavaFX 11, I found I had to add a dependency on javafx.web even though I'm not actually using the default "About" stage (window)

I think I can work around this by calling createDefaultApplicationMenu with a preexisting Stage, but for a modular JDK, it would be nice if the menu library didn't have a dependency on javafx.web at all.

0x4a616e commented 3 years ago

I like the idea, but I'm not sure if/how this could be done without creating a breaking change or surprising other users with an about menu that just stops working as before.

mipastgt commented 3 years ago

How does that dependency come in? I don't have a dependency on javafx.web in my software although I use NSMenuFX.

msgilligan commented 3 years ago

How does that dependency come in?

The default "About" window uses an HTML/Web view to display.

msgilligan commented 3 years ago

I like the idea, but I'm not sure if/how this could be done without creating a breaking change or surprising other users with an about menu that just stops working as before.

I think the default About box could (and should) be replaced with something with minimal dependencies (i.e. no web, no FXML.) I wouldn't call it a breaking change, because the app would still work the same as before. Yes, the appearance of the About box might change a little, but anyone who cares about the appearance of their About box would have implemented a custom one anyway.

Note that for Java Module System it becomes more important to not have this dependency.

I'm proposing this change for the 3.0 (JDK 11+) version.

msgilligan commented 3 years ago

Another thing that could be done I suppose, is to still provide the WebView-based implementation and detect at run-time if it is present and use it if it is present. That seems like overkill to me for a default About box that most people will be replacing anyway.

mipastgt commented 3 years ago

One could give all dependencies the <scope>provided</scope>. Avoiding hard dependencies would also solve the problem that people might want to use different versions of JavaFX from 11 to 16 EA.

msgilligan commented 3 years ago

I'm also concerned about the dependencies in module-info.java. I'm using NSMenuFX in applications that are being packaged with jlink and jpackageand I'm trying to build the smallest package possible.

(I'm using the BadAss JLink Plugin which helps adapt non-modular JARs and is working well)

mipastgt commented 3 years ago

I am using this concept here https://github.com/dlemmermann/JPackageScriptFX instead of the BadAss plugin. This works non-modular but still uses jlink and jpackage to keep the image small. But it would be indeed best to remove the WebView from the default about dialog. 3.0 seems to be a good point to make this slightly breaking change.

0x4a616e commented 3 years ago

There it is, javafx.web dependency removed: https://github.com/0x4a616e/NSMenuFX/commit/17cd591cbd2e2f059642a749d09734464d593cba

I'll push an update soon together with other changes like dark mode, native context menu and tray menu support. I renamed the API method so that everyone who has been using the old version will immediately notice that something has changed.

0x4a616e commented 3 years ago

Closing this as solved in

<dependency>
  <groupId>de.jangassen</groupId>
  <artifactId>nsmenufx</artifactId>
  <version>3.0.0</version>
</dependency>