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

Native Integration: Platform alert dialog #276

Merged
merged 17 commits into from
Jan 29, 2019

Conversation

bryphe
Copy link
Member

@bryphe bryphe commented Jan 29, 2019

This is a proposal for how we could (potentially) implement a native layer for Revery, re #250 #249

This adds a Revery_Native module (which actually talks to the native Win32/Cocoa APIs). An open question is how we'll implement support on Linux platforms - what's the best way to handle this in X11? I see some apps "roll their own" platform dialogs, which seems pretty intense. I'm leaving this stubbed out for now, but perhaps someone more knowledgeable than me in X11 can help out.

This is what the 'native' example looks like on Windows:
native alert

Also works on OSX (Cocoa) and JS with these changes. The Linux/X11 strategy is just a stub right now.

This depends on the work done in bryphe/reason-glfw#89 to expose a glfwGetNativeWindow API, which gives us access to the platform-specific window.

There is also a wrapper on top - these are accessible via the Revery.Platform API. The reason for the indirection is that, if we need to implement these primitives on the Revery side, we could check if the functionality is available (ie, via Revery_Native.Dialog.alertSupported()), and then, if it isn't, create a Revery-based fallback.

There's also some messiness in terms of how we cast from the value -> void * -> whatever platform-specific pointer is being used (HWND, NSWindow *, unsigned long int, etc..).

I was hoping I could conditionally include c_names based on the platform - but it turns out this is not possible. My current approach uses a lot of #ifdef platform detection, which isn't really ideal. This is what it looks like now:

  CAMLprim value
  revery_alert(value vWindow, value vMessage) {
      CAMLparam2(vWindow, vMessage);
      const char *szMessage = String_val(vMessage);
      void* pWin = (void *)vWindow;

  #ifdef WIN32
      revery_alert_win32(pWin, szMessage);
  #elif __APPLE__
      revery_alert_cocoa(pWin, szMessage);
  #else
      printf("WARNING - Not implemented: alert");
  #endif
      return Val_unit;
  }

But I'm sure there's better ways to do this... Some ideas were:

  • Implementing the platform APIs as sub-packages
  • Virtual modules to implement the back-ends

I like both of these ideas, but not sure how to implement them.

Once we've iterated on this API surface a bit, we could look at moving it out to a shared library like revery-native that brisk could use (if the APIs are helpful / reusable at all) cc @wokalski @rauanmayemir

@OhadRau
Copy link
Collaborator

OhadRau commented Jan 29, 2019

We could try https://github.com/janestreet/ppx_optcomp for conditional compilation.

@bryphe
Copy link
Member Author

bryphe commented Jan 29, 2019

@OhadRau - hmm, it looks like ppx_optcomp only works on OCaml files? Or is there a way to use it for .c/.h files?

@tcoopman
Copy link
Contributor

Dune also has support for conditional compilation. You can see an example here https://github.com/mirage/irmin-watcher/blob/master/src/dune

Unfortunately esy doesn't support it yet esy/esy#777

For the Linux dialogs, I would think that picking gtk or Qt dialogs is the preferred choice. I'm not sure what electron supports but will check when I'm at my laptop

@tcoopman
Copy link
Contributor

Electron uses Gtk dialogs on Linux. This PR might be relevant as well: electron/electron#15293.
GtkFileChooserNative should drop to a native dialog chooser.

@bryphe
Copy link
Member Author

bryphe commented Jan 29, 2019

Thanks @tcoopman - very helpful! I read about separate compilation but never saw a usage example - that linked helped clarify how it could work.

The Electron PR is useful too - following their precedent makes a lot of sense. It seems like a bit of work to get Gtk3 hooked up to our build, so I tried to sum up the work and track the remaining issues here: #278

@bryphe bryphe changed the title [WIP] Native Integration: Platform alert dialog Native Integration: Platform alert dialog Jan 29, 2019
@bryphe bryphe merged commit 0beaee4 into master Jan 29, 2019
@bryphe bryphe deleted the bryphe/feature/initial-native-integration branch January 29, 2019 17:16
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.

3 participants