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

fix(core): 在没有拖拽的情况下,Control组件突然销毁,不触发cancelDrag(#1926) #1938

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

wbccb
Copy link
Contributor

@wbccb wbccb commented Oct 25, 2024

fix #1934
fix #1926

问题发生的原因

为了解决Control组件意外销毁,但是没有及时释放监听事件的问题,在组件销毁时,强制触发了

  componentWillUnmount() {
    this.dragHandler.cancelDrag()
  }

这导致一个问题

  • 用户点击node,会显示出Control组件,点击空白地方会隐藏Control组件,从而触发componentWillUnmount,从而触发cancelDrag()->onDragEnd()->updateEdgePointByAnchors()触发线的重新计算
  • 用户经过node,然后离开node,也会触发触发componentWillUnmount,从而触发cancelDrag()->onDragEnd()->updateEdgePointByAnchors()触发线的重新计算

自动触发计算逻辑一直存在,但是一般是拖拽时才会触发,用户也能理解这个逻辑

但是目前这个组件销毁时就触发一次自动触发计算会给用户造成一种bug的错觉

解决方法

我们的本意是为了防止mousemovemouseup没有及时被移除,如下面所示,在触发handleMouseDown时,会注册监听,在handleMouseUp时,会解除监听

handleMouseDown = (e: MouseEvent) => {
  const DOC: any = window?.document;
  if (e.button !== LEFT_MOUSE_BUTTON_CODE) return;
  if (this.isStopPropagation) e.stopPropagation();
  this.isStartDragging = true;
  //...
  DOC.addEventListener('mousemove', this.handleMouseMove, false)
  DOC.addEventListener('mouseup', this.handleMouseUp, false)
};

handleMouseUp = (e: MouseEvent) => {
  const DOC = window.document;

  this.isStartDragging = false;
  if (this.isStopPropagation) e.stopPropagation();
  // fix #568: 如果onDragging在下一个事件循环中触发,而drop在当前事件循环,会出现问题。
  Promise.resolve().then(() => {
    DOC.removeEventListener("mousemove", this.handleMouseMove, false);
    DOC.removeEventListener("mouseup", this.handleMouseUp, false);
    //...
  });
};

因此这里增加if(this.isStartDragging)的判断,isStartDragging=true代表没有触发handleMouseUp(),此时监听还没被移除;在拖拽情况下(isStartDragging=true),在组件突然销毁时,强制触发cancelDrag()进行监听事件的移除

// packages/core/src/util/drag.ts
destroy = () => {
  if (this.isStartDragging) {
    this.cancelDrag();
  }
};

Copy link

changeset-bot bot commented Oct 25, 2024

⚠️ No Changeset found

Latest commit: fab2468

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@boyongjiong boyongjiong merged commit b4dc07b into didi:master Oct 29, 2024
@wbccb wbccb deleted the fix/resize-dragend branch October 29, 2024 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants