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

React Mosaic windows are not resizable in a popup window. #91

Closed
ConfuddledPenguin opened this issue Nov 6, 2018 · 3 comments
Closed

Comments

@ConfuddledPenguin
Copy link

When you use the component in a new browser window (using a react portal), you can not resize or move the react-mosaic windows.

Bug report

  • Package version(s): 2.0.1
  • Browser and OS versions: Windows: Chrome, Firefox

Steps to reproduce

An example - codesandbox github repo
You will need to enable popups and refresh the sandbox.

  1. On loading the page a popup window should appear
  2. In the popup try to resize the mosaic windows.

I noticed that if you then move the cursor into the original document the movements will then affect the popup. Could this be that there is a listener bound on the document, that isn't being added in the popup window?

Actual behavior

Windows are not being resized / moved.

Expected behavior

Windows should be resized / moved

@nomcopter
Copy link
Owner

nomcopter commented Nov 6, 2018

It looks like this is your issue facebook/react#12355 unless other React Components are working in that window. Let me know if this is not the case and I can reopen!

@ConfuddledPenguin
Copy link
Author

ConfuddledPenguin commented Nov 7, 2018

@nomcopter

Other react components are working fine. So I still think this is a bug.

Spent a little time digging into it. When you render something in a new window the JS is running in the original window, this means that any references to window and document are to the orginal window and not the popup.

I think this means there is two issues:

  1. The use of document in the Split component preventing window resizing.

    I got this working by swapping:

    document.removeEventListener('mousemove', this.onMouseMove, true);
    

    with

    this.rootElement!.ownerDocument!.removeEventListener('mousemove', this.onMouseMove, true);
    

    This ensures that the event is registered on the correct document.

    repo (couldn't get it to run in a sandbox to show).

  2. The use of react-dnd for the window positioning.

    I am not familiar with react-dnd in order to produce a simple example with it not functioning in a popup window, but I guess we should open an issue with react-dnd once we can demonstrate the issue.

@nomcopter nomcopter reopened this Nov 7, 2018
@ConfuddledPenguin
Copy link
Author

ConfuddledPenguin commented Nov 13, 2018

I have managed to produce an example of react-dnd working across windows, can be found here. In WindowPortal I register the required react-dnd event listeners on to the newly opened window using the react-dnd manager.

In the above example it is also possible to drag and drop the chess piece from one window to the other.

I have also managed to get drag and drop as well as window resizing working within react-mosaic (although its rather messy atm). The code can be found in this repo

I guess the next step is deciding which of the below to support:

  1. Supporting mosiac window resizing in a browser popup window. This is the simple change i mentioned above where you change the following:

    document.removeEventListener('mousemove', this.onMouseMove, true);
    

    to

    this.rootElement!.ownerDocument!.removeEventListener('mousemove', this.onMouseMove, true);
    

    This leaves the code to open a new window and handling drag and drop up to the user. I think this would place a limitation on never being able to drag between browser windows.

  2. Supporting mosiac window resizing in all browser windows and drag and dropping only in the browser window a mosiac window is in. A rough start of to this can be found in the (repo)[https://github.com/ConfuddledPenguin/react-mosaic/tree/DragandDropInExternalWindow] I linked above.

    This might mean that a mechanism to open a new window would need to be included. While I have been using my own implementation so far there is this library to do this.

    I think this would also place a limitation on never being able to drag between browser windows.

  3. Supporting mosiac window resizing in all browser windows and drag and dropping between browser windows.

    I guess this is optimal case from a users perspective, ease of use, and feature completeness; but would not be without a large chunk of work. It would require being able to open popup windows, as well as keeping track of the popup windows that mosiac windows are in. As this would require a rework of the state used to keep track of windows this could potentially be combined with some of the work required to also support tabbed windows Tabbed Windows #50.

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

No branches or pull requests

2 participants