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

move to an electron-like api #24

Merged
merged 22 commits into from Apr 5, 2018

Conversation

Projects
None yet
3 participants
@mathieudutour
Member

mathieudutour commented Dec 1, 2017

fix #10, fix #26, Fix #35

@cameronmcefee

This comment has been minimized.

Show comment
Hide comment
@cameronmcefee

cameronmcefee Jan 5, 2018

Contributor

I'm trying to set this up on my project in hopes of helping test it (and getting myself ready to use it). How do I use the new event system, for example, to do something when the panel loads. Based on the readme, I tried:

browserWindow.webContents.on('dom-ready', function() {
  console.log('Hello world');
});

But that didn't appear to do anything

Contributor

cameronmcefee commented Jan 5, 2018

I'm trying to set this up on my project in hopes of helping test it (and getting myself ready to use it). How do I use the new event system, for example, to do something when the panel loads. Based on the readme, I tried:

browserWindow.webContents.on('dom-ready', function() {
  console.log('Hello world');
});

But that didn't appear to do anything

@mathieudutour

This comment has been minimized.

Show comment
Hide comment
@mathieudutour

mathieudutour Jan 5, 2018

Member

it should work, that's the test plugin I'm using while developing:

import WebUI from '../../lib'

export default function(context) {
  const ui = new WebUI({
    show: false,
    acceptsFirstMouse: true
  })

  ui.loadURL('https://google.com')

  ui.on('ready-to-show', () => {
    ui.show()
  })

  ui.on('closed', () => {
    context.document.showMessage("closed")
  })

  context.document.showMessage("It's alive 🙌")
}
Member

mathieudutour commented Jan 5, 2018

it should work, that's the test plugin I'm using while developing:

import WebUI from '../../lib'

export default function(context) {
  const ui = new WebUI({
    show: false,
    acceptsFirstMouse: true
  })

  ui.loadURL('https://google.com')

  ui.on('ready-to-show', () => {
    ui.show()
  })

  ui.on('closed', () => {
    context.document.showMessage("closed")
  })

  context.document.showMessage("It's alive 🙌")
}
@mathieudutour

This comment has been minimized.

Show comment
Hide comment
@mathieudutour

mathieudutour Jan 5, 2018

Member

I haven't tested the webContents events yet so it might not work...

Member

mathieudutour commented Jan 5, 2018

I haven't tested the webContents events yet so it might not work...

@cameronmcefee

This comment has been minimized.

Show comment
Hide comment
@cameronmcefee

cameronmcefee Jan 6, 2018

Contributor

Ah yep, got it working. The delayed show is a nice touch.

Contributor

cameronmcefee commented Jan 6, 2018

Ah yep, got it working. The delayed show is a nice touch.

cameronmcefee and others added some commits Jan 6, 2018

Merge pull request #27 from cameronmcefee/patch-1
Fix typo in code sample
Improve acceptsFirstMouse hack
This addresses two problems:

- elementFromPoint gets children of anchors. In some cases, like when an anchor contains a svg path element which has no .click() method, this causes an error. By dispatching a click event from the element, we avoid the error and the event bubbles up.

- In most cases, when using acceptsFirstMouse to click a text-based input, the expected behavior is actually .focus()
Merge pull request #29 from cameronmcefee/electron-api
Improve acceptsFirstMouse hack
Show outdated Hide outdated lib/webview-api.js Outdated
Merge pull request #31 from cameronmcefee/fix-orderout
pass null to orderOut to prevent errors
Show outdated Hide outdated lib/browser-api.js Outdated
@jenil

This comment has been minimized.

Show comment
Hide comment
@jenil

jenil Feb 28, 2018

Sketch v49 just landed, any clue when this will be merged? I am starting a project and would love to use this.

jenil commented Feb 28, 2018

Sketch v49 just landed, any clue when this will be merged? I am starting a project and would love to use this.

@mathieudutour

This comment has been minimized.

Show comment
Hide comment
@mathieudutour

mathieudutour Feb 28, 2018

Member

This is not really related to Sketch 49. There are still quite a few bugs in this PR and I, unfortunately, don't have much time to work on it. If you want to test it a bit more, I'd happy to merge it once it's in a good shape!

Member

mathieudutour commented Feb 28, 2018

This is not really related to Sketch 49. There are still quite a few bugs in this PR and I, unfortunately, don't have much time to work on it. If you want to test it a bit more, I'd happy to merge it once it's in a good shape!

@cameronmcefee

This comment has been minimized.

Show comment
Hide comment
@cameronmcefee

cameronmcefee Feb 28, 2018

Contributor

@jenil It's also possible to install and use it from the branch if you're willing to take the risk of future breaking changes.

npm install https://github.com/skpm/sketch-module-web-view.git#electron-api
Contributor

cameronmcefee commented Feb 28, 2018

@jenil It's also possible to install and use it from the branch if you're willing to take the risk of future breaking changes.

npm install https://github.com/skpm/sketch-module-web-view.git#electron-api
@jenil

This comment has been minimized.

Show comment
Hide comment
@jenil

jenil Feb 28, 2018

okay got it, I'm taking the risk. Thanks @mathieudutour @cameronmcefee

jenil commented Feb 28, 2018

okay got it, I'm taking the risk. Thanks @mathieudutour @cameronmcefee

@mathieudutour

This comment has been minimized.

Show comment
Hide comment
@mathieudutour

mathieudutour Apr 4, 2018

Member

I added some docs and fixed the frameless window.

I'm tempted to merge this. Anything still broken @cameronmcefee @jenil?

Member

mathieudutour commented Apr 4, 2018

I added some docs and fixed the frameless window.

I'm tempted to merge this. Anything still broken @cameronmcefee @jenil?

var x = point.x
var y = webView.frame().size.height - point.y // the coord start from the bottom instead of the top
return (
'var el = document.elementFromPoint(' + // get the DOM element that match the event

This comment has been minimized.

@cameronmcefee

cameronmcefee Apr 4, 2018

Contributor

I haven't had a chance to submit a fix but this throws an error when the click target is the window title bar, since there are no elements to click in that region.

@cameronmcefee

cameronmcefee Apr 4, 2018

Contributor

I haven't had a chance to submit a fix but this throws an error when the click target is the window title bar, since there are no elements to click in that region.

@cameronmcefee

This comment has been minimized.

Show comment
Hide comment
@cameronmcefee

cameronmcefee Apr 4, 2018

Contributor

@mathieudutour Nice! I left one comment about a bug I haven't had time to fix but otherwise I haven't run into anything. I never resolved my "can't prevent close" issue but given that it works on your end, I suspect it's an issue with my code.

Contributor

cameronmcefee commented Apr 4, 2018

@mathieudutour Nice! I left one comment about a bug I haven't had time to fix but otherwise I haven't run into anything. I never resolved my "can't prevent close" issue but given that it works on your end, I suspect it's an issue with my code.

@mathieudutour

This comment has been minimized.

Show comment
Hide comment
@mathieudutour

mathieudutour Apr 5, 2018

Member

Actually the "can't prevent close" is not working for me as well. But I don't think it's possible to fix right now due to how the cocoascript bridge works atm.

The technical issue is that the delegate needs to return a BOOL (NO) but we can only return an NSObject which is probably cast as YES.

I'll see how we can fix it, it probably involves some changes to cocoascript itself which means we will need a Sketch release to bring the changes. But let's ship that for now!

Member

mathieudutour commented Apr 5, 2018

Actually the "can't prevent close" is not working for me as well. But I don't think it's possible to fix right now due to how the cocoascript bridge works atm.

The technical issue is that the delegate needs to return a BOOL (NO) but we can only return an NSObject which is probably cast as YES.

I'll see how we can fix it, it probably involves some changes to cocoascript itself which means we will need a Sketch release to bring the changes. But let's ship that for now!

@mathieudutour mathieudutour changed the title from [WIP] move to an electron-like api to move to an electron-like api Apr 5, 2018

@mathieudutour mathieudutour merged commit 9e89ea1 into master Apr 5, 2018

@mathieudutour mathieudutour deleted the electron-api branch Apr 5, 2018

@cameronmcefee

This comment has been minimized.

Show comment
Hide comment
@cameronmcefee

cameronmcefee Apr 5, 2018

Contributor

🎉 Nice. For my own solution I just hid the title bar entirely which removed the problem

Contributor

cameronmcefee commented Apr 5, 2018

🎉 Nice. For my own solution I just hid the title bar entirely which removed the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment