search

branchseer / DeskGap

A cross-platform desktop app framework based on Node.js and the system webview
https://deskgap.com/
MIT License
1.83k stars 75 forks source link

feat: add 'will-finish-launching' #16

Closed hacdias closed 4 years ago

hacdias commented 5 years ago

Add support for 'will-finish-launching': https://electronjs.org/docs/api/app#event-will-finish-launching.

For what I analysed of the code, this seems right. I'm really interested in this project. We're build a tray app and we're just using Electron because of its cross-platform flexibility but I'm feeling it as an overkill. There are some APIs you don't support yet, but I'd like to help with some 😄

hacdias commented 5 years ago

/cc @patr0nus

branchseer commented 5 years ago

Great job digging! The changes look legit.

However this is adding a “will” event. This kind of events are usually meant for an opportunity to do some preparation that must happen before something occurred.

In this case, according to the documentation of applicationWillFinishLaunching: and application:openFile:, I image that a common use case of will-finish-lanuching in electron would be registering an ”open-file” handler in will-finish-lanuching, to let the app know what to do when application:openFile: (which occurs before applicationWillFinishLaunching:) is called.

In DeskGap Node.js function calls (jsOnWillFinishLaunching->Call() in this case) are asynchronous (related code), so when the callbacks_.onWillFinishLaunching() returns and applicationWillFinishLaunching: finishes, the actually call to the js function onWillFinishLaunching is probably still waiting in the Node.js event queue. It makes a ”will” event kind of meaningless that the event may be raised late.

Pardon my English. I hope I made myself clear.

branchseer commented 5 years ago

BTW, have you successfully built DeskGap? I just updated the Build Instructions.