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

Mouse backend support #27

Open
SergiuB opened this issue Feb 2, 2018 · 3 comments
Open

Mouse backend support #27

SergiuB opened this issue Feb 2, 2018 · 3 comments

Comments

@SergiuB
Copy link

SergiuB commented Feb 2, 2018

There's also this mouse backend which is not currently supported by this library.

Seems that the only change required to support it would be to add event handlers for mousemove, similar to the existing ones for dragover and touchmove. I tried and it works as expected.

Any insights, other things to be aware of? It would be great to have this backend also supported.

@SergiuB
Copy link
Author

SergiuB commented Feb 2, 2018

On a closer look through the code, is there any good reason to subscribe to dragover and touchmove events, when the monitor subscription seems to provide all that's required, namely the coordinates and the dragging state?

I removed all the event handlers and calling updateScrolling directly from handleMonitorChange (modified its signature to take directly coordinates instead of an event).

handleMonitorChange() {
        const monitor = this.context.dragDropManager.getMonitor();

        const isDragging = monitor.isDragging();

        if (isDragging) {
          const coords = monitor.getClientOffset();
          if (coords) {
            this.updateScrolling(coords);
          }
        } else if (!isDragging) {
          this.stopScrolling();
        }
      }
    }

If I'm not missing something, this would make it backend agnostic, and also simplify the code quite a bit.

@nickclaw
Copy link
Contributor

nickclaw commented Feb 2, 2018

That's a good thought, it would be great to remove the event listeners and make this totally backend agnostic.

The reason react-dnd-scrollzone works the way it does right now comes down to performance, for our use case it's not unusual to have 100s of different scrollzones (sometimes even scrollzones inside scrollzones). I'm worried what the performance impact would be if every single component did the updateScrolling check even when they're clearly not being dragged over.

I'll try it out and look at the performance profile.

@SergiuB
Copy link
Author

SergiuB commented Feb 2, 2018

Thanks a lot for the clarification, makes sense, very helpful.

It would be good to at least come up with a way to hook up these event handlers that would not require changes in the lib to support a new backend, something similar to how backends are plugged in React DnD, without making core changes.

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