daybrush / moveable

Moveable! Draggable! Resizable! Scalable! Rotatable! Warpable! Pinchable! Groupable! Snappable!
https://daybrush.com/moveable/
MIT License
9.93k stars 616 forks source link

moveable.esm.js returns Uncaught TypeError #917

Closed Unruly-Coder closed 1 year ago

Unruly-Coder commented 1 year ago

Environments

Description

movable.esm.js returns uncaught TypeError.

image

When I try to select an element that is covered by a group overlay (it's not in the group yet, I'm just trying to add it to the group, but it's inside the bounding box of an existing group), I don't use onClickGroupEvent to add the new element. Instead, I have my own selection logic pined to the canvas.

I think this issue may be related to the group dragging start event and attempting to add a new element to the group at the same time.

daybrush commented 1 year ago

@Crazy-Ivan

It does not allow target to be set (added) on dragStart yet.

I'll add the defense logic.

Unruly-Coder commented 1 year ago

@daybrush Thank you 🙏. Just for clearing things up, I think it is happening somewhere in onDragGroupEnd event but there was no drag just a click. And as always -> great library! 👏 👏 👍

daybrush commented 1 year ago

@Crazy-Ivan

release as a beta version. Test it. (0.50.6-beta.0)

Unruly-Coder commented 1 year ago

@daybrush Now I got a slightly different error. It appeared in the same situation as before.

image
daybrush commented 1 year ago

@Crazy-Ivan

Oh. Thank you . Test 0.50.6-beta.1 version.

Unruly-Coder commented 1 year ago

@daybrush unfortunately still got the same error,

image image image

It looks like method getBeforeRenderableDatas returns empty object and method getNextStyle try to read a property nextStyle which is undefined in this case.

image
daybrush commented 1 year ago

@Crazy-Ivan

You are changing target in onClickGroup. Is this possible?

requestAnimationFrame(() => {
    setTargetes(...);
});

Changing the target asynchronously.

Unruly-Coder commented 1 year ago

Nope, I don't use onClickGroup. I have something like this (simplified version)

<Canvas onMouseDown = {selectElementFromCursor}>
  {elements.map(element => <Element data={element.data} ref={setElementRef} / >)}

  <Moveable
        zoom={1 / canvasManager.canvasScale}
        ref={moveableRef}
        className={styles.moveable}
        target={elementReferences}
        viewContainer={document.body}
        snappable={snappable}
        snapThreshold={3}
        snapGap={false}
        stopPropagation={!keyboardManager.shift && !keyboardManager.command}
        useResizeObserver={true}
        useMutationObserver={true}
        elementGuidelines={elementGuidelines}
        elementSnapDirections={{ left: true, top: true, right: true, bottom: true, center: true, middle: true }}
        snapDirections={{ left: true, top: true, right: true, bottom: true, center: true, middle: true }}
        verticalGuidelines={verticalGuidelines}
        horizontalGuidelines={horizontalGuidelines}
        origin={false}
        draggable={true}
        throttleDrag={dragThrottle}
        onDrag={onDragHandler}
        onDragEnd={onSingleEventEndHandler}
        onDragGroup={onDragGroupHandler}
        onDragGroupEnd={onGroupEventEndHandler}
        resizable={true}
        edge={true}
        linePadding={10}
        throttleResize={0}
        onResizeStart={onResizeStartHandler}
        onResize={onResizeHandler}
        onResizeEnd={onSingleEventEndHandler}
        onResizeGroup={onResizeGroupHandler}
        onResizeGroupStart={onGroupEventStartHandler}
        onResizeGroupEnd={onGroupEventEndHandler}
        keepRatio={keepRatio}
        rotatable={true}
        rotationPosition={"none"}
        rotateAroundControls={true}
        throttleRotate={rotationThrottle}
        onRotateStart={onRotateStartHandler}
        onRotate={onRotateHandler}
        onRotateEnd={onSingleEventEndHandler}
        onRotateGroup={onRotateGroupHandler}
        onRotateGroupEnd={onGroupEventEndHandler}
        onRotateGroupStart={onGroupEventStartHandler}
      />
 </Canvas>

selectElementFromCursor is finding element base on cursor position and adds it to the array of selected elements. This array then is mapped to array of html references which is passed to Moveable component.

I try to remove all the props that potentially could lead to this issue like this:

<Moveable
        ref={moveableRef}
        target={elementReferences}
        viewContainer={document.body}
        draggable={true}
        resizable={true}
        rotatable={true}
   />

but still get same error. However, I find out that the only way to not getting it is to set draggable to false. Which could be a solution for me as I allow multi select elements only when shift key is pressed.

daybrush commented 1 year ago

@Crazy-Ivan

Perhaps somewhere in the end event there is code where the target changes. I think that changing to synchronous causes an error.

Test 0.50.6-beta.2 veresion. Added code to forcibly end the event when the target changes.

Unruly-Coder commented 1 year ago

https://github.com/daybrush/moveable/assets/4831814/09580cee-624b-453b-9251-6fbff9653742

The error not visible anymore but now after selecting fourth element I can't drag it anymore either.

daybrush commented 1 year ago

@Crazy-Ivan

Can you check whether it execute by taking a console.log in onDragGroupStart?

I don't know why the 4th problem occurs. The code I tested is as follows: Are there structural differences with your code?

import * as React from "react";
import Moveable from "@/react-moveable";

export default function App() {
    const [targets, setTargets] = React.useState<Array<string>>([
        ".cube1",
        ".cube2",
    ]);
    const moveableRef = React.useRef<Moveable>(null);
    const cubes = [];

    for (let i = 0; i < 30; ++i) {
        cubes.push(i);
    }

    return <div className="root">
        <div className="container">
            <Moveable
                ref={moveableRef}
                draggable={true}
                target={targets}
                onClickGroup={() => {
                    if (targets.length === 2) {
                        setTargets([".cube1", ".cube2", ".cube3"]);
                    } else if (targets.length === 3) {
                        setTargets([".cube1", ".cube2", ".cube3", ".cube4"]);
                    }
                }}
                onDrag={e => {
                    e.target.style.cssText += e.cssText;
                }}
                onDragGroup={e => {
                    e.events.forEach(ev => {
                        ev.target.style.cssText += ev.cssText;
                    });
                }}
            ></Moveable>

            <div className="elements selecto-area">
                {cubes.map(i => <div className={`cube cube${i}`} key={i}></div>)}
            </div>
            <div className="empty elements"></div>
        </div>
    </div>;
}
Unruly-Coder commented 1 year ago

https://github.com/daybrush/moveable/assets/4831814/d12f7483-c260-493b-96a2-682017e3880b

The onClickGroup event stops working after selecting an element inside the bounding box of a group selection. My implementation is similar, but there are some differences. I don't use class names to define targets, but rather HTML element instances. However, I don't think it is a problem here. I don't use onClickGroup to select elements inside the group, but I have onMouseDown event attached to the container where I have the entire selection (and deselection) logic. So, my selection logic is working totally outside the Moveable event system. I assume the problem is that the dragEvent starts first, as it is also attached to the onMouseDown event, and then my selection logic is fired after, which updates the arrays of selected targets. So, the number of selected elements is different between the start and end drag from the Moveable point of view. Drag Start will fire with three targets, and dragEnd will have four targets. I could use MouseDownCapture event, so my selection event will fire before the Moveable drag event, but then I can't use stopPropagation, which is handy when I want to really drag the group by its bounding box.

Edit: Sorry, I console.log a wrong event. You asked about onDragGroupStart.
https://github.com/daybrush/moveable/assets/4831814/44b7f3fd-f9a5-40c3-a70f-1ac4b0952945

But the effect is the same.

daybrush commented 1 year ago

@Crazy-Ivan

I haven't figured out why yet. But thanks for continuing to help me figure out the cause.

When it doesn't work, take a picture of the specific properties of the moveable's Ref to the console.

    React.useEffect(() => {
        console.log(moveableRef.current?.moveable);
    }, [targets]);

Can you show me like the picture below?

스크린샷 2023-05-15 오후 11 38 58

Or is it possible an address I can test?

Unruly-Coder commented 1 year ago

@daybrush sure!

I have made simplified example of my implementation: https://codesandbox.io/s/purple-bush-uhyv10?file=/src/App.tsx

daybrush commented 1 year ago

@Crazy-Ivan

There seems to be no problem. Test with 0.50.7-beta.0.

https://codesandbox.io/s/hidden-microservice-9jg8t7?file=/src/App.tsx:2828-2842

Unruly-Coder commented 1 year ago

@daybrush Yep, there seems to be no problem, and it works as expected 👍. However, I'm still experiencing the issue in my target implementation 🤔 . There must be a specific detail or aspect in my target implementation that causes it to behave what's shown in the attached videos. I'll keep digging and tring to recreate this behavior in outisde my project. I discovered that when I select the fourth element by clicking on the group bounding box, the .movable-area div receives the .moveable-avoid class. Additionally, the draggable container starts to be covered by .moveable-area-piece. This only happens when I attempt to select the element by clicking on the group bounding box, and it remains that way until I deselect all elements. This is the main reason why the dragging is not working. If i remove the .moveable-avoid class and hide .moveable-area-piece all backs to normal behavior. Perhaps you can give me a hint about the situation in which these elements are added?

https://github.com/daybrush/moveable/assets/4831814/21a8bf11-d59b-466d-8e24-a2278f78a6d7

daybrush commented 1 year ago

@Crazy-Ivan

thanks. I found a solution from your video. Test with version 0.50.7. Now it will really work.

Unruly-Coder commented 1 year ago

@daybrush Perfect! 👍 🥇 🎉