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

Incorrect Binding to Window Event Listeners #139

Closed
kgorgi opened this issue Oct 10, 2017 · 1 comment
Closed

Incorrect Binding to Window Event Listeners #139

kgorgi opened this issue Oct 10, 2017 · 1 comment
Labels

Comments

@kgorgi
Copy link
Contributor

kgorgi commented Oct 10, 2017

Bug or feature request?

Bug

Expected behaviour

React-beautiful-dnd binds event listeners to current browser window that has the draggable component.
For comparison both react-sortable-hoc and react-dnd do not always directly bind to the global window object.

Actual behaviour

I am using a package called react-popout with react-beautiful-dnd. This library allows react elements to be "popped out" of the current browser window by opening another browser window. As react-beautiful-dnd binds its event listeners to the global window object, the cursor is bound to the wrong window. (see demo)

Steps to reproduce

Enclose a react-beautiful-dnd list in a PopoutWindow component (imported from react-popout). Attempt to drag draggables in the list.

Browser version

Version 61.0.3163.100 (Official Build) (64-bit)

Demo

https://drive.google.com/open?id=0B4Bi-ARyjM_hT1dFaE9ueHVOQVU

Notice how in the popout window, items will only drag when the cursor is in the original window.

How to Fix

The fix for this is quite easy, in the drag-handle.jsx file use the draggableRef prop to get access to the actual window that the draggable is rendered on.

Original Code:

unbindWindowEvents = () => {

Suggested Fix:

unbindWindowEvents = () => {
    const win = this.props.draggableRef.ownerDocument.defaultView;
    win.removeEventListener('mousemove', this.onWindowMouseMove);
    win.removeEventListener('mouseup', this.onWindowMouseUp);
    win.removeEventListener('mousedown', this.onWindowMouseDown);
    win.removeEventListener('keydown', this.onWindowKeyDown);
    win.removeEventListener('resize', this.onWindowResize);
    win.removeEventListener('scroll', this.onWindowScroll);
    win.removeEventListener('webkitmouseforcechanged', this.mouseForceChanged);
  }

  bindWindowEvents = () => {
    const win = this.props.draggableRef.ownerDocument.defaultView;
    win.addEventListener('mousemove', this.onWindowMouseMove);
    win.addEventListener('mouseup', this.onWindowMouseUp);
    win.addEventListener('mousedown', this.onWindowMouseDown);
    win.addEventListener('keydown', this.onWindowKeyDown);
    win.addEventListener('resize', this.onWindowResize);
    win.addEventListener('scroll', this.onWindowScroll, { passive: true });
    win.addEventListener('webkitmouseforcechanged', this.mouseForceChanged);
  }
@jaredcrowe
Copy link
Contributor

Hey @kgorgi thanks for raising this :) We'd be happy to accept a PR for this if you'd like to submit one. Otherwise this is something we're aiming to look at next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants