fix: fix icon scale reset during attention animation#1588
fix: fix icon scale reset during attention animation#1588deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR adjusts the attention animation behavior for dock taskmanager app items by deferring the reset of the icon’s scale until after the attention animation deactivates, using Qt.callLater to avoid race conditions or binding loops. Sequence diagram for attention animation icon scale resetsequenceDiagram
actor User
participant DockPanel
participant AppItem
participant AttentionAnimation
participant Icon
participant Qt
User->>DockPanel: click or event triggers attention
DockPanel->>AppItem: set attention = true
AppItem->>AttentionAnimation: active = true
AttentionAnimation-->>Icon: animate scale > 1.0
User->>DockPanel: condition cleared
DockPanel->>AppItem: set attention = false
AppItem->>AttentionAnimation: active = false
AttentionAnimation->>AppItem: onActiveChanged(active = false)
AppItem->>Qt: callLater(resetIconScale)
Qt-->>AppItem: invoke resetIconScale()
AppItem->>Icon: set scale = 1.0
Class diagram for AppItem attention animation and icon scale resetclassDiagram
class AppItem {
bool attention
bool isDragging
void onAttentionAnimationActiveChanged(bool active)
}
class AttentionAnimation {
bool active
void onActiveChanged(bool active)
}
class Icon {
float scale
}
class QtHelper {
void callLater(function callback)
}
AppItem --> AttentionAnimation : controls active
AttentionAnimation --> Icon : animates scale
AppItem --> Icon : resets scale to 1_0
AppItem --> QtHelper : uses callLater
%% Implementation detail
AppItem : onAttentionAnimationActiveChanged(active)
AppItem : if active == false
AppItem : Qt.callLater(resetIconScale)
AppItem : resetIconScale()
AppItem : icon.scale = 1_0
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 found 1 issue, and left some high level feedback:
- Using Qt.callLater here avoids the immediate binding issue, but it also introduces a window where the animation could become active again before the callback runs; consider re-checking
active(orroot.attention && ...) inside the callback before resettingicon.scaleto avoid resetting while the animation is active again. - To make the intent clearer for future maintainers, consider adding a brief comment above this Qt.callLater usage explaining that it is specifically to avoid binding loops/ordering issues when mutating
icon.scalefromonActiveChanged.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using Qt.callLater here avoids the immediate binding issue, but it also introduces a window where the animation could become active again before the callback runs; consider re-checking `active` (or `root.attention && ...`) inside the callback before resetting `icon.scale` to avoid resetting while the animation is active again.
- To make the intent clearer for future maintainers, consider adding a brief comment above this Qt.callLater usage explaining that it is specifically to avoid binding loops/ordering issues when mutating `icon.scale` from `onActiveChanged`.
## Individual Comments
### Comment 1
<location path="panels/dock/taskmanager/package/AppItem.qml" line_range="308" />
<code_context>
onActiveChanged: {
if (!active) {
- icon.scale = 1.0
+ Qt.callLater(function() {
+ icon.scale = 1.0
+ })
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against `icon` being invalid when the deferred callback runs
Because `Qt.callLater` runs asynchronously, `icon` may no longer exist by the time the callback executes (e.g., if the item is removed or the component is reloaded). Add a null check inside the callback (e.g., `if (icon)`) before accessing `icon.scale` to avoid runtime errors.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Use Qt.callLater to safely reset icon scale when attention animation deactivates. This prevents potential race conditions or binding loops that could occur when directly resetting the scale property in the onActiveChanged handler. Log: Fixed icon scale reset behavior during attention animation Influence: 1. Test attention animation activation and deactivation sequence 2. Verify icon scale correctly resets to 1.0 after animation ends 3. Ensure no visual glitches or delayed scale updates 4. Test dragging behavior does not interfere with scale reset fix: 修复注意动画期间图标缩放重置问题 在注意动画停用时使用 Qt.callLater 安全地重置图标缩放。避免在 onActiveChanged 处理器中直接重置 scale 属性时可能出现的竞态条件或绑定循 环问题。 Log: 修复注意动画过程中的图标缩放重置逻辑 Influence: 1. 测试注意动画的激活和停用序列 2. 验证动画结束后图标缩放正确重置为 1.0 3. 确保没有视觉异常或缩放更新延迟 4. 测试拖动行为不影响缩放重置 PMS: TASK-389209
deepin pr auto review这段,针对这段 1. 代码逻辑与语法分析修改内容: 逻辑分析:
2. 代码质量
3. 代码性能
4. 代码安全
总结与改进建议这段代码修改是正确且必要的,它解决了 QML 动画属性绑定中常见的时序冲突问题。 改进建议: 为了提高代码的可读性和可维护性,建议添加简短的注释说明原因。 onActiveChanged: {
if (!active) {
// 使用 Qt.callLater 延迟执行,避免与正在进行的动画产生冲突
Qt.callLater(function() {
icon.scale = 1.0
})
}
}或者,如果项目风格允许,可以使用箭头函数(如果 QML 环境支持 ES6+ 语法)使代码更紧凑: onActiveChanged: {
if (!active) {
// 延迟重置缩放,确保动画流程结束
Qt.callLater(() => icon.scale = 1.0)
}
}总体而言,这是一个高质量的 Bug 修复或优化提交。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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: behind) |
Use Qt.callLater to safely reset icon scale when attention animation deactivates. This prevents potential race conditions or binding loops that could occur when directly resetting the scale property in the onActiveChanged handler.
Log: Fixed icon scale reset behavior during attention animation
Influence:
fix: 修复注意动画期间图标缩放重置问题
在注意动画停用时使用 Qt.callLater 安全地重置图标缩放。避免在
onActiveChanged 处理器中直接重置 scale 属性时可能出现的竞态条件或绑定循
环问题。
Log: 修复注意动画过程中的图标缩放重置逻辑
Influence:
PMS: TASK-389209
Summary by Sourcery
Bug Fixes: