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

useDrop child trigger avoidance does not work reliably #2036

Open
nh2 opened this issue Jan 28, 2023 · 3 comments
Open

useDrop child trigger avoidance does not work reliably #2036

nh2 opened this issue Jan 28, 2023 · 3 comments
Assignees

Comments

@nh2
Copy link

nh2 commented Jan 28, 2023

In the code there's a helpful link (great!) that explains what useDrop() does to avoid the child dragleave problem:

// https://stackoverflow.com/a/26459269

That link expanded: https://stackoverflow.com/questions/7110353/html5-dragleave-fired-when-hovering-a-child-element/26459269#26459269

So apparently the code is trying to guard against duplicate events when the drag moves over child elements (nice). I think it's not documented on https://ahooks.js.org/hooks/use-drop though (would be good if it were).

However, this approach does not work well.

As one commenter says:

This doesn't help tho for the problem of duplicate dragenter events

Indeed, see this example:

https://codesandbox.io/s/basic-usage-forked-z966n5

When you drag in between multiple child elements, onDragEnter is fired repeatedly; video:

ahooks-usedrop-repeated-dragenter.mp4

This can result in flickering if you do GUI operations in onDragEnter.

@nh2
Copy link
Author

nh2 commented Jan 28, 2023

Another issue that the the current implementation causes is that if you have nested drags, the outer one becomes incorrect.

Consider this example:

https://codesandbox.io/s/basic-usage-forked-sd3hi7

ahooks-usedrop-on-body-incorrect.mp4

Here I'm doing:

  const [isBodyDragActive, setIsBodyDragActive] = useState(false);
  const [isHovering, setIsHovering] = useState(false);

  const dropRef = useRef(null);

  useDrop(document.body, {
    onDragEnter: () => {
      console.log("onDragEnter body");
      setIsBodyDragActive(true);
    },
    onDragLeave: () => {
      console.log("onDragLeave body");
      setIsBodyDragActive(false);
    }
  });

  useDrop(dropRef, {
    onDragEnter: () => {
      console.log("onDragEnter");
      setIsHovering(true);
    },
    onDragLeave: () => {
      console.log("onDragLeave");
      setIsHovering(false);
    }
  });

As shown in the video, Body drag active unexpectedly becomes false as soon we drag the file onto the <div>. It should stay true.

@nh2
Copy link
Author

nh2 commented Jan 28, 2023

Please consider this proposed fix:

Use the first solution from the StackOverflow question linked that uses counter++ instead of the one you're using currently from the same StackOverflow question.

I've implemented it in a small wrapper for example purposes:

function useDragIgnoringChildren(
  target: BasicTarget, // accept `() => Element` such that elements that may not exist on first render can be used (such as the ones from `useRef()` + `<someTag ref={ref}>`). Needed because React hooks cannot be declared conditionally, so `const x = someCondition ? useX() : ...` is illegal.
  options?: {
    onChange?: (isDragActive: boolean) => void; // provided in case the user wants to get it not as a returned React value
  }
): { isDragActive: boolean } {
  const [isDragActive, setIsDragActive] = React.useState(false);

  function changeDrag(active: boolean) {
    setIsDragActive(active);
    options?.onChange?.(active);
  }

  // Inspired by:
  // https://github.com/alibaba/hooks/blob/375cc75646a3bcd02cde157739e35868833f0a04/packages/hooks/src/useDrop/index.ts#L25-L27
  useEffectWithTarget(
    () => {
      const targetElement = getTargetElement(target);

      if (!targetElement?.addEventListener) {
        return;
      }

      // From: https://stackoverflow.com/questions/7110353/html5-dragleave-fired-when-hovering-a-child-element/21002544#21002544
      let counter = 0;

      function handleDragEnter(_ev: Event) {
        counter++;
        changeDrag(true);
      }
      function handleDragLeave(_ev: Event) {
        counter--;
        if (counter === 0) {
          changeDrag(false);
        }
      }
      function handleDrop(_ev: Event) {
        counter = 0;
        changeDrag(false);
      }

      // Use `capture: true` so that child elements cannot hide the event from us
      // using `stopPropagation()` in the bubble phase.
      const listenerOtions = { capture: true };

      targetElement.addEventListener(
        "dragenter",
        handleDragEnter,
        listenerOtions
      );
      targetElement.addEventListener(
        "dragleave",
        handleDragLeave,
        listenerOtions
      );
      targetElement.addEventListener("drop", handleDrop, listenerOtions);

      // Cleanup
      return () => {
        targetElement.removeEventListener(
          "dragenter",
          handleDragEnter,
          listenerOtions
        );
        targetElement.removeEventListener(
          "dragleave",
          handleDragLeave,
          listenerOtions
        );
        targetElement.removeEventListener("drop", handleDrop, listenerOtions);
      };
    },
    [],
    target
  );

  return { isDragActive };
}

I'm proposing to use just the same approach, not the wrapper.

Behaviour of my proposed solution

https://codesandbox.io/s/basic-usage-forked-x17z9n

ahooks-usedrop-on-body-proposed-fix.mp4

@nh2
Copy link
Author

nh2 commented Jan 29, 2023

Please see my full proposed alternative implementation of useDrop() (called useDropCounting() for now) here:

To test the behaviour:

Code copy-pasted here:

import useLatest from "ahooks/lib/useLatest";
import type { BasicTarget } from "ahooks/lib/utils/domTarget";
import { getTargetElement } from "ahooks/lib/utils/domTarget";
import useEffectWithTarget from "ahooks/lib/utils/useEffectWithTarget";

export interface Options {
  onFiles?: (files: File[], event?: React.DragEvent) => void;
  onUri?: (url: string, event?: React.DragEvent) => void;
  onDom?: (content: any, event?: React.DragEvent) => void;
  onText?: (text: string, event?: React.ClipboardEvent) => void;
  onDragEnter?: (event?: React.DragEvent) => void;
  onDragOver?: (event?: React.DragEvent) => void;
  onDragLeave?: (event?: React.DragEvent) => void;
  onDrop?: (event?: React.DragEvent) => void;
  onPaste?: (event?: React.ClipboardEvent) => void;
}

const useDropCounting = (target: BasicTarget, options: Options = {}) => {
  const optionsRef = useLatest(options);

  useEffectWithTarget(
    () => {
      // From: https://stackoverflow.com/questions/7110353/html5-dragleave-fired-when-hovering-a-child-element/21002544#21002544
      let counter = 0;

      const targetElement = getTargetElement(target);
      if (!targetElement?.addEventListener) {
        return;
      }

      const onData = (
        dataTransfer: DataTransfer,
        event: React.DragEvent | React.ClipboardEvent
      ) => {
        const uri = dataTransfer.getData("text/uri-list");
        const dom = dataTransfer.getData("custom");

        if (dom && optionsRef.current.onDom) {
          let data = dom;
          try {
            data = JSON.parse(dom);
          } catch (e) {
            data = dom;
          }
          optionsRef.current.onDom(data, event as React.DragEvent);
          return;
        }

        if (uri && optionsRef.current.onUri) {
          optionsRef.current.onUri(uri, event as React.DragEvent);
          return;
        }

        if (
          dataTransfer.files &&
          dataTransfer.files.length &&
          optionsRef.current.onFiles
        ) {
          optionsRef.current.onFiles(
            Array.from(dataTransfer.files),
            event as React.DragEvent
          );
          return;
        }

        if (
          dataTransfer.items &&
          dataTransfer.items.length &&
          optionsRef.current.onText
        ) {
          dataTransfer.items[0].getAsString((text) => {
            optionsRef.current.onText!(text, event as React.ClipboardEvent);
          });
        }
      };

      const onDragEnter = (event: React.DragEvent) => {
        event.preventDefault();
        counter++;
        optionsRef.current.onDragEnter?.(event);
      };

      const onDragOver = (event: React.DragEvent) => {
        event.preventDefault();
        optionsRef.current.onDragOver?.(event);
      };

      const onDragLeave = (event: React.DragEvent) => {
        counter--;
        if (counter === 0) {
          optionsRef.current.onDragLeave?.(event);
        }
      };

      const onDrop = (event: React.DragEvent) => {
        event.preventDefault();
        counter = 0;
        onData(event.dataTransfer, event);
        optionsRef.current.onDrop?.(event);
      };

      const onPaste = (event: React.ClipboardEvent) => {
        onData(event.clipboardData, event);
        optionsRef.current.onPaste?.(event);
      };

      targetElement.addEventListener("dragenter", onDragEnter as any);
      targetElement.addEventListener("dragover", onDragOver as any);
      targetElement.addEventListener("dragleave", onDragLeave as any);
      targetElement.addEventListener("drop", onDrop as any);
      targetElement.addEventListener("paste", onPaste as any);

      return () => {
        targetElement.removeEventListener("dragenter", onDragEnter as any);
        targetElement.removeEventListener("dragover", onDragOver as any);
        targetElement.removeEventListener("dragleave", onDragLeave as any);
        targetElement.removeEventListener("drop", onDrop as any);
        targetElement.removeEventListener("paste", onPaste as any);
      };
    },
    [],
    target
  );
};

export default useDropCounting;

@liuyib liuyib self-assigned this Feb 23, 2023
@liuyib liuyib added this to To do in ahooks tasks via automation Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

2 participants