fix: disable tray items during collapse animation#1430
fix: disable tray items during collapse animation#1430deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR prevents tray items from remaining interactive while the tray is in its collapse animation by wiring the delegates’ enabled/input handling to TraySortOrderModel.isCollapsing instead of the collapsed state and by disabling the legacy tray plugin delegate during the animation. Class diagram for tray item delegates and TraySortOrderModelclassDiagram
class TraySortOrderModel {
bool collapsed
bool isCollapsing
}
class TrayItemDelegateChooser {
bool disableInputEvents
string model_sectionType
bool inputEventsEnabled
bool traySurfacePositioner_itemVisible
}
class ActionLegacyTrayPluginDelegate {
bool visible
bool hoverEnabled
bool enabled
bool itemVisible
bool inputEventsEnabled
updatePluginMargins()
}
TrayItemDelegateChooser --> ActionLegacyTrayPluginDelegate : instantiates_delegate
TrayItemDelegateChooser --> TraySortOrderModel : uses_isCollapsing_for_inputEvents
ActionLegacyTrayPluginDelegate --> TraySortOrderModel : uses_isCollapsing_for_enabled
State diagram for tray collapsing behavior and item interactivitystateDiagram-v2
[*] --> Expanded_isCollapsing_false
Expanded_isCollapsing_false --> Collapsing_isCollapsing_true : user_clicks_collapse
Collapsing_isCollapsing_true --> Collapsed_isCollapsing_false : collapse_animation_finished
Collapsed_isCollapsing_false --> Expanding_isCollapsing_true : user_clicks_expand
Expanding_isCollapsing_true --> Expanded_isCollapsing_false : expand_animation_finished
state Expanded_isCollapsing_false {
[*] --> TrayItemsInteractive
}
state Collapsing_isCollapsing_true {
[*] --> TrayItemsDisabled
}
state Collapsed_isCollapsing_false {
[*] --> TrayItemsInteractive
}
state Expanding_isCollapsing_true {
[*] --> TrayItemsDisabled
}
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:
- The
enabled: !DDT.TraySortOrderModel.isCollapsingbinding inActionLegacyTrayPluginDelegateignoresinputEventsEnabledandsectionType, so the enabled state can get out of sync withhoverEnabledand the chooser’sinputEventsEnabledfilter; consider reusing the same composite condition or wiringenableddirectly toinputEventsEnabledto keep interaction logic consistent. - Using the global
DDT.TraySortOrderModel.isCollapsingto driveenabledwill disable all legacy tray delegates whenever any collapse animation runs; if that’s not intended, consider scoping the condition bymodel.sectionTypeor the specific tray section being animated.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `enabled: !DDT.TraySortOrderModel.isCollapsing` binding in `ActionLegacyTrayPluginDelegate` ignores `inputEventsEnabled` and `sectionType`, so the enabled state can get out of sync with `hoverEnabled` and the chooser’s `inputEventsEnabled` filter; consider reusing the same composite condition or wiring `enabled` directly to `inputEventsEnabled` to keep interaction logic consistent.
- Using the global `DDT.TraySortOrderModel.isCollapsing` to drive `enabled` will disable all legacy tray delegates whenever any collapse animation runs; if that’s not intended, consider scoping the condition by `model.sectionType` or the specific tray section being animated.
## Individual Comments
### Comment 1
<location> `panels/dock/tray/package/ActionLegacyTrayPluginDelegate.qml:34` </location>
<code_context>
visible: !Drag.active && itemVisible
hoverEnabled: inputEventsEnabled
+ enabled: !DDT.TraySortOrderModel.isCollapsing
function updatePluginMargins()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider basing `enabled` on the same condition as `inputEventsEnabled` to avoid inconsistent interaction states.
`hoverEnabled` already uses `inputEventsEnabled`, which incorporates `disableInputEvents` and section type, but `enabled` only checks `!isCollapsing`. This can leave the item logically enabled (e.g., keyboard/a11y activation) while hover/pointer input is blocked. Please align `enabled` with the same gating (e.g. `enabled: inputEventsEnabled` or a shared helper) so all interaction paths use consistent conditions.
Suggested implementation:
```
visible: !Drag.active && itemVisible
hoverEnabled: inputEventsEnabled
enabled: inputEventsEnabled
function updatePluginMargins()
{
```
If there are other properties or handlers in this component (e.g. `onClicked`, `Keys.onReturnPressed`) that manually check `DDT.TraySortOrderModel.isCollapsing`, consider refactoring them to use `inputEventsEnabled` as well to keep all interaction paths consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[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 |
1. Added `enabled: !DDT.TraySortOrderModel.isCollapsing` to ActionLegacyTrayPluginDelegate to disable tray items during collapse animation 2. Updated condition in TrayItemDelegateChooser to use `DDT.TraySortOrderModel.isCollapsing` instead of `DDT.TraySortOrderModel.collapsed` for input events 3. This prevents user interaction with tray items while the collapse animation is in progress 4. Fixes potential issues where users could click on items that are being animated or in transition state Log: Fixed tray items remaining interactive during collapse animation Influence: 1. Test tray collapse/expand animation to ensure items are disabled during transition 2. Verify that tray items become interactive again after animation completes 3. Check that input events are properly blocked during collapse animation 4. Test with different tray item types (legacy, modern, fixed, collapsable) 5. Verify that drag operations are not affected by this change fix: 修复托盘项在折叠动画期间保持交互的问题 1. 在 ActionLegacyTrayPluginDelegate 中添加 `enabled: ! DDT.TraySortOrderModel.isCollapsing`,在折叠动画期间禁用托盘项 2. 更新 TrayItemDelegateChooser 中的条件,使 用 `DDT.TraySortOrderModel.isCollapsing` 替代 `DDT.TraySortOrderModel.collapsed` 来控制输入事件 3. 防止用户在折叠动画进行期间与托盘项交互 4. 修复了用户可能点击正在动画或过渡状态中的托盘项的问题 Log: 修复了托盘项在折叠动画期间保持交互的问题 Influence: 1. 测试托盘折叠/展开动画,确保托盘项在过渡期间被禁用 2. 验证托盘项在动画完成后重新变为可交互状态 3. 检查折叠动画期间输入事件是否被正确阻止 4. 测试不同类型的托盘项(传统、现代、固定、可折叠) 5. 验证拖拽操作不受此更改影响 PMS: BUG-342345
7f42f4b to
9772d4f
Compare
|
/forcemerge |
|
This pr force merged! (status: blocked) |
deepin pr auto review这段代码是对 Qt/QML 中托盘区域(Tray)相关组件的修改,主要目的是在折叠动画或状态改变期间禁用用户交互,以防止动画冲突或状态不一致。 以下是对该 diff 的详细审查意见,包括语法逻辑、代码质量、性能和安全性方面的分析与建议: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
5. 综合改进建议如果确认需求是“在折叠动画进行时禁用交互,折叠完成后允许交互(或根据折叠状态决定)”,建议对代码进行如下微调: ActionLegacyTrayPluginDelegate.qml: // 建议明确注释意图,并确保模型引用安全
// 在折叠动画期间禁用整个控件
enabled: DDT.TraySortOrderModel ? !DDT.TraySortOrderModel.isCollapsing : trueTrayItemDelegateChooser.qml: // 建议使用常量替代魔法字符串
inputEventsEnabled: !disableInputEvents &&
(model.sectionType !== DDT.TrayModel.SectionType.Collapsable ||
!DDT.TraySortOrderModel.isCollapsing)总结这段代码的主要改动是合理的,旨在通过引入 |
enabled: !DDT.TraySortOrderModel.isCollapsingto ActionLegacyTrayPluginDelegate to disable tray items during collapse animationDDT.TraySortOrderModel.isCollapsinginstead ofDDT.TraySortOrderModel.collapsedfor input eventsLog: Fixed tray items remaining interactive during collapse animation
Influence:
fix: 修复托盘项在折叠动画期间保持交互的问题
enabled: ! DDT.TraySortOrderModel.isCollapsing,在折叠动画期间禁用托盘项DDT.TraySortOrderModel.isCollapsing替代DDT.TraySortOrderModel.collapsed来控制输入事件Log: 修复了托盘项在折叠动画期间保持交互的问题
Influence:
PMS: BUG-342345
Summary by Sourcery
Prevent tray items from remaining interactive while the tray is collapsing.
Bug Fixes: