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

onDragging event on PanelResizeHandle does not fire correctly #388

Closed
paulm17 opened this issue Aug 19, 2024 · 5 comments · Fixed by #391
Closed

onDragging event on PanelResizeHandle does not fire correctly #388

paulm17 opened this issue Aug 19, 2024 · 5 comments · Fixed by #391

Comments

@paulm17
Copy link

paulm17 commented Aug 19, 2024

Similar to: #94

Sandbox: [...]

Dragging the handles does not fire the onDragging event. However, if you mouseover the handle, it will.

Could be a regression?

@paulm17
Copy link
Author

paulm17 commented Aug 19, 2024

Found this sandbox: https://codesandbox.io/p/sandbox/react-resizable-panels-forked-di6vn0?file=%2Fsrc%2FResizeHandle.tsx

In an earlier version, it's working as expected.

@paulm17
Copy link
Author

paulm17 commented Aug 20, 2024

I ended up forking the project, to see if I can resolve.

By doing so, I found out I didn't need onDragging but also an onDragStart and onDragEnd.

https://github.com/bvaughn/react-resizable-panels/blob/main/packages/react-resizable-panels/src/PanelResizeHandle.ts#L40

I added the following props to PanelResizeHandleProps:

  onDragStart,
  onDragEnd,

But I also fixed the onDrag issue as well. Now fires correctly and not on mouseover/out.

https://github.com/bvaughn/react-resizable-panels/blob/main/packages/react-resizable-panels/src/PanelResizeHandle.ts#L129

const setResizeHandlerState = (
      action: ResizeHandlerAction,
      isActive: boolean,
      event: ResizeEvent
    ) => {
      if (isActive) {
        switch (action) {
          case "down": {
            setState("drag");

            onDragStart?.();

            startDragging(resizeHandleId, event);

            const { onDragging } = callbacksRef.current;
            if (onDragging) {
              onDragging(true);
            }
            break;
          }
          case "move": {
            const { state } = committedValuesRef.current;

            if (state !== "drag") {
              setState("hover");
            } else if (state === "drag") {
              const { onDragging } = callbacksRef.current;
              if (onDragging) {
                onDragging(true);
              }
            }            

            resizeHandler(event);
            break;
          }
          case "up": {
            setState("hover");

            onDragEnd?.();
            stopDragging();

            if (state !== "inactive") {
              const { onDragging } = callbacksRef.current;
              if (onDragging) {
                onDragging(false);
              }
            }
            break;
          }
        }
      } else {
        setState("inactive");
      }
    };

@paulm17
Copy link
Author

paulm17 commented Aug 20, 2024

Forgot to say. Thank you for creating this library.

Out of all the libraries out there. I've tested all of them...

This one is the best one, bar none!

@bvaughn
Copy link
Owner

bvaughn commented Aug 21, 2024

The onDragging handle is supposed to be called once when dragging starts (with a param of true) and once more when it ends (with a param of false). It looks like it's currently calling onDragging(false) too often because it's not checking the previous action. This regressed with #374 so thanks for the bug report. I believe I've fixed the regression in #391.

By doing so, I found out I didn't need onDragging but also an onDragStart and onDragEnd.

onDragStart/onDragEnd sounds like the same behavior as onDragging(true)/onDragging(false), just with a different name 😄 I'll just keep the current name to avoid making backwards-breaking changes.

Forgot to say. Thank you for creating this library.

You're welcome!

@bvaughn
Copy link
Owner

bvaughn commented Aug 21, 2024

Bugfix published in version 2.1.1


❤️ → ☕ givebrian.coffee

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 a pull request may close this issue.

2 participants