Skip to content

fix: improve window grouping and drag reordering in dock#1478

Open
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-343469
Open

fix: improve window grouping and drag reordering in dock#1478
wjyrich wants to merge 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-343469

Conversation

@wjyrich
Copy link
Contributor

@wjyrich wjyrich commented Mar 4, 2026

  1. Fixed window grouping logic to correctly group windows by application after drag reordering
  2. Enhanced window insertion algorithm to find the last window of an app even when windows are not consecutive
  3. Added moveItem() method to properly handle item movement in the model
  4. Connected windowSplitChanged signal to trigger re-grouping when window split mode changes
  5. Updated QML drag handlers to use moveItem() when window split is enabled

Log: Fixed dock task manager window grouping issues after drag reordering

Influence:

  1. Test dragging windows in dock when window split is disabled - windows should stay grouped by application
  2. Test dragging windows in dock when window split is enabled - windows should move independently
  3. Verify that window grouping is maintained after toggling window split mode
  4. Test drag and drop reordering of both individual windows and application groups
  5. Verify that the dock maintains correct ordering after multiple drag operations

fix: 修复任务管理器停靠栏窗口分组和拖拽重排序问题

  1. 修复窗口分组逻辑,确保拖拽重排序后窗口能正确按应用程序分组
  2. 改进窗口插入算法,即使窗口不连续也能找到应用程序的最后一个窗口
  3. 添加 moveItem() 方法以正确处理模型中的项目移动
  4. 连接 windowSplitChanged 信号,在窗口分割模式更改时触发重新分组
  5. 更新 QML 拖拽处理程序,在窗口分割启用时使用 moveItem() 方法

Log: 修复停靠栏任务管理器拖拽重排序后的窗口分组问题

Influence:

  1. 测试在窗口分割禁用时拖拽停靠栏中的窗口 - 窗口应按应用程序保持分组
  2. 测试在窗口分割启用时拖拽停靠栏中的窗口 - 窗口应独立移动
  3. 验证切换窗口分割模式后窗口分组是否保持正确
  4. 测试单个窗口和应用程序组的拖拽重排序功能
  5. 验证多次拖拽操作后停靠栏是否保持正确的排序

PMS: BUG-343469

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Sorry @wjyrich, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要实现了任务栏(Dock)中应用窗口的拖拽排序和分组功能。以下是对代码的详细审查和改进建议:

1. 语法与逻辑

优点:

  • 拖拽逻辑清晰moveItem 函数正确处理了 beginMoveRowsendMoveRows,这是修改 QAbstractItemModel 派生类数据时的标准做法,能确保视图正确更新。
  • 分组逻辑基本正确groupItemsByApp 函数通过遍历列表,将分散的相同应用窗口移动到一起,逻辑上能够实现目标。

潜在问题与改进建议:

  • groupItemsByApp 中的索引管理风险
    groupItemsByApp 函数中,使用了嵌套循环和 beginMoveRows/endMoveRows。虽然代码在最后更新了 i = insertPos - 1 来跳过已处理部分,但在内层循环中调用 m_data.move(j, insertPos) 会改变列表结构。虽然 QList(假设 m_dataQList)的 move 操作通常是 O(1) 的,但频繁的模型更新(信号发射)可能导致性能问题或界面闪烁。

    • 建议:考虑在内存中构建一个新的有序列表,然后使用 resetModel 或更高效的批量更新机制(如果可能),而不是在循环中逐个移动。如果必须逐个移动,确保在操作前禁用视图更新(虽然 Qt 模型通常不建议这样做,除非有特殊优化)。
  • DockItemModel 中的 rowsMoved 连接

    connect(sourceModel(), &QAbstractItemModel::rowsMoved, this, [this](const QModelIndex &parent, int start, int end, const QModelIndex &destination, int row) {
        if (parent.isValid() || m_isUpdating)
            return;
        beginMoveRows(QModelIndex(), start, end, QModelIndex(), row);
        endMoveRows();
    });

    这段代码将源模型的 rowsMoved 信号转发。这里需要确保 sourceModel 的索引逻辑与当前模型一致。如果 sourceModelDockGlobalElementModel,而当前是 DockItemModel,通常这种代理转发是正确的。但需注意 beginMoveRowsdestination 行参数逻辑:当 rowstart 之前时,Qt 的行为与之后不同,这里的直接转发假设了两者索引空间完全一致且 row 参数直接可用。

2. 代码质量

优点:

  • 版权信息更新:更新了 SPDX 版权年份范围(如 2024-2026),符合规范。
  • 注释更新:修改了关于窗口插入位置的注释,解释了为什么需要搜索整个列表(因为拖拽重排导致窗口可能不连续),提高了代码可读性。

改进建议:

  • 代码复用:在 DockGlobalElementModel::groupItemsByApp 中,查找相同 ID 的逻辑与构造函数中的部分逻辑相似。

    // 构造函数中的逻辑
    auto lastIt = firstIt;
    for (auto it = firstIt + 1; it != m_data.end(); ++it) {
        if (std::get<0>(*it) == desktopId) {
            lastIt = it;
        }
    }

    考虑提取一个私有辅助函数,例如 findLastIndexOfApp,以减少重复代码并统一查找逻辑。

  • QML 中的重复代码:在 TaskManager.qmlonDroppedonPositionChanged 中,有一段完全相同的逻辑:

    if(taskmanager.Applet.windowSplit) {
        taskmanager.Applet.moveItem(currentIndex, targetIndex)
    } else {
        visualModel.items.move(currentIndex, targetIndex)
    }

    建议:可以将此逻辑提取为一个 JavaScript 函数,例如 moveTaskItem(currentIndex, targetIndex),然后在两个事件处理中调用它,保持 DRY (Don't Repeat Yourself) 原则。

3. 代码性能

  • groupItemsByApp 的复杂度
    该函数使用了双重循环。虽然外层循环会跳过已排序的元素,但在最坏情况下(例如列表完全乱序),复杂度接近 O(N^2)。对于任务栏这种通常只有几十个条目的场景,性能尚可接受。但如果应用数量极多,可能会有轻微卡顿。

    • 建议:如果性能成为瓶颈,可以使用 QMap<QString, QList<Item>> 先按 ID 分组,然后再合并回 m_data,这可以将复杂度降低到 O(N)。
  • 频繁的信号发射
    groupItemsByApp 中,每次 beginMoveRowsendMoveRows 都会导致视图重绘。如果有大量窗口需要重新分组,这会导致显著的 UI 开销。

    • 建议:如果 UI 框架支持,可以尝试批量操作。但在 Qt 标准模型中,很难避免逐个通知。作为优化,可以在函数开始时检查 m_data 是否已经是有序的,如果是则直接返回,避免不必要的计算和信号发射。

4. 代码安全

  • 边界检查
    moveItem 函数中有良好的边界检查:

    if (from < 0 || from >= m_data.size() || to < 0 || to >= m_data.size() || from == to)
        return;

    这防止了越界访问,是很好的实践。

  • 空指针检查
    TaskManager::moveItem 中检查了 m_dockGlobalElementModel 是否为空,这也是安全的。

  • 潜在的逻辑死循环或状态不一致
    groupItemsByApp 中,如果 m_data 在执行过程中被外部修改(例如另一个线程或异步事件触发了修改),可能会导致索引失效或崩溃。

    • 建议:虽然 Qt 的信号槽机制通常在同一线程,但确保在执行此操作时,模型处于“锁定”或“不可变”状态是更安全的做法。或者,确保在调用此函数前,相关的信号连接已被临时阻塞,防止重入。

总结

这段代码整体上是健壮的,正确实现了拖拽排序和分组功能。主要的改进空间在于:

  1. 性能优化groupItemsByApp 算法复杂度和信号发射频率。
  2. 代码结构:减少 QML 和 C++ 中的重复代码。
  3. 注释与可读性:虽然注释有所改进,但 groupItemsByApp 这种复杂的逻辑可以添加更多注释说明其算法步骤。

建议优先考虑重构 groupItemsByApp 以提高性能,并提取 QML 中的重复逻辑以提升可维护性。

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes dock task manager ordering issues by making drag-reordering update the underlying models correctly in window-split mode, and by re-grouping windows by application when split mode is turned off.

Changes:

  • Add a QML-callable moveItem(from,to) API and route it to the underlying dock global model.
  • Improve new-window insertion to locate the rightmost existing window for an app even if windows aren’t consecutive.
  • Add regrouping on windowSplitChanged and propagate rowsMoved so views stay in sync during model moves.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
panels/dock/taskmanager/taskmanager.h Exposes moveItem(from,to) to QML.
panels/dock/taskmanager/taskmanager.cpp Forwards moveItem() to DockGlobalElementModel.
panels/dock/taskmanager/package/TaskManager.qml Uses moveItem() for drag-reorder when window-split is enabled.
panels/dock/taskmanager/dockitemmodel.cpp Mirrors rowsMoved from the source model to keep the proxy/view updated.
panels/dock/taskmanager/dockglobalelementmodel.h Declares moveItem() and regroup helper.
panels/dock/taskmanager/dockglobalelementmodel.cpp Implements item moving + regrouping; improves “insert after last window of app” logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +82 to +86
connect(sourceModel(), &QAbstractItemModel::rowsMoved, this, [this](const QModelIndex &parent, int start, int end, const QModelIndex &destination, int row) {
if (parent.isValid() || m_isUpdating)
return;
beginMoveRows(QModelIndex(), start, end, QModelIndex(), row);
endMoveRows();
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

This rowsMoved handler’s lambda takes a destination parameter that is never used. If the project builds with -Werror=unused-parameter, this will fail compilation. Either remove the unused parameter from the lambda signature or mark it unused (e.g., via Q_UNUSED(destination)).

Copilot uses AI. Check for mistakes.
1. Fixed window grouping logic to correctly group windows by application
after drag reordering
2. Enhanced window insertion algorithm to find the last window of an app
even when windows are not consecutive
3. Added moveItem() method to properly handle item movement in the model
4. Connected windowSplitChanged signal to trigger re-grouping when
window split mode changes
5. Updated QML drag handlers to use moveItem() when window split is
enabled

Log: Fixed dock task manager window grouping issues after drag
reordering

Influence:
1. Test dragging windows in dock when window split is disabled - windows
should stay grouped by application
2. Test dragging windows in dock when window split is enabled - windows
should move independently
3. Verify that window grouping is maintained after toggling window split
mode
4. Test drag and drop reordering of both individual windows and
application groups
5. Verify that the dock maintains correct ordering after multiple drag
operations

fix: 修复任务管理器停靠栏窗口分组和拖拽重排序问题

1. 修复窗口分组逻辑,确保拖拽重排序后窗口能正确按应用程序分组
2. 改进窗口插入算法,即使窗口不连续也能找到应用程序的最后一个窗口
3. 添加 moveItem() 方法以正确处理模型中的项目移动
4. 连接 windowSplitChanged 信号,在窗口分割模式更改时触发重新分组
5. 更新 QML 拖拽处理程序,在窗口分割启用时使用 moveItem() 方法

Log: 修复停靠栏任务管理器拖拽重排序后的窗口分组问题

Influence:
1. 测试在窗口分割禁用时拖拽停靠栏中的窗口 - 窗口应按应用程序保持分组
2. 测试在窗口分割启用时拖拽停靠栏中的窗口 - 窗口应独立移动
3. 验证切换窗口分割模式后窗口分组是否保持正确
4. 测试单个窗口和应用程序组的拖拽重排序功能
5. 验证多次拖拽操作后停靠栏是否保持正确的排序

PMS: BUG-343469
@deepin-bot
Copy link

deepin-bot bot commented Mar 5, 2026

TAG Bot

New tag: 2.0.31
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1481

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