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): 修复多选旋转的节点后进行移动时,节点位置异常问题 #2032

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

ZivvW
Copy link
Contributor

@ZivvW ZivvW commented Jan 2, 2025

多选时的移动未更新 transform 导致旋转的节点位置异常。改成在 getMoveDistance 方法中更新 transform,统一处理所有移动逻辑

before:
chrome-capture-2025-1-2

after:
chrome-capture-2025-1-2 (1)

移动后未更新 transform 导致旋转的节点位置异常。改成在 getMoveDistance 方法中更新 transform
Copy link

changeset-bot bot commented Jan 2, 2025

⚠️ No Changeset found

Latest commit: 6d4d784

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

@ZivvW
Copy link
Contributor Author

ZivvW commented Jan 2, 2025

相关 issue : #1794

情况2:开启旋转,如:旋转后的节点A+未旋转的节点B,一起被框选后移动出现问题(A会跑偏,和整体鼠标移动的方向不一致,单独稍微移动一下A,A就会恢复到正常的位置:也就是整体移动后的正确位置);

@ZivvW ZivvW changed the title fix(core): 修复多选旋转的节点后进行移到时,节点位置异常问题 fix(core): 修复多选旋转的节点后进行移动时,节点位置异常问题 Jan 3, 2025
@DymoneLewis
Copy link
Collaborator

也太酷了,我们看下

@DymoneLewis
Copy link
Collaborator

太赞了,看了一下没问题,马上就过!
不过有点没太理解为什么放到model里就好了,方便的话可以解答一下😄

@DymoneLewis DymoneLewis merged commit bd5b2a9 into didi:master Feb 20, 2025
@ZivvW
Copy link
Contributor Author

ZivvW commented Feb 21, 2025

太赞了,看了一下没问题,马上就过! 不过有点没太理解为什么放到model里就好了,方便的话可以解答一下😄

@DymoneLewis

移动逻辑中,更新 transform 旋转矩阵只在 BaseNode.tsxonDragging 中有更新,只有移动单个节点时能触发。

而多选时的移动是通过 graphModel.moveNodes 触发的,里面没有更新ransform旋转矩阵。

这里的改动是将更新 transform 的逻辑放到了 BaseNodeModel.tsxgetMoveDistance 中,移动相关的最终都会调用这个方法来更新节点位置。所以将更新 transform 的逻辑放到了这里,在节点坐标更新后同步更新变换矩阵。

重新理了下这里的解决方案,感觉可以再优化下。现在 BaseNodeModel 中有三个移动位置相关的方法,movemoveTogetMoveDistance ,他们都实现了一遍移动逻辑,之前只有move 中被加上了同步更新旋转矩阵的逻辑。这里是否将这些移动逻辑合并下比较好?

@DymoneLewis
Copy link
Collaborator

DymoneLewis commented Feb 21, 2025

重新理了下这里的解决方案,感觉可以再优化下。现在 BaseNodeModel 中有三个移动位置相关的方法,movemoveTogetMoveDistance ,他们都实现了一遍移动逻辑,之前只有move 中被加上了同步更新旋转矩阵的逻辑。这里是否将这些移动逻辑合并下比较好?

@ZivvW
看了一下确实有点相似,不过个人觉得合并的话感觉空间不大🤔,你现在的合并思路是什么呢,可以讨论一波😄

@ZivvW
Copy link
Contributor Author

ZivvW commented Feb 21, 2025

重新理了下这里的解决方案,感觉可以再优化下。现在 BaseNodeModel 中有三个移动位置相关的方法,movemoveTogetMoveDistance ,他们都实现了一遍移动逻辑,之前只有move 中被加上了同步更新旋转矩阵的逻辑。这里是否将这些移动逻辑合并下比较好?

@ZivvW 看了一下确实有点相似,不过个人觉得合并的话感觉空间不大🤔,你现在的合并思路是什么呢,可以讨论一波😄

@DymoneLewis 类似这样吧,提取移动逻辑,3个方法调用核心函数返回各自的值,后续要修改移动逻辑的时候不用在 3 个方法里都加一遍

  @action moveCore(
    deltaX: number,
    deltaY: number,
    isIgnoreRule = false,
  ): false | [number, number] {
    const { isAllowMoveX, isAllowMoveY } = this.isAllowMoveByXORY(
      deltaX,
      deltaY,
      isIgnoreRule,
    )
    if (!isAllowMoveX && !isAllowMoveY) return false

    let moveX = 0
    let moveY = 0

    if (isAllowMoveX && deltaX) {
      this.x = this.x + deltaX
      this.text && this.moveText(deltaX, 0)
      moveX = deltaX
    }
    if (isAllowMoveY && deltaY) {
      this.y = this.y + deltaY
      this.text && this.moveText(0, deltaY)
      moveY = deltaY
    }

    // 更新x和y的同时也要更新对应的transform旋转矩阵(依赖x、y)
    this.rotate = this._rotate
    return [moveX, moveY]
  }

  @action move(deltaX: number, deltaY: number, isIgnoreRule = false): boolean {
    return !!this.moveCore(deltaX, deltaY, isIgnoreRule)
  }

  @action getMoveDistance(
    deltaX: number,
    deltaY: number,
    isIgnoreRule = false,
  ): [number, number] {
    const moveResult = this.moveCore(deltaX, deltaY, isIgnoreRule)
    if (!moveResult) return [0, 0]
    return moveResult
  }

  @action moveTo(x: number, y: number, isIgnoreRule = false): boolean {
    const deltaX = x - this.x
    const deltaY = y - this.y
    return this.move(deltaX, deltaY, isIgnoreRule)
  }

另外目前代码中的 moveTo 还缺少更新 transform 逻辑,看了下该方法目前只在 dnd 中有应用,如果在 dnd.startDrag 时传入了 rotate 参数,拖拽时的 fakeNode 也会有异常

@ZivvW ZivvW deleted the fix/multiSelect-rotate-move branch February 22, 2025 07:28
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.

2 participants