goabstract / electron-panel-window

Enables creating a browser window in Electron that behaves like an NSPanel.
MIT License
56 stars 17 forks source link

Issue with bindings #2

Closed mattpilott closed 5 years ago

mattpilott commented 5 years ago

Hello,

i'm starting a new project and would like to use this panel window package. I've got a fresh electron quick start setup and i've added your npm package. Following that i've added this line to main.js const { PanelWindow } = require('electron-panel-window');

I then get the following error when running npm start

screenshot 2018-11-07 at 9 02 20 am

Any help much appreciated!

tommoor commented 5 years ago

Hey @matt3224 first time creating a native package here so I likely missed something that needs to be included in the postinstall. Could you try rebuilding the package locally with these instructions and let me know what works…

https://github.com/goabstract/electron-panel-window#development

mattpilott commented 5 years ago

Ok sure, so after cloning the repo and running npm i i get the following error:

[Matt:...lectron-panel-window-master]$ npm i
Error: Failed to replace env in config: ${NPM_TOKEN}
    at /usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/config/core.js:415:13

    at String.replace (<anonymous>)
    at envReplace (/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/config/core.js:411:12)
    at parseField (/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/config/core.js:389:7)
    at /usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/config/core.js:330:24
    at Array.forEach (<anonymous>)
    at Conf.add (/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/config/core.js:328:23)
    at ConfigChain.addString (/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/node_modules/config-chain/index.js:244:8)
    at Conf.<anonymous> (/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/config/core.js:316:10)
    at /usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/node_modules/graceful-fs/graceful-fs.js:78:16
/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/npm.js:61
      throw new Error('npm.load() required')
      ^

Error: npm.load() required
    at Object.get (/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/npm.js:61:13)
    at process.errorHandler (/usr/local/Cellar/node@10/10.13.0/lib/node_modules/npm/lib/utils/error-handler.js:205:18)
    at process.emit (events.js:182:13)
    at process._fatalException (internal/bootstrap/node.js:491:27)

Hopefully that's somewhat helpful

tommoor commented 5 years ago

That's not really surprising based on the presence of the npmrc file for our CI publish process.

I actually meant rebuilding the package within the context of your project, are you using any other native modules successfully? I've been able to install without issue on several different machines and the module gets built as part of the install.

You can also check if there is a build directory present inside the package to narrow things down

mattpilott commented 5 years ago

Ok so here's the steps i'm taking, in a new empty directory:

$ git clone https://github.com/electron/electron-quick-start .
$ npm i
$ npm i electron-panel-window

Then I get this error:

screenshot 2018-11-08 at 6 25 39 pm

So then i added nan using $ npm i nan -D

I then re ran $ npm i electron-panel-window which seems to do something positive: console

Then tried npm start which gives this:

dyld: lazy symbol binding failed: Symbol not found: __ZN2v816FunctionTemplate3NewEPNS_7IsolateEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEENS_5LocalIS4_EENSA_INS_9Signatu
reEEEiNS_19ConstructorBehaviorENS_14SideEffectTypeE
  Referenced from: /Developer/Verlay-e/node_modules/electron-panel-window/build/Release/NativeExtension.node
  Expected in: flat namespace

dyld: Symbol not found: __ZN2v816FunctionTemplate3NewEPNS_7IsolateEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEENS_5LocalIS4_EENSA_INS_9SignatureEEEiNS_19ConstructorBehavi
orENS_14SideEffectTypeE
  Referenced from: /Developer/Verlay-e/node_modules/electron-panel-window/build/Release/NativeExtension.node
  Expected in: flat namespace

Then I went to the /node_modules/electron-panel-window folder and ran npm run configure then npm run build back up to the project and i get the same "Symbol not found"

Am i missing something or perhaps theres some assumed knowledge?

Thanks for your help on this!

tommoor commented 5 years ago

Thanks for the help debugging. I think the nan dependency is a legit issue, I based this repo off a boilerplate in the nan docs so I've asked the question in an issue.

I was able to reproduce that, however upon installing nan and rerunning npm i the module compiled successfully for me…

image

So now I'm thinking this could be an issue with the node version? Because of various electron bugs we're stuck on version 1.8 which means I'm running node 8 – I guess you're using something newer…

tommoor commented 5 years ago

Or, the last issue might be related to your XCode install:

http://sd.jtimothyking.com/2018/07/26/stub-file-and-library-file-out-of-sync/

mattpilott commented 5 years ago

Ok switching to node 8 and electron 1.8 makes it work, would i be able to replicate the fixed window functionality of this that you get with macos picture in picture? Where the window is glued to the screen and retains its positions even when swiping between desktops?

tommoor commented 5 years ago

I haven't tried it, but you should be able to combine with this Electron API to achieve https://electronjs.org/docs/api/browser-window#winisvisibleonallworkspaces

mattpilott commented 5 years ago

I did try that but it only allows it to display over fullscreen apps rather than sticking it to the screen like picture-in-picture. I read up a bit awhile ago about how pip works and found it uses nspanel which is why i was super excited to try out your panel window package. I'm not sure what specifically needs to be set in nspanel to have the behaviour i'm looking for either

tommoor commented 5 years ago

it only allows it to display over fullscreen apps

That shouldn't be the case with that particular method – does it change behavior between using BrowserWindow and this module?

It could be because we're explicitly setting the collection behavior here: https://github.com/goabstract/electron-panel-window/blob/master/functions_mac.cc#L39

mattpilott commented 5 years ago

Yes so it does do something when set on BrowserWindow, it does in-fact throw an error for PanelWindow:

2018-11-09 22:02:39.779 Electron[53099:3596396] *** Assertion failure in -[PROPanel _validateCollectionBehavior:], /BuildRo
ot/Library/Caches/com.apple.xbs/Sources/AppKit/AppKit-1671.10.106/AppKit.subproj/NSWindow.m:12591

The behaviour is quite close to pip, it seems to exist on every screen rather than be stuck. I think it's this i need (maybe) https://developer.apple.com/documentation/appkit/nspanel/1531901-isfloatingpanel

mattpilott commented 5 years ago

Just seen this actually in your code: [mainContentView.window setFloatingPanel:YES];

So i guess it might work if that bug can be fixed

tommoor commented 5 years ago

The error makes sense, Electron tries to set the following collectionBehaviors:

https://github.com/electron/electron/blob/9aed2a465f998849921a3ab3df70bc15357332e8/atom/browser/native_window_mac.mm#L1154-L1159

NSWindowCollectionBehaviorCanJoinAllSpaces is incompatible with NSWindowCollectionBehaviorMoveToActiveSpace

mattpilott commented 5 years ago

Is there not a way around that then? Or could it work as a setting perhaps?

tommoor commented 5 years ago

I think setVisibleOnAllWorkspaces method on PanelWindow could be overridden without too much trouble, although this isn't an API I need and am currently swamped with other work.

For a quick fix you could fork the repo and change out the string here: https://github.com/goabstract/electron-panel-window/blob/master/functions_mac.cc#L39

tommoor commented 5 years ago

nan is now included in the dependencies. If you'd like to open an issue for the ontop on all workspaces behavior please go ahead