Skip to content

fix: sync dock position before position change operations#1477

Open
robertkill wants to merge 1 commit intolinuxdeepin:masterfrom
robertkill:master
Open

fix: sync dock position before position change operations#1477
robertkill wants to merge 1 commit intolinuxdeepin:masterfrom
robertkill:master

Conversation

@robertkill
Copy link
Contributor

@robertkill robertkill commented Mar 4, 2026

  1. Fixed issue where dock.positionForAnimation was not synchronized with Panel.position before cursorShape and changeDragAreaAnchor operations
  2. Moved dock.positionForAnimation assignment earlier in onPositionChanged function to ensure correct position reference
  3. This prevents cursor shape and drag area anchor calculations from using outdated position data during position transitions

Log: Fixed dock position synchronization issue during panel position changes

Influence:

  1. Test dock position changes through settings or drag operations
  2. Verify cursor shape changes correctly during dock repositioning
  3. Check drag area anchors update properly when dock position changes
  4. Test dock visibility transitions during position changes
  5. Verify dock animation behavior when position is changed while dock is hidden

fix: 修复停靠栏位置在位置变更操作前同步问题

  1. 修复了在cursorShape和changeDragAreaAnchor操作之前 dock.positionForAnimation未与Panel.position同步的问题
  2. 将dock.positionForAnimation赋值移到onPositionChanged函数更早的位置, 确保使用正确的位置参考
  3. 防止在位置转换期间,光标形状和拖拽区域锚点计算使用过时的位置数据

Log: 修复了面板位置变化期间停靠栏位置同步问题

Influence:

  1. 测试通过设置或拖拽操作更改停靠栏位置
  2. 验证在停靠栏重新定位期间光标形状正确变化
  3. 检查拖拽区域锚点在停靠栏位置变化时正确更新
  4. 测试位置变化期间的停靠栏可见性转换
  5. 验证停靠栏隐藏时位置变化的动画行为

PMS: BUG-351781

Summary by Sourcery

Bug Fixes:

  • Fix desynchronization between dock.positionForAnimation and Panel.position that could cause incorrect cursor shape, drag area anchors, and visibility behavior when the dock position changes.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 4, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Synchronizes dock.positionForAnimation with Panel.position earlier in the onPositionChanged lifecycle so cursor shape, drag area anchors, and visibility/animation logic use the current dock position during position transitions.

Sequence diagram for updated dock onPositionChanged handling

sequenceDiagram
    participant Panel
    participant Dock as dock
    participant DockAnimation as dockAnimation

    Panel->>Panel: onPositionChanged()
    Panel->>Dock: set positionForAnimation = Panel.position
    Panel->>Dock: changeDragAreaAnchor()
    Panel->>Panel: requestClosePopup()
    Panel->>DockAnimation: check isPositionChanging
    Panel->>Dock: check visible
    alt dockAnimation.isPositionChanging == false and dock.visible == false
        Panel->>DockAnimation: setTransformToHiddenPosition()
        Panel->>Dock: show at new position
    end
Loading

Flow diagram for updated onPositionChanged logic

flowchart TD
    A[onPositionChanged called] --> B[Set dock.positionForAnimation = Panel.position]
    B --> C[changeDragAreaAnchor]
    C --> D[Panel.requestClosePopup]
    D --> E{dockAnimation.isPositionChanging == false<br/>and dock.visible == false}
    E -- Yes --> F[dockAnimation.setTransformToHiddenPosition]
    F --> G[Show dock at new position]
    E -- No --> H[End]
    G --> H[End]
Loading

File-Level Changes

Change Details Files
Ensure dock.positionForAnimation is updated before drag/interaction logic on position change.
  • Assign dock.positionForAnimation = Panel.position at the start of onPositionChanged.
  • Call changeDragAreaAnchor and Panel.requestClosePopup after positionForAnimation is updated so they use the current position.
  • Remove the later, redundant assignment of dock.positionForAnimation inside the visibility/animation conditional to avoid using stale position data.
panels/dock/package/main.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • The new inline comment is quite verbose and includes bug history; consider shortening it to a concise description of the behavior (e.g., “Keep positionForAnimation in sync with Panel.position before cursor/drag area updates”) to avoid embedding PR-specific context in the code.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new inline comment is quite verbose and includes bug history; consider shortening it to a concise description of the behavior (e.g., “Keep positionForAnimation in sync with Panel.position before cursor/drag area updates”) to avoid embedding PR-specific context in the code.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@robertkill robertkill force-pushed the master branch 3 times, most recently from 038cf16 to 8b78689 Compare March 4, 2026 08:53
1. Fixed issue where dock.positionForAnimation was not synchronized with
Panel.position before cursorShape and changeDragAreaAnchor operations
2. Moved dock.positionForAnimation assignment earlier in
onPositionChanged function to ensure correct position reference
3. This prevents cursor shape and drag area anchor calculations from
using outdated position data during position transitions

Log: Fixed dock position synchronization issue during panel position
changes

Influence:
1. Test dock position changes through settings or drag operations
2. Verify cursor shape changes correctly during dock repositioning
3. Check drag area anchors update properly when dock position changes
4. Test dock visibility transitions during position changes
5. Verify dock animation behavior when position is changed while dock
is hidden

fix: 修复停靠栏位置在位置变更操作前同步问题

1. 修复了在cursorShape和changeDragAreaAnchor操作之前
dock.positionForAnimation未与Panel.position同步的问题
2. 将dock.positionForAnimation赋值移到onPositionChanged函数更早的位置,
确保使用正确的位置参考
3. 防止在位置转换期间,光标形状和拖拽区域锚点计算使用过时的位置数据

Log: 修复了面板位置变化期间停靠栏位置同步问题

Influence:
1. 测试通过设置或拖拽操作更改停靠栏位置
2. 验证在停靠栏重新定位期间光标形状正确变化
3. 检查拖拽区域锚点在停靠栏位置变化时正确更新
4. 测试位置变化期间的停靠栏可见性转换
5. 验证停靠栏隐藏时位置变化的动画行为

PMS: BUG-351781
@deepin-ci-robot
Copy link

deepin pr auto review

针对这段 git diff 的代码审查,我将从代码逻辑、代码质量和潜在风险三个方面进行分析。

1. 代码逻辑与行为分析

变更摘要:
这段代码主要调整了 Dock(任务栏)位置改变时的一些触发顺序。具体来说,将 dock.positionForAnimation 的赋值和 changeDragAreaAnchor() 函数的调用从 isPositionChangingif 块内部移到了该块的末尾(即无论 isPositionChanging 状态如何,只要该代码块被执行,这两个操作都会进行)。同时,在 onPositionChanged 函数中移除了 changeDragAreaAnchor() 的调用。

潜在逻辑问题:

  1. dock.positionForAnimation 的赋值时机与副作用

    • 原代码:在 if (isPositionChanging && !isShowing) 块内赋值。这意味着只有当位置正在改变且 Dock 处于隐藏状态时,才更新动画位置。
    • 新代码:移到了 if 块外。这意味着无论 Dock 是否正在改变位置,只要该函数被触发,动画位置就会被强制更新为当前 Panel.position
    • 风险:如果这个函数(包含该代码块的函数)被其他原因触发(不仅仅是位置改变),可能会意外覆盖 positionForAnimation,导致动画跳变或状态不一致。
  2. changeDragAreaAnchor() 的调用位置变更

    • 原代码:在 if 块内和 onPositionChanged 中都有调用。
    • 新代码:只在 if 块外调用一次。
    • 分析:开发者似乎试图合并调用以避免重复或确保顺序。但是,onPositionChanged 是专门响应位置变化的信号。如果将 changeDragAreaAnchor()onPositionChanged 中移除,依赖于该信号来更新拖拽区域锚点的逻辑可能会失效,除非 onPositionChanged 触发的函数链最终会再次执行包含新代码的那个函数。
    • 关键点:如果 onPositionChanged 触发的逻辑路径经过那个包含 changeDragAreaAnchor() 的代码块,那么拖拽区域将无法更新,导致交互错误。

2. 代码质量与可维护性

  1. 缺乏注释说明移动原因

    • 代码移动了关键逻辑,但没有添加注释解释为什么要移动(例如:是为了修复特定 Bug,还是为了优化性能?)。这增加了后续维护的难度。
  2. 逻辑耦合

    • dock.positionForAnimationchangeDragAreaAnchor() 的调用现在被捆绑在一起,且位置发生了变化。如果这两个操作原本是为了应对不同场景,现在强制同步执行可能会引入新的 Bug。

3. 改进建议

  1. 确认调用路径

    • 必须确认 onPositionChanged 的触发流程。如果 onPositionChanged 不会触发包含新代码的函数,那么必须保留 onPositionChanged 中的 changeDragAreaAnchor() 调用,或者确保新代码位置的逻辑覆盖了所有场景。
  2. 条件判断

    • 如果 dock.positionForAnimation 只需要在特定条件下更新,建议保留条件判断,而不是无条件地移出 if 块。例如:
      if (isPositionChanging && !isShowing) {
          // ... 原有逻辑 ...
      } else if (isPositionChanging) {
          // 如果需要在其他情况下也更新,可以增加 else if
          dock.positionForAnimation = Panel.position;
      }
      // 或者明确只在需要时更新
      if (needsAnimationUpdate) {
          dock.positionForAnimation = Panel.position;
      }
  3. 添加防御性代码

    • 为了防止状态不一致,可以在更新 positionForAnimation 之前检查位置是否真的发生了变化:
      if (dock.positionForAnimation !== Panel.position) {
          dock.positionForAnimation = Panel.position;
      }
  4. 代码注释

    • 建议添加注释说明为什么这两个操作需要在这里执行,以及为什么从原来的位置移除。
      // 统一在此处更新动画位置和拖拽区域,确保在显示/隐藏逻辑处理完毕后生效
      dock.positionForAnimation = Panel.position;
      changeDragAreaAnchor();

总结

这段代码修改存在较高的逻辑回归风险。主要风险在于改变了关键状态更新函数(changeDragAreaAnchor)的触发时机和条件。建议在合并前进行充分的测试,特别是针对以下场景:

  1. Dock 在不同位置(左、右、底)之间切换时。
  2. Dock 在隐藏状态下改变位置。
  3. 快速连续改变位置时,拖拽区域是否响应正确。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ivy233, robertkill

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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.

3 participants