netbeans icon indicating copy to clipboard operation
netbeans copied to clipboard

Update NetBeans icns icon to match macOS style guidlines

Open oyarzun opened this issue 1 year ago • 6 comments

  • This PR goes with the updated NetBeans app icon for macOS to match current design guidelines created by @mhalachev #7127 and https://github.com/apache/netbeans-nbpackage/pull/51

  • Do not call the frame's setIconImages on macOS. This allows macOS to use the icns to create a minimized icon for the Dock (https://github.com/apache/netbeans-nbpackage/pull/51#issuecomment-1974857999) Screen Shot 2024-03-04 at 11 14 10 AM

NOTE: This only works when NetBeans is run from the App bundle. If you run from the terminal using the netbeans shell script it will show the terminal as the minimized icon

This PR supersedes #3168

oyarzun avatar Mar 04 '24 16:03 oyarzun

Can we use an environment variable (or system property at a push) to control this?

Would System.getenv("APP_DOCK_NAME") != null work? Probably not ideal. Does macOS set environment variables related to the app bundle?

We can always add additional options into https://github.com/apache/netbeans-nbpackage/blob/master/src/main/resources/org/apache/netbeans/nbpackage/macos/main.swift.template

neilcsmith-net avatar Mar 04 '24 17:03 neilcsmith-net

Can we use an environment variable (or system property at a push) to control this?

Would System.getenv("APP_DOCK_NAME") != null work? Probably not ideal.

We can always add additional options into https://github.com/apache/netbeans-nbpackage/blob/master/src/main/resources/org/apache/netbeans/nbpackage/macos/main.swift.template

I suppose you could set a flag via an environment variable and add some logic in MainWindow.java to use it to decided when to call setIconImages on macOS.

But launching NetBeans via the command line (shell script or Swift launcher) has other side effects like the icon in the Dock has the name java instead of Apache NetBeans. It also creates a new icon in the Dock if Keep in Dock was set on NetBeans previously.

I think the true fix to these issues would be to have the macOS launcher to spawn the JVM using libjvm akin to what the Windows launcher does.

oyarzun avatar Mar 04 '24 18:03 oyarzun

OK, from my perspective, let's keep it simple and stick with what you have for now. Although I had meant an environment variable instead of checking for the OS. Might be useful elsewhere.

Ironically there was some discussion on ASF Slack a while ago about the possibility of moving away from the libjvm approach in the Windows launcher.

neilcsmith-net avatar Mar 04 '24 20:03 neilcsmith-net

@oyarzun Looks very good! 👍 So if we exit early before calling setIconImages(... on macOS it works as expected.

This only works when NetBeans is run from the App bundle. If you run from the terminal using the netbeans shell script it will show the terminal as the minimized icon

How much of an issue will this cause in real-world scenarios, considering that most users will likely launch the app directly from the bundle?

mhalachev avatar Mar 05 '24 12:03 mhalachev

@neilcsmith-net I agree that it may be kept simple for now with Utilities.isMac(), since it is used for some other macOS-specific checks within the same class, e.g. line 206, 736.. plus it becomes self-explanatory.

However, it's worth considering the environment variable approach for future use. This could prove useful if there is a need to combine it with additional options or set parameters dynamically.

mhalachev avatar Mar 05 '24 13:03 mhalachev

Indeed, it appears to be good as is.

P.S. If there are still concerns about the minimisation issue while running from the terminal, given that it may generally manifest with development builds in practice, adding a comment or logging may be considered. (e.g. minimised during development -> info in console)

mhalachev avatar Mar 11 '24 20:03 mhalachev

@oyarzun @neilcsmith-net I would suggest to get this in without further changes for now, as generally it appears to work fine with the app bundle.

mhalachev avatar Mar 28 '24 15:03 mhalachev

Thank you all; I merged this one now!

Ironically there was some discussion on ASF Slack a while ago about the possibility of moving away from the libjvm approach in the Windows launcher.

@neilcsmith-net I didn't see the libjvm discussion, but I can mention one thing to be aware of, which is that the "Make the Windows launcher work with Unicode paths" patch at https://github.com/apache/netbeans-native-launchers/pull/7 would need to be reworked if the launcher ends up spawning java in a different process, because the activeCodePage setting would then be taken from javaw.exe rather than from nbexec.exe.

eirikbakke avatar Mar 29 '24 13:03 eirikbakke