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

Filtering onDrag events out when there's no movement. #522

Closed
wants to merge 1 commit into from
Closed

Filtering onDrag events out when there's no movement. #522

wants to merge 1 commit into from

Conversation

johnnybenson
Copy link

@johnnybenson johnnybenson commented Sep 17, 2020

onDrag events should only run when there is change in position.

It's possible to filter these events in "userland" but this should really be the default behavior for the package, IMO

// user land
const onDrag = (event: DraggableEvent, info: DraggableData) => {
  console.log("[ReactDraggable] onDrag");
  const wasDragged = !(info.x === info.lastX && info.y === info.lastY);
  if (wasDragged) {
    console.log("[ReactDraggable] onDragStart", info.deltaX, info.deltaY);
  }
};

@STRML
Copy link
Collaborator

STRML commented Sep 18, 2020 via email

@johnnybenson
Copy link
Author

johnnybenson commented Sep 18, 2020

Hrmm, I don't quite follow, do you mind describing a "real world" scenario?

Maybe I'm just getting caught up in the semantics, here is my expectation for the handlers:
onMouseDown same
onDragStart mouse is down and the first movement occurred
onDrag mouse is down and movement occurs
onDragEnd mouse is up after movement occurred
onMouseUp same
onClick same (mouse down, mouse up, and no movement occurred)

Here's a more complete picture of my user land implementation, where OtherComponent cares about if it's being dragged and has its own onClick handlers. Following the semantics described above, If it's being dragged, it's not being clicked, they're mutually exclusive. Right now, all three drag handler functions appear to behave like click events if no dragging occurs and this additional work is necessary to approximate the semantics laid out above:

const Component = () => {
  const isDraggingRef = React.useRef(false);

  const handleDragStart = (event: DraggableEvent, info: DraggableData) => {
    const wasDragged = !(info.x === info.lastX && info.y === info.lastY);
    if (wasDragged && !isDraggingRef.current) {
      console.log("[ReactDraggable] onDragStart", `deltaX: ${info.deltaX}`, `deltaY: ${info.deltaY}`);
    }
    isDraggingRef.current = true;
  };

  const handleDragEnd = (event: DraggableEvent, info: DraggableData) => {
    if (isDraggingRef.current) {
      console.log("[ReactDraggable] onDragEnd", `X: ${info.x}`, `Y: ${info.y}`);
    }
    isDraggingRef.current = false;
  };

  return (
    <Draggable
      // Note: Do not use onStart, it fires immediately on click, not when dragging begins
      //       Relying on local ref here to simulate onDragStart in the first run of onDrag
      // onStart={handleDragStart}
      onDrag={handleDragStart}
      onStop={handleDragEnd}
    >
      <OtherComponent onClick={doSomething} />
    </Draggable>
  );
};

Can certainly just close this PR if it poses deeper issues, figured sharing this might help anyone else facing similar implementation challenges 😎

@johnnybenson johnnybenson closed this by deleting the head repository Feb 26, 2023
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 this pull request may close these issues.

None yet

2 participants