fix: enable touch scrolling in launcher#764
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds touch-awareness to launcher icon press handling so that touch-generated mouse events are not consumed by item delegates, allowing Flickable to handle vertical scrolling while preserving existing left-click behavior for real mouse input. Sequence diagram for touch vs mouse press handling in launcher icon viewssequenceDiagram
actor User
participant TouchInput
participant IconMouseArea as IconMouseArea_onPressed
participant Flickable
User ->> TouchInput: touch press on icon
TouchInput ->> IconMouseArea: onPressed(mouse)
alt [mouse.source != 0]
IconMouseArea ->> IconMouseArea: mouse.accepted = false
IconMouseArea -->> Flickable: return
Flickable ->> Flickable: handle vertical scrolling
else [mouse.source == 0]
IconMouseArea ->> IconMouseArea: isTouchLongPressed = false
IconMouseArea ->> IconMouseArea: grabToImage(callback)
IconMouseArea ->> Flickable: (existing icon interactions)
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 found 1 issue, and left some high level feedback:
- The
mouse.source !== 0check uses a magic number; consider comparing against the appropriateQt.MouseEventSourceenum (e.g.,Qt.MouseEventNotSynthesized) to make the intent clearer and less fragile to Qt changes. - The new
onPressedlogic to ignore synthesized touch events is duplicated in bothAppListView.qmlandFreeSortListView.qml; consider extracting this into a shared helper or component to keep the behavior consistent and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `mouse.source !== 0` check uses a magic number; consider comparing against the appropriate `Qt.MouseEventSource` enum (e.g., `Qt.MouseEventNotSynthesized`) to make the intent clearer and less fragile to Qt changes.
- The new `onPressed` logic to ignore synthesized touch events is duplicated in both `AppListView.qml` and `FreeSortListView.qml`; consider extracting this into a shared helper or component to keep the behavior consistent and easier to maintain.
## Individual Comments
### Comment 1
<location path="qml/windowed/FreeSortListView.qml" line_range="354-358" />
<code_context>
onPressed: function (mouse) {
+ // 触屏合成的鼠标事件不接受,交给 Flickable 处理滚动
+ if (mouse.source !== undefined && mouse.source !== 0) {
+ mouse.accepted = false
+ return
+ }
</code_context>
<issue_to_address>
**suggestion:** Avoid using a magic number for the mouse source and rely on the Qt enum instead.
Using `0` here ties the logic to the enum’s current numeric value and reduces readability. Please compare against `Qt.MouseEventNotSynthesized` (or the relevant enum) instead so the intent is clear and the code remains correct if the enum values change.
```suggestion
// 触屏合成的鼠标事件不接受,交给 Flickable 处理滚动
if (mouse.source !== undefined && mouse.source !== Qt.MouseEventNotSynthesized) {
mouse.accepted = false
return
}
```
</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. Update the icon press handling in AppListView.qml and FreeSortListView.qml. 2. Detect synthesized mouse events generated from touch input by checking mouse.source. 3. When the event comes from touch rather than a real mouse, mark it as not accepted and return early so the parent Flickable can handle vertical scrolling. 4. Keep the existing left-click image grab and subsequent interaction logic unchanged for real mouse input. 5. This change is necessary because launcher icons were consuming touch-generated press events, which blocked swipe up/down gestures and prevented smooth scrolling in the app list. Log: Enabled vertical touch scrolling on launcher app icons Influence: 1. Verify that swiping up and down directly on launcher icons scrolls the app list smoothly. 2. Verify that mouse left-click behavior on icons is unchanged, including image grab related interactions. 3. Test long-press behavior on touch devices to ensure it does not regress unexpectedly. 4. Verify scrolling behavior in both AppListView and FreeSortListView. 5. Test with real mouse input and touch input separately to confirm correct event routing. 6. Check that icon click, drag, and reorder related interactions still work as expected after the event acceptance change. fix: 启动器支持图标区域触控滑动 1. 更新了 AppListView.qml 和 FreeSortListView.qml 中的图标按下事件处理 逻辑。 2. 通过检查 mouse.source 来识别由触摸输入合成的鼠标事件。 3. 当事件来源于触摸而不是真实鼠标时,将事件设置为不接受并提前返回,让父 级 Flickable 接管上下滑动处理。 4. 对真实鼠标输入,保留原有的左键截图抓取及后续交互逻辑不变。 5. 之所以需要此修改,是因为启动器图标此前会消费触摸生成的按下事件,导致 用户在图标区域上下滑动时手势被拦截,应用列表无法顺畅滚动。 Log: 已支持在启动器应用图标区域进行上下触控滑动 Influence: 1. 验证在启动器图标区域直接上下滑动时,应用列表能够平滑滚动。 2. 验证鼠标左键点击图标的行为保持不变,包括相关的图像抓取交互。 3. 在触屏设备上测试长按行为,确认没有引入异常回归。 4. 分别验证 AppListView 和 FreeSortListView 中的滚动表现。 5. 使用真实鼠标输入和触摸输入分别测试,确认事件分发符合预期。 6. 检查图标点击、拖拽、自由排序等相关交互在事件接受逻辑调整后仍然正常。 PMS: BUG-362161 Change-Id: I54e01447f677a2275ab285c576dabbf84799f5f5
deepin pr auto review你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff 代码。这次修改主要针对 QML 中的 整体来看,这次重构的方向非常好,通过将触屏的点击逻辑移至 以下是针对语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见和改进建议: 1. 语法与逻辑问题:
问题:
2. 代码质量优点:消除了状态变量,降低了复杂度
建议:补充代码注释 // MouseArea 负责处理原生鼠标输入(拖拽、右键菜单),
// TapHandler 负责处理触屏输入(点击启动、长按菜单),
// 通过在 MouseArea onPressed 中拒绝触屏合成事件,避免冲突并将滚动交还给 Flickable。3. 代码性能风险:
4. 代码安全风险:QML 上下文属性的隐式依赖 这种强依赖上下文环境的写法在 QML 中虽然常见,但属于“弱类型安全”,如果重构了后端注入逻辑,QML 运行时才会报错(
综合修改建议示例 (以 AppListView.qml 为例) // MouseArea 负责处理原生鼠标输入(拖拽、右键菜单),
// TapHandler 负责处理触屏输入(点击启动、长按菜单),
// 通过在 MouseArea onPressed 中拒绝触屏合成事件,避免冲突并将滚动交还给 Flickable。
MouseArea {
// ... 其他属性
TapHandler {
acceptedDevices: PointerDevice.TouchScreen
gesturePolicy: TapHandler.DragThreshold
// 修复:移除不存在的 buttons 参数
onTapped: function(eventPoint) {
launchApp(desktopId)
}
onLongPressed: {
showContextMenu(itemDelegate, model)
}
}
onPressed: function (mouse) {
// 修复:显式判断触屏合成事件,避免误杀原生鼠标事件
if (mouse.source === Qt.MouseEventSynthesizedByQt || mouse.source === Qt.MouseEventSynthesizedBySystem) {
mouse.accepted = false
return
}
if (mouse.button === Qt.LeftButton) {
itemDelegate.contentItem.grabToImage(function(result) {
itemDelegate.Drag.imageSource = result.url
})
}
}
onClicked: function (mouse) {
if (mouse.button === Qt.RightButton) {
showContextMenu(itemDelegate, model)
baseLayer.focus = true
}
}总体而言,你的重构思路非常正确,大大提升了代码的健壮性和可读性。只需修复上述几个小细节,代码将会更加规范和安全。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, fly602 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 |
Log: Enabled vertical touch scrolling on launcher app icons
Influence:
fix: 启动器支持图标区域触控滑动
Log: 已支持在启动器应用图标区域进行上下触控滑动
Influence:
PMS: BUG-362161
Change-Id: I54e01447f677a2275ab285c576dabbf84799f5f5
Summary by Sourcery
Bug Fixes: