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

[Bug]: Inconsistency of events when more buttons are used #10027

Open
7 tasks done
sushuier opened this issue Jul 27, 2024 · 12 comments
Open
7 tasks done

[Bug]: Inconsistency of events when more buttons are used #10027

sushuier opened this issue Jul 27, 2024 · 12 comments

Comments

@sushuier
Copy link

CheckList

  • I agree to follow this project's Code of Conduct
  • I have read and followed the Contributing Guide
  • I have read and followed the Issue Tracker Guide
  • I have searched and referenced existing issues and discussions
  • I am filing a BUG report.
  • I have managed to reproduce the bug after upgrading to the latest version
  • I have created an accurate and minimal reproduction

Version

5.3.0

In What environments are you experiencing the problem?

Chrome

Node Version (if applicable)

14.20.0

Link To Reproduction

https://codesandbox.io/p/devbox/fabric-vanillajs-sandbox-forked-e5r48u?file=%2Fsrc%2Findex.ts

Steps To Reproduce

中文:在画布上绘制任意一个图形,点击鼠标左键选中后,点击鼠标右键,松开鼠标左键和右键;图形会跟随鼠标移动;
chatgpt translate:Draw any shape on the canvas. After selecting it by clicking the left mouse button, click the right mouse button and release both the left and right mouse buttons; the shape will follow the mouse movement.

Expected Behavior

中文:点击鼠标右键,不要影响到左键的正常拖拽操作
chatgpt translate: Clicking the right mouse button should not affect the normal drag operation of the left mouse button.

Actual Behavior

code:image
test video:

2024-07-28.00-25-23.mp4

Error Message & Stack Trace

No response

@zhe-he
Copy link
Contributor

zhe-he commented Aug 6, 2024

嗯。。。 鼠标右键的点击也会触发你设置的mouse:down, mouse:move 和 mouse:up。 你可以通过opt.e.button判断。它的值可以是以下之一:
0:主鼠标按钮(通常是左键)
1:中间鼠标按钮(通常是滚轮)
2:次鼠标按钮(通常是右键)

// translate
Well... A right-click of the mouse will also trigger the mouse:down, mouse:move, and mouse:up events you set. You can determine this by checking opt.e.button. Its value can be one of the following:
0: Primary mouse button (usually the left button)
1: Middle mouse button (usually the scroll wheel)
2: Secondary mouse button (usually the right button)

@asturur
Copy link
Member

asturur commented Aug 6, 2024

This is something to fix, but is not trivial, all the interactions are built around the left mouse button and that is very restrictive for mixed events

@asturur asturur changed the title [Bug]: [Bug]: Inconsistency of events when more buttons are used Aug 6, 2024
@asturur
Copy link
Member

asturur commented Aug 6, 2024

There are also other issues on the same topic

@sushuier
Copy link
Author

@zhe-he 试过,无效

@zhe-he
Copy link
Contributor

zhe-he commented Aug 16, 2024

@sushuier
我照着你的图片写了份,你试试这种方法是不是可以解决你的问题。
I wrote a version based on your picture. You can try this method to see if it solves your problem.

    const down = (e: TPointerEventInfo<MouseEvent>) => {
      if (e.e.button != 0) return;
      let lastPosX = e.e.clientX;
      let lastPosY = e.e.clientY;
      const move = (ev: MouseEvent) => {
        const vpt = canvas.viewportTransform.slice(0) as TMat2D;
        vpt[4] += ev.clientX - lastPosX;
        vpt[5] += ev.clientY - lastPosY;
        canvas.setViewportTransform(vpt);
        canvas.requestRenderAll();
        lastPosX = ev.clientX;
        lastPosY = ev.clientY;
      };

      document.addEventListener("mousemove", move, { passive: true });
      document.addEventListener(
        "mouseup",
        (_) => {
          document.removeEventListener("mousemove", move);
        },
        { once: true }
      );
    };
    canvas.on("mouse:down", down);

demo

@sushuier
Copy link
Author

sushuier commented Sep 6, 2024

@sushuier 我照着你的图片写了份,你试试这种方法是不是可以解决你的问题。

    const down = (e: TPointerEventInfo<MouseEvent>) => {
      if (e.e.button != 0) return;
      let lastPosX = e.e.clientX;
      let lastPosY = e.e.clientY;
      const move = (ev: MouseEvent) => {
        const vpt = canvas.viewportTransform.slice(0) as TMat2D;
        vpt[4] += ev.clientX - lastPosX;
        vpt[5] += ev.clientY - lastPosY;
        canvas.setViewportTransform(vpt);
        canvas.requestRenderAll();
        lastPosX = ev.clientX;
        lastPosY = ev.clientY;
      };

      document.addEventListener("mousemove", move, { passive: true });
      document.addEventListener(
        "mouseup",
        (_) => {
          document.removeEventListener("mousemove", move);
        },
        { once: true }
      );
    };
    canvas.on("mouse:down", down);

demo

已测试,无效;这个bug有操作步骤的;你确定有理解我提的bug吗,操作步骤:
点击鼠标左键,选中元素,然后点击右键(此时左键不松开),然后同时松开左键右键就会发现 元素跟着鼠标跑了

@zhe-he
Copy link
Contributor

zhe-he commented Sep 6, 2024

@sushuier
已解决
Fixed
fix demo

@sushuier
Copy link
Author

sushuier commented Sep 6, 2024

@sushuier 已解决 fix demo

很棒,果然ok了;
可否请教,你是如何找出这个解决方案呢:
canvas._currentTransform = null;
canvas['_target'] = undefined;
canvas['_groupSelector'] = null;

@zhe-he
Copy link
Contributor

zhe-he commented Sep 6, 2024

Shouldn't we add if (button != 0) return; below line 997 and line 787?
Waiting for a fated person to fix this bug.

// if right/middle click just fire events
const { button } = e as MouseEvent;
if (button) {
((this.fireMiddleClick && button === 1) ||
(this.fireRightClick && button === 2)) &&
this._handleEvent(e, 'down');
this._resetTransformEventData();
return;
}

// if right/middle click just fire events and return
// target undefined will make the _handleEvent search the target
const { button } = e as MouseEvent;
if (button) {
((this.fireMiddleClick && button === 1) ||
(this.fireRightClick && button === 2)) &&
this._handleEvent(e, 'up');
this._resetTransformEventData();
return;
}

@zhe-he
Copy link
Contributor

zhe-he commented Sep 6, 2024

@sushuier
鼠标按下的时候,内部会设置临时变量
鼠标移动的时候,内部会利用临时变量
鼠标抬起的时候,内部会重置临时变量

你列举的这些,是我上面说到的临时变量的一部分。

When the mouse is pressed down, a temporary variable will be set internally.
When the mouse is moved, the internal system will utilize the temporary variable.
When the mouse is released, the temporary variable will be reset internally.
The items you listed are part of the temporary variable I mentioned above.

@asturur
Copy link
Member

asturur commented Sep 7, 2024

This issue needs to be fixed in a way that if you press more buttons the events fire anyway.
Also you had a discussion in a language i can't read, i m stopped looking honestly

@zhe-he
Copy link
Contributor

zhe-he commented Sep 11, 2024

This issue needs to be fixed in a way that if you press more buttons the events fire anyway.
Also you had a discussion in a language i can't read, i m stopped looking honestly

I'm sorry, translating from Chinese to English and then back to Chinese can lead to ambiguity. Therefore, when I encounter Chinese speakers, I use Chinese to avoid misunderstandings. This is my issue.
I have added English to all my responses.
Regarding the "at" in the last reply, I tagged the wrong person.

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

No branches or pull requests

3 participants