vlaaad / reveal

Read Eval Visualize Loop for Clojure
https://vlaaad.github.io/reveal/
MIT License
600 stars 21 forks source link

Dock icon on macOS #34

Open svdo opened 3 years ago

svdo commented 3 years ago

Hi @vlaaad,

First of all, thank you so much for all your work on Reveal. I think it's a very valuable tool in the Clojure space!

I would like to ask if you can include a bit of code to set the Dock icon on macOS. Currently Reveal has a very generic default icon, which makes it harder to recognise. The required code is quite simple. I'd make a pull request myself, but I'm not familiar with JavaFX so I have no idea how to do this, and for you it's likely quite easy. In Java it looks something like like this:

    try {
        URL iconURL = Main.class.getResource("ui/resources/Logo@2x.png");
        Image image = new ImageIcon(iconURL).getImage();
        com.apple.eawt.Application.getApplication().setDockIconImage(image);
    } catch (Exception e) {
        // Won't work on Windows or Linux.
    }

The icon should preferably be 1024x1024 pixels.

If you don't have macOS to test this, maybe you can provide instructions for me on how to accomplish this in the Reveal source code, and I can make a PR and test it myself?

Thank you!

vlaaad commented 3 years ago

Hi! I think that's a good improvement that should be added. I did some quick investigation and it turns out this class was removed in Java 9, so to support both Java 8 and newer JDKs we'll need to also use Taskbar API: https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/java/awt/Taskbar.html#setIconImage(java.awt.Image)

svdo commented 3 years ago

That's a good find. So how do we proceed on this? Are you ok with picking it up? Do you need help?

vlaaad commented 3 years ago

I can proceed with this!

vlaaad commented 3 years ago

For later:

(.setIconImage (java.awt.Taskbar/getTaskbar)
                 (.createImage (java.awt.Toolkit/getDefaultToolkit)
                               (clojure.java.io/resource "vlaaad/reveal/logo-512.png")))
vlaaad commented 3 years ago

(str/starts-with (System/getProperty "os.name") "Mac")

vlaaad commented 3 years ago

@svdo I decided to not do it for now, since there are items with higher priority on my todo list for reveal, but I added this to my todo list and I recognise how annoying this issue is (I'm using reveal on mac at work).