react-grid-layout / react-resizable

A simple React component that is resizable with a handle.
https://strml.github.io/react-resizable/examples/1.html
MIT License
2.43k stars 367 forks source link

Resize handle lagging behind mouse cursor #237

Open smooth038 opened 5 months ago

smooth038 commented 5 months ago

Problem Report

We have been using react-grid-layout for some time in our application, as we have a dashboard page with many different resizable and draggable widgets. We had no problem for the longest time.

Recently, we did an upgrade of our main UI library, and somehow the performance of the application dropped noticeably, but not too big a deal. Except for one thing, when we resized or dragged a widget in a dashboard with many other widgets, the handle would move 2-3 times more slowly than the mouse cursor, creating an increasing gap between the two, and making the resizing/dragging process very difficult and unpleasant.

We could not find any obvious issue with our code, other than the fact that the application needs some important refactoring in order to limit the number of unnecessary re-renders and improve the overall performances, but that will have to wait a little bit. We also noticed that the issue was already there before the major library update, only on a much smaller scale and no one would notice it.

Since the issue seemed to come from the react-grid-layout and react-resizable libraries, we investigated and found a way to fix the issue within the libraries' code.

When dragging or resizing, the resulting size or position of the widget is calculated in the resizeHandler by adding the difference in X and Y of every mousemove event, using the deltaX and deltaY fields of the event. In our case, it seems that for some reason, only some proportion of the mousemove events were actually processed, while others would be dropped, which means that the sum of all deltas was not equivalent to the displacement of the cursor.

I modified the code so that instead, we would do the following (this example is for react-resizable but we did something equivalent in react-grid-layout for dragging) :

  1. Upon the onResizeStart event, save the original size of the widget (props.width and props.height) and the position of the mouse cursor at beginning of resize (lastX and lastY of the event, which are absolute values)
  2. On each call of resizeHandler, calculate the resulting size by using the original size and adding the difference between the actual cursor position (lastX and lastY) and the initial position of the cursor at beginning of resize.

By doing this, we were able to effectively correct the problematic behavior that we had and made a custom version of both libraries that we published on our local artifactory repository.

I tried to reproduce the issue by using your sandbox link (thank you!) and adding some artificial processing (finding a certain number of prime numbers for fun) in the rendering of the widgets in the hope of making the browser drop some mouse events, but with no luck. Maybe some of you may have an idea on how we could reproduce this? If useful, maybe I can share a video of the bug, but I'll probably need to hide some stuff because of non-disclosure agreements, so it will take me a bit of time.

I understand that, unless we can reproduce this consistently, we can't really consider this as a bug in the library. However, I wanted to raise the issue in case other people experience it, and see what you guys think about this. I wonder, was there a reason why we are using the deltaX/Y approach rather than using lastX/Y?

If you are interested in seeing the modifications that we made to fix the issue on our end, let me know and I can open a PR.

Thanks a lot for your time,

System Info

Node Version: 20.8.1 Browser: Chrome, Firefox, Edge (was fine on a mac with Arc, but I think that's because the machine was really fast) OS: Ubuntu, Windows

Reproduction

Unfortunately I was unable to reproduce the issue in a sandbox, but here is how we reproduced it in our application:

  1. Have a big dashboard with many heavy-on-resource widgets
  2. Click and drag the resize handle of a widget and shake the mouse a little.
STRML commented 5 months ago

Thanks, this is the first I've heard of something like this. We use deltaX and deltaY from react-draggable because that's how it does it, and it's used for grid snapping, transformScale, constraints and so on. I recall changing this from calculating from starting coordinates to deltas something like 6-7 years ago, but to be honest I can't recall exactly why.

I'd say what's more concerning is that you're "losing" data somehow, which should never happen. It sounds like something is capturing mouse events or creating an early exit from resizeHandler, but I can't tell you quite how this happens. I would suggest introducing some logging at the beginning and end of the resize events to see if you can figure out where it drops out - if it's inside the function or if something else is intercepting events.

spoofy9 commented 2 months ago

I have a similar problem. Can you share the fix?

spoofy9 commented 2 months ago

@smooth038 can help me solution?

smooth038 commented 2 months ago

Here are the changes we made. Simply run a git apply on both patches and you are good to go. Ideally you will need to build and publish the new version of the two packages to a private npm repository that your web app has access to, then just use the new version number in package.json.

Hope this helps!

fix-resizing-and-dragging-issues.zip

By the way, we did not investigate further why the issue was arising, as we have other priorities and the problem is now fixed. But I will keep you updated if I have any news.

EDIT: Just in case, the react-resizable patch should be applied after commit 54a8518b68e189b31e72149e693d60bbf3bde5df, and the react-grid-layout patch should be applied after commit 04a778264b056a024334d33d17305a57970ccc40. But if you undestand the changes, you probably can just adapt them to the latest versions.

RobertRochonCodi commented 3 weeks ago

@smooth038 - Thank you for your patches. Those were very helpful to apply and you saved us a lot of time. This is an issue and it should be solved within this package.

Using same solution as react-draggable is not a proper way to handle this.