fix: sync dock position before position change operations#1477
fix: sync dock position before position change operations#1477deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSynchronizes 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 handlingsequenceDiagram
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
Flow diagram for updated onPositionChanged logicflowchart 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]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
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.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
8b78689 to
031a1b5
Compare
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 pr auto review这段代码的修改主要涉及 QML 文件中关于 Dock 栏位置变化时的动画处理逻辑。下面是对这段 diff 的详细审查和改进建议: 代码变更分析
审查意见1. 语法逻辑优点:
潜在问题:
2. 代码质量优点:
改进建议: // 建议添加注释说明为什么需要提前设置这些值
// 提前设置位置动画参数,确保在任何情况下都有正确的初始值
dock.positionForAnimation = Panel.position;
// 调整拖拽区域锚点,为可能的动画做准备
changeDragAreaAnchor();3. 代码性能优点:
潜在问题:
4. 代码安全优点:
改进建议:
综合改进建议// 添加状态检查
if (!dock || !Panel) {
console.error("Dock or Panel not initialized");
return;
}
// 提前设置位置动画参数,确保在任何情况下都有正确的初始值
dock.positionForAnimation = Panel.position;
// 只在位置真正变化时调整拖拽区域
if (isPositionChanging) {
changeDragAreaAnchor();
}
// If this was a hide animation during position change, prepare for show animation
if (isPositionChanging && !isShowing) {
isPositionChanging = false;
// Set transform to hidden position before showing
setTransformToHiddenPosition();
// ... 其余代码
}总结这段代码的修改总体上是合理的,提高了代码的一致性和可维护性。主要改进点在于将位置动画的准备工作提前,确保状态管理更加一致。建议添加适当的注释和状态检查,以提高代码的健壮性和可维护性。 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unknown) |
Log: Fixed dock position synchronization issue during panel position changes
Influence:
fix: 修复停靠栏位置在位置变更操作前同步问题
Log: 修复了面板位置变化期间停靠栏位置同步问题
Influence:
PMS: BUG-351781
Summary by Sourcery
Bug Fixes: