Skip to content

refactor: replace DragHandler with MouseArea for app dragging#655

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-303989
Oct 10, 2025
Merged

refactor: replace DragHandler with MouseArea for app dragging#655
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-303989

Conversation

@wjyrich

@wjyrich wjyrich commented Oct 10, 2025

Copy link
Copy Markdown
Contributor
  1. Replaced DragHandler with MouseArea for better drag control and simpler implementation
  2. Changed drag state detection from dragHandler.active to drag.active
  3. Simplified drag initialization by directly setting Drag.active from mouseArea
  4. Removed complex state management and replaced with direct property binding
  5. Combined left/right click handling in single MouseArea for cleaner code
  6. Improved drag image handling by showing/hiding background during drag operation
  7. Fixed potential crash issues by eliminating the need for external dndItem

refactor: 使用 MouseArea 替换 DragHandler 处理应用拖拽

  1. 使用 MouseArea 替换 DragHandler 以获得更好的拖拽控制和更简单的实现
  2. 将拖拽状态检测从 dragHandler.active 改为 drag.active
  3. 通过直接从 mouseArea 设置 Drag.active 简化了拖拽初始化
  4. 移除了复杂的状态管理,改用直接属性绑定
  5. 在单个 MouseArea 中合并左右键点击处理,代码更清晰
  6. 改进了拖拽图像处理,在拖拽操作期间显示/隐藏背景
  7. 通过消除对外部 dndItem 的需求修复了潜在的崩溃问题

PMS: BUG-303989

Summary by Sourcery

Refactor app dragging implementation to use MouseArea instead of DragHandler, simplifying drag control, unifying click handling, improving drag visuals, and eliminating crash potential

Bug Fixes:

  • Fix potential crash by removing dependency on external dndItem during drag

Enhancements:

  • Replace DragHandler with MouseArea for application dragging and bind Drag.active directly to mouseArea.drag.active
  • Simplify drag initialization and state management with direct property binding
  • Combine left and right click handling into a single MouseArea for cleaner code
  • Improve drag image handling by toggling background visibility during drag operations

@sourcery-ai

sourcery-ai Bot commented Oct 10, 2025

Copy link
Copy Markdown

Reviewer's Guide

This refactor replaces the custom DragHandler and multiple TapHandlers with a unified MouseArea to handle drag activation, click events, and state binding, simplifying drag logic and eliminating complex state management.

Sequence diagram for unified drag and click handling with MouseArea

sequenceDiagram
    participant User as actor
    participant MouseArea
    participant ItemDelegate
    participant Drag
    participant ItemBackground
    User->>MouseArea: Press (LeftButton)
    MouseArea->>ItemDelegate: grabToImage()
    ItemDelegate->>Drag: Set imageSource
    MouseArea->>ItemBackground: Hide background
    User->>MouseArea: Click (RightButton)
    MouseArea->>ItemDelegate: showContextMenu()
    MouseArea->>ItemBackground: Show background
    User->>MouseArea: Click (LeftButton)
    MouseArea->>ItemDelegate: launchApp(desktopId)
Loading

Class diagram for refactored drag and click handling in AppListView.qml

classDiagram
    class ItemDelegate {
        Drag : DragAttached
        MouseArea : MouseArea
        ItemBackground : ItemBackground
    }
    class DragAttached {
        +active : bool
        +dragType : enum
        +mimeData : object
        +hotSpot : point
        +imageSource : url
    }
    class MouseArea {
        +acceptedButtons : enum
        +drag : object
        +onPressed(mouse)
        +onClicked(mouse)
        +enabled : bool
    }
    class ItemBackground {
        +focusPolicy : enum
        +implicitWidth : int
        +implicitHeight : int
        +button : ItemDelegate
    }
    ItemDelegate --> DragAttached : Drag
    ItemDelegate --> MouseArea : MouseArea
    ItemDelegate --> ItemBackground : background
    MouseArea --> ItemDelegate : drag.target
    ItemBackground --> ItemDelegate : button
Loading

File-Level Changes

Change Details Files
Replaced DragHandler and state machine with MouseArea for drag and click handling
  • Removed DragHandler and TapHandler elements
  • Added a full-coverage MouseArea accepting left and right buttons
  • Bound Drag.active to mouseArea.drag.active
qml/windowed/AppListView.qml
Unified left/right click actions in MouseArea
  • Handled right-click to show context menu and focus base layer
  • Handled left-click to launch app directly
qml/windowed/AppListView.qml
Simplified drag initialization and image handling
  • On mouse press, hid background and grabbed delegate image
  • Set Drag.imageSource and deferred background visibility via binding
qml/windowed/AppListView.qml
Removed external dndItem usage and complex state logic
  • Eliminated dependency on external dndItem for drag operations
  • Removed custom State block resetting positions after drag
qml/windowed/AppListView.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `qml/windowed/AppListView.qml:238-239` </location>
<code_context>
+                        if (mouse.button === Qt.RightButton) {
+                            showContextMenu(itemDelegate, model)
+                            baseLayer.focus = true
+                        } else {
+                            launchApp(desktopId)
+                        }
                     }
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Launching app on non-right click may trigger unintended launches.

Restrict the launchApp call to left mouse button clicks to prevent unintended launches from other buttons.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +238 to +239
} else {
launchApp(desktopId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): Launching app on non-right click may trigger unintended launches.

Restrict the launchApp call to left mouse button clicks to prevent unintended launches from other buttons.

1. Replaced DragHandler with MouseArea for better drag control and
simpler implementation
2. Changed drag state detection from dragHandler.active to drag.active
3. Simplified drag initialization by directly setting Drag.active from
mouseArea
4. Removed complex state management and replaced with direct property
binding
5. Combined left/right click handling in single MouseArea for cleaner
code
6. Improved drag image handling by showing/hiding background during
drag operation
7. Fixed potential crash issues by eliminating the need for external
dndItem

refactor: 使用 MouseArea 替换 DragHandler 处理应用拖拽

1. 使用 MouseArea 替换 DragHandler 以获得更好的拖拽控制和更简单的实现
2. 将拖拽状态检测从 dragHandler.active 改为 drag.active
3. 通过直接从 mouseArea 设置 Drag.active 简化了拖拽初始化
4. 移除了复杂的状态管理,改用直接属性绑定
5. 在单个 MouseArea 中合并左右键点击处理,代码更清晰
6. 改进了拖拽图像处理,在拖拽操作期间显示/隐藏背景
7. 通过消除对外部 dndItem 的需求修复了潜在的崩溃问题
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

代码审查报告

Helper.qml

  1. 语法问题

    • Qt.rgba(0, 0, 0, 0.1)Qt.rgba(1, 1, 1, 0.1) 直接替换为字符串 "transparent" 是不正确的。在QML中,颜色值应该是Color类型或使用Qt.rgba()函数创建的颜色对象。
  2. 改进建议

    • 应该使用正确的颜色语法:
    normal {
        common: Qt.rgba(0, 0, 0, 0.1)
        crystal: Qt.rgba(0, 0, 0, 0.1)
    }
    normalDark {
        common: Qt.rgba(1, 1, 1, 0.1)
        crystal: Qt.rgba(1, 1, 1, 0.1)
    }

AppListView.qml

  1. 代码改进

    • visible: !dragHandler.active 改为 visible: !Drag.active 是正确的简化,减少了组件依赖。
    • 移除 DragHandler 相关的状态机是合理的,简化了代码结构。
  2. 性能优化

    • 使用 MouseArea 替代多个事件处理器(TapHandler, DragHandler)可以提高性能,减少了组件数量。
    • 将背景组件的条件渲染移除,改为始终存在,可以减少动态加载/卸载的开销。
  3. 代码结构改进

    • 将事件处理逻辑整合到 MouseArea 中,使代码更加清晰和易于维护。
    • 将拖拽相关的逻辑集中处理,提高了代码的可读性。
  4. 潜在问题

    • 在 onPressed 中直接调用 grabToImage 可能会在频繁操作时造成性能问题,建议添加防抖机制。

FreeSortListView.qml

  1. 性能优化

    • 移除了不必要的背景可见性绑定,简化了渲染逻辑。
    • 直接使用 contentItem 进行图像抓取,减少了中间步骤。
  2. 代码改进

    • 将图像抓取逻辑简化,减少了不必要的代码。

ItemBackground.qml

  1. 功能回归

    • 直接将 color1 和 color2 设置为 background 参数,移除了原有的样式选择逻辑,可能会导致样式不一致的问题。
  2. 改进建议

    • 应该保留原有的样式选择逻辑:
    color1: selectValue(background, DS.Style.checkedButton.background, DS.Style.highlightedButton.background1)
    color2: selectValue(background, DS.Style.checkedButton.background, DS.Style.highlightedButton.background2)

总体建议

  1. 代码风格

    • 保持一致的代码风格和命名规范。
    • 添加必要的注释,特别是关于复杂逻辑的部分。
  2. 性能考虑

    • 对于频繁触发的事件(如拖拽),考虑添加性能优化措施。
    • 避免在渲染过程中进行复杂的计算或操作。
  3. 功能完整性

    • 在修改代码时,确保不会引入功能回归。
    • 对于样式相关的修改,需要进行充分的测试。
  4. 错误处理

    • 添加适当的错误处理机制,特别是在处理用户输入和文件操作时。
  5. 测试覆盖

    • 确保所有修改都经过充分的测试,包括边界条件和异常情况。

@deepin-ci-robot

Copy link
Copy Markdown

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wjyrich

wjyrich commented Oct 10, 2025

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Oct 10, 2025

Copy link
Copy Markdown

This pr force merged! (status: behind)

@deepin-bot deepin-bot Bot merged commit 944967e into linuxdeepin:master Oct 10, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants