fix: fix touchscreen drag interruption issue#719
fix: fix touchscreen drag interruption issue#719deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces DragHandler-based drag logic with a MouseArea-driven drag implementation for IconItemDelegate to better support touch screens, adjusts visibility handling to use opacity, and synchronizes Drag state with MouseArea.drag to avoid interruptions and layout shifts during drag operations. Sequence diagram for the new touchscreen drag handling with MouseAreasequenceDiagram
actor User
participant TouchDevice
participant root as IconItemDelegate_root
participant mouseArea as IconItemDelegate_mouseArea
participant Drag as DragAttached
User->>TouchDevice: Touch on icon
TouchDevice->>mouseArea: press(Qt.LeftButton)
mouseArea->>mouseArea: onPressed(mouse)
mouseArea->>root: check dndEnabled
alt drag_allowed
mouseArea->>root: appIcon.grabToImage(callback)
root-->>mouseArea: image result
mouseArea->>Drag: set root.Drag.imageSource
end
User->>TouchDevice: Move finger
TouchDevice->>mouseArea: drag events
mouseArea->>mouseArea: drag.target = root
mouseArea->>mouseArea: drag.active = true
mouseArea->>Drag: Drag.active bound to mouseArea.drag.active
Drag->>root: Drag.active true
root->>root: opacity = 0
root->>root: state dragged when mouseArea.drag.active
User->>TouchDevice: Release finger
TouchDevice->>mouseArea: release event
mouseArea->>mouseArea: drag.active = false
mouseArea->>Drag: Drag.active false (via binding)
Drag->>root: Drag.active false
root->>root: opacity = 1
root->>root: exit dragged state
Updated class diagram for IconItemDelegate drag handlingclassDiagram
class IconItemDelegate_root {
<<Control>>
opacity : real
property string text
signal menuTriggered()
Drag_dragType : DragType
Drag_active : bool
Drag_imageSource : url
}
class IconItemDelegate_mouseArea {
<<MouseArea>>
anchors_fill_parent : bool
hoverEnabled : bool
acceptedButtons : QtMouseButtons
enabled : bool
drag_target : IconItemDelegate_root
drag_active : bool
onPressed(mouse)
}
class DragAttached {
<<DragAttached>>
dragType : DragType
active : bool
imageSource : url
}
class DragState_dragged {
<<State>>
when : bool ~mouseArea.drag.active~
PropertyChanges_x : real
PropertyChanges_y : real
}
IconItemDelegate_root o-- IconItemDelegate_mouseArea : contains
IconItemDelegate_root o-- DragAttached : has_Drag
IconItemDelegate_root o-- DragState_dragged : has_state
IconItemDelegate_mouseArea --> IconItemDelegate_root : drag_target
IconItemDelegate_mouseArea --> DragAttached : updates_imageSource
DragAttached --> IconItemDelegate_mouseArea : active_bound_to_drag_active
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:
- Switching from
visibletoopacitymeans the item still participates in layout and hit‑testing while invisible; if you intend the icon to be non-interactive during drag, consider also updatingenabledorvisible(oropacity+visiblein a transition) to avoid unexpected input behavior. - The
grabToImagecall is now triggered on every left press instead of only after the drag becomes active, which may be unnecessarily expensive on quick taps; consider deferringgrabToImageuntil the drag actually starts (e.g., in response tomouseArea.drag.activeor movement beyond a threshold).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Switching from `visible` to `opacity` means the item still participates in layout and hit‑testing while invisible; if you intend the icon to be non-interactive during drag, consider also updating `enabled` or `visible` (or `opacity` + `visible` in a transition) to avoid unexpected input behavior.
- The `grabToImage` call is now triggered on every left press instead of only after the drag becomes active, which may be unnecessarily expensive on quick taps; consider deferring `grabToImage` until the drag actually starts (e.g., in response to `mouseArea.drag.active` or movement beyond a threshold).
## Individual Comments
### Comment 1
<location path="qml/windowed/IconItemDelegate.qml" line_range="15" />
<code_context>
Control {
id: root
- visible: !dragHandler.active
+ opacity: Drag.active ? 0 : 1
property string text: display.startsWith("internal/category/") ? getCategoryName(display.substring(18)) : display
</code_context>
<issue_to_address>
**issue (bug_risk):** Using opacity instead of visible may leave the item interactive while visually hidden.
With `visible: !dragHandler.active`, the control was removed from hit-testing while dragging. Using `opacity: Drag.active ? 0 : 1` keeps it fully interactive even when invisible, so clicks/drag/hover can still be processed on a hidden item. If the goal is to hide it functionally during drag, consider restoring the `visible` condition or also disabling input (e.g., via `enabled` or combining `visible` with `opacity`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Replaced DragHandler with MouseArea for drag handling to better support touchscreen interactions 2. Changed visibility control from `visible` to `opacity` to prevent layout reflow during drag operations 3. Updated drag activation logic to use MouseArea's drag state instead of DragHandler's active state 4. Simplified drag initialization by directly setting Drag.imageSource on the root item 5. Fixed drag state synchronization by binding Drag.active to mouseArea.drag.active Log: Fixed touchscreen drag and drop interruption issues Influence: 1. Test touchscreen drag operations on icon items 2. Verify drag visual feedback (opacity changes) during drag operations 3. Test drag and drop functionality with mouse input 4. Verify drag image appears correctly during drag operations 5. Test drag cancellation and completion scenarios 6. Verify no layout shifts occur during drag operations fix: 修复触摸屏拖拽被打断的问题 1. 将 DragHandler 替换为 MouseArea 以更好地支持触摸屏交互 2. 将可见性控制从 `visible` 改为 `opacity`,防止拖拽过程中的布局重排 3. 更新拖拽激活逻辑,使用 MouseArea 的拖拽状态而非 DragHandler 的活动 状态 4. 简化拖拽初始化,直接在根项目上设置 Drag.imageSource 5. 通过将 Drag.active 绑定到 mouseArea.drag.active 来修复拖拽状态同步 问题 Log: 修复触摸屏拖拽被打断的问题 Influence: 1. 测试触摸屏上的图标拖拽操作 2. 验证拖拽过程中的视觉反馈(透明度变化) 3. 测试鼠标输入的拖放功能 4. 验证拖拽过程中拖拽图像正确显示 5. 测试拖拽取消和完成场景 6. 验证拖拽过程中没有布局偏移发生 PMS: BUG-346077
deepin pr auto review#,我对这段 QML 代码的变更进行了审查。以下是从语法逻辑、代码质量、代码性能和代码安全四个方面的详细分析和改进建议: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
综合改进建议代码为了解决上述逻辑、性能和潜在安全问题,建议对代码进行如下调整: Control {
id: root
// 使用 visible 控制显示,opacity 控制拖拽时的透明度
visible: !mouseArea.drag.active
opacity: mouseArea.drag.active ? 0.5 : 1
// ... 其他属性保持不变 ...
// 建议保留或重新引入一个稳定的拖拽代理对象,例如 dndItem
// Item { id: dndItem; visible: false; ... }
Drag.dragType: Drag.Automatic
// 绑定到 MouseArea 的拖拽状态,而不是直接绑定 MouseArea 对象
Drag.active: mouseArea.drag.active
Drag.hotSpot.x: width / 2
Drag.hotSpot.y: height / 2
states: State {
name: "dragged";
when: mouseArea.drag.active
PropertyChanges {
target: root
// 保持原有的重置位置逻辑
}
}
background: Rectangle {
// ...
AppIcon {
id: appIcon
// ...
}
MouseArea {
id: mouseArea
anchors.fill: parent
hoverEnabled: false // 如果不需要悬停效果,保持 false
acceptedButtons: Qt.LeftButton
enabled: root.dndEnabled
// 防止拖拽本身改变 root 的 x/y,除非这是预期行为
// 如果仅用于触发 Drag 附加属性,建议 drag.target: null 或 undefined
// 但原代码逻辑似乎允许拖拽改变位置,这里保持原样
drag.target: root
// 记录按下的初始位置,用于计算阈值
property point pressPos: Qt.point(0, 0)
onPressed: function (mouse) {
if (mouse.button === Qt.LeftButton && root.dndEnabled) {
pressPos = Qt.point(mouse.x, mouse.y)
}
}
onPositionChanged: function (mouse) {
if (!mouseArea.drag.active && root.dndEnabled) {
// 计算移动距离
var dx = mouse.x - pressPos.x
var dy = mouse.y - pressPos.y
// 设置拖拽阈值,例如 5 像素
if (Math.abs(dx) > 5 || Math.abs(dy) > 5) {
// 确认开始拖拽,执行抓取和启动
appIcon.grabToImage(function(result) {
// 安全起见,检查 root 是否仍然有效
if (root) {
// 如果使用了 dndItem,这里应该操作 dndItem.Drag
root.Drag.imageSource = result.url
// 启动拖拽
root.Drag.active = true
}
})
}
}
}
onReleased: {
// 拖拽结束,重置状态
root.Drag.active = false
}
}
}
}总结这段代码的变更试图简化拖拽逻辑,但引入了严重的回归风险(崩溃)和交互逻辑缺陷(点击即抓图)。建议:
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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: blocked) |
support touchscreen interactions
visibletoopacityto preventlayout reflow during drag operations
of DragHandler's active state
on the root item
mouseArea.drag.active
Log: Fixed touchscreen drag and drop interruption issues
Influence:
fix: 修复触摸屏拖拽被打断的问题
visible改为opacity,防止拖拽过程中的布局重排状态
问题
Log: 修复触摸屏拖拽被打断的问题
Influence:
PMS: BUG-346077
Summary by Sourcery
Improve drag-and-drop behavior for icon items to avoid interruptions on touchscreens and prevent layout shifts during drags.
Bug Fixes: