Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(native): add initial tray-support for macOS #972

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

lessp
Copy link
Member

@lessp lessp commented Jul 28, 2020

This adds tray-support for macOS like so:

Native.Tray.make(~title=`Image("absolute/path/to/image.png"), ());
Native.Tray.make(~title=`Text("SomeTitle"), ());

image

Begins to address #322

@github-actions
Copy link

I have updated your lock dirs and formatted the code.
Please @lessp pull the last commit before pushing any more changes.

examples/Examples.re Outdated Show resolved Hide resolved
examples/Examples.re Outdated Show resolved Hide resolved
src/Native/tray.c Outdated Show resolved Hide resolved
src/Native/tray.c Outdated Show resolved Hide resolved
src/Native/tray_cocoa.c Outdated Show resolved Hide resolved
src/Native/Tray.rei Outdated Show resolved Hide resolved
@lessp
Copy link
Member Author

lessp commented Aug 11, 2020

Not sure how we'd like to separate things going forward without naked pointers. I guess we could keep a similar structure but we'd need to import the ocaml-ffi-helpers in foo_cocoa.h as well?

Currently src/Native/tray_cocoa.c is unused and I dumped all of that into tray.c

cc @zbaylin

@lessp lessp force-pushed the feat/native/tray-mac branch from 1aeae23 to 9e0b01f Compare August 28, 2020 08:22
@cdaringe
Copy link

cdaringe commented Nov 6, 2020

I can't wait to try this! Is it on pause indefinitely or just delayed?

@lessp
Copy link
Member Author

lessp commented Nov 6, 2020

I can't wait to try this! Is it on pause indefinitely or just delayed?

hey @cdaringe, currently (unfortunately) I have extremely limited free-time for OSS, so if anyone wants to take this further feel free. 🙂

@lessp
Copy link
Member Author

lessp commented May 22, 2021

Merged with master and moved the examples to a separate file, I believe what's left here is:

  • Being able to remove an item from the tray
  • Vaguely remember @zbaylin creating helpers for allocating stuff, might want to refactor to use those

@lessp lessp marked this pull request as ready for review May 29, 2021 15:59
@lessp lessp requested a review from zbaylin May 29, 2021 15:59
@lessp
Copy link
Member Author

lessp commented May 29, 2021

Hey Zach, I believe this covers the first minimal surface area, albeit for macOS only:

The API surface is fairly large, so we should start by thinking of a minimal subset - perhaps addressing the following scenarios:

Create a tray icon
Destroy a tray icon

Very possible that I might have missed something!

Copy link
Member

@zbaylin zbaylin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @lessp -- so happy to finally see this getting in! I had some minor comments (mostly just rebasing with new APIs like the revery_wrapPointer). I also wonder if it's possible to hook into the NSMenu stuff here too. Maybe that should be a separate PR, though.

src/Native/tray.c Outdated Show resolved Hide resolved
src/Native/tray.c Outdated Show resolved Hide resolved
src/Native/tray.c Outdated Show resolved Hide resolved
src/Native/Tray.rei Show resolved Hide resolved
src/Native/tray.c Show resolved Hide resolved
@lessp
Copy link
Member Author

lessp commented May 29, 2021

Hey @lessp -- so happy to finally see this getting in! I had some minor comments (mostly just rebasing with new APIs like the revery_wrapPointer). I also wonder if it's possible to hook into the NSMenu stuff here too. Maybe that should be a separate PR, though.

Thanks for the review Zach, feels like I left quite a bit for you there! 😄

I also wonder if it's possible to hook into the NSMenu stuff here too. Maybe that should be a separate PR, though.

Yeah, that would have to be up for grabs, just thought I'd try to get this in!


statusItem.button.image = nsImage;

UNUSED(imagePath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: is this still needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, actually, I don't know 🤔 Most likely I followed another example, but looking at the context, I was under the impression that it is, but this is your forte!

    if (vImagePath != Val_none) {
        const char *imagePath = String_val(Some_val(vImagePath));

        NSImage *nsImage = revery_makeImageFromAbsolutePath_cocoa(imagePath);

        statusItem.button.image = nsImage;

        UNUSED(imagePath);
    }

Copy link
Member

@zbaylin zbaylin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lessp! I'm going to push up some changes to the unwrap/wrap semantics, but other than that it looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants