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: allow configuration of window global for event subscriptions #1400

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

Conversation

loc
Copy link

@loc loc commented Sep 15, 2022

Background

In multi-window applications, both on the web and in web embedders like Electron or CEF, it is not unusual to open child windows as "dumb" windows that serve as a render target only and allow their parent to retain control of all scripting responsibilities. In order for event listeners to work correctly, though, rendering code has to take into account the fact that input subscriptions need to be setup for the render target (child) window, rather than the control window.

Proposed change

As far as I can tell, only the drag and drop module and the Transformer class access the window object for event subscription.

Transformer

This change adds an optional configuration property to Stage which sets the window global that the Konva should be using to make event subscriptions within this stage. It also adds a public getter for this value.

// new constructor config property
var stage = new Konva.Stage({
    // ...normal configuration
    windowGlobal: childWindow,
});
// new getter
stage.getWindowGlobal();

Drag and drop

The drag and drop code is a little tricky because the event subscription happens at the top level of the module. To delay that in order to subscribe to the window global we actually care about, we defer that event subscription until Stage construction.

Miscellaneous

I had to reorder the Transformer destructor to remove events before detaching — I don't know the code well enough to know the implications of that, so please advise if that is undesirable.

@lavrton
Copy link
Member

lavrton commented Sep 16, 2022

How child windows are created? Can they use their own scripts?

@loc
Copy link
Author

loc commented Sep 20, 2022

Child windows are created with the window.open API, and can be used to load a webpage, as normal, or to create an empty page (with the url about:blank) that is available for manipulation. It does produce a separate javascript runtime instance, but you don't have to use it, and can instead control the window from the parent runtime.

For example:

const child = window.open('about:blank', '_blank', 'width=100,height=100');
const ele = child.document.createElement('marquee');
ele.innerText = 'Hello world';
child.document.body.replaceChildren(ele);

@loc
Copy link
Author

loc commented Oct 13, 2022

@lavrton anything I can do to get this merged? Thanks!

@lavrton
Copy link
Member

lavrton commented Oct 24, 2022

What I don't like in the current change, is that creating several stages my break DragAndDrop module.

registerDragAndDropListenersForWindowGlobal(this.getWindowGlobal()); is called in constructor. But what if we create another stage?

@loc
Copy link
Author

loc commented Oct 28, 2022

What do you think breaks in the DragAndDrop module? If two stages are created with the same window global, the second call to registerDragAndDropListenersForWindowGlobal is effectively a no-op, as it just tears down and sets up the listeners again. If the second stage uses a different window global, then it just sets up listeners on the new window global, which is essential for drag and drop to work in the new window.

@lavrton
Copy link
Member

lavrton commented Nov 17, 2022

Probably should work ok. Can you restore nodejs tests? Need to make sure that window can be undefined.

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.

2 participants