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

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

这段代码主要实现了任务栏中窗口项的拖拽移动、分组重排以及设置变更时的响应功能。整体逻辑是清晰的,但存在一些关于性能、模型操作规范性和代码健壮性的改进空间。以下是详细的审查意见:

1. 代码逻辑与功能

改进点 1:DockGlobalElementModel::groupItemsByApp 的性能优化

  • 问题:当前的 groupItemsByApp 实现使用了嵌套循环,并且在内层循环中直接调用了 beginMoveRowsendMoveRows
    • 对于一个包含 $N$ 个元素的列表,如果所有元素都需要移动,这会触发大量的模型更新信号(layoutChangedrowsMoved)。
    • 频繁触发模型更新会导致视图(QML界面)进行大量的重绘和布局计算,造成明显的界面卡顿。
  • 建议
    • 方案 A(推荐):先在内存中对 m_data 进行排序或重组,计算出最终的正确顺序,然后一次性调用 beginResetModel()endResetModel()(如果顺序变化很大),或者使用更智能的算法计算最小移动路径并批量发出 rowsMoved 信号。由于分组逻辑是将相同 ID 的元素聚在一起,可以先遍历提取所有 ID,去重后按顺序排列,再重建 m_data
    • 方案 B:如果必须保持逐个移动以利用动画效果,应尽量减少不必要的 beginMoveRows 调用。目前的逻辑中,insertPos 会随移动递增,逻辑是正确的,但可以考虑使用 QPersistentModelIndex 来跟踪位置,防止在多次移动过程中索引失效(虽然这里用的是 m_data 列表索引,相对安全)。

改进点 2:DockGlobalElementModel::moveItemgroupItemsByApp 的冲突

  • 问题moveItem 允许用户任意拖拽改变顺序,而 groupItemsByApp 会强制按应用 ID 分组。如果用户拖拽后紧接着触发分组(例如设置变更),用户的操作会被覆盖。
  • 建议:确保 groupItemsByApp 仅在特定时机(如设置切换、初始化)调用。代码中已连接到 windowSplitChanged 信号,这看起来是合理的,但需确认是否还有其他地方会意外触发。

2. 代码质量与规范

改进点 3: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();
    });
    这里只是简单地透传了 beginMoveRowsendMoveRows。如果源模型是 DockGlobalElementModel,而 DockItemModel 又是它的代理,这种处理方式在多层模型嵌套时可能会导致索引计算错误或重复触发。
  • 建议:检查 DockItemModelDockGlobalElementModel 的关系。如果 DockGlobalElementModel 已经发出了 rowsMoved 信号,且 DockItemModel 是直接映射,通常不需要手动再次调用 beginMoveRows/endMoveRows,除非 DockItemModel 对行进行了过滤或映射转换。如果是透传,确保 sourceModel 的信号参数直接对应 this 的模型索引。

改进点 4:DockGlobalElementModel::DockGlobalElementModel 构造函数中的查找逻辑

  • 问题
    for (auto it = firstIt + 1; it != m_data.end(); ++it) {
        if (std::get<0>(*it) == desktopId) {
            lastIt = it;
        }
    }
    这段代码用于查找最后一个相同 ID 的元素。虽然逻辑正确,但注释提到 "Search the entire list since windows may not be consecutive after drag reorder"。这意味着在插入新窗口时,为了保持分组,可能需要扫描整个列表。
  • 建议:如果 m_data 经常处于非排序状态(因为允许拖拽),这个 $O(N)$ 的扫描是不可避免的。但如果能维护一个辅助数据结构(如 QHash<QString, int> 记录每个 App 最后一个窗口的索引),可以将此操作优化到接近 $O(1)$。不过考虑到代码复杂度,目前的写法在列表长度不大时是可以接受的。

3. 代码安全

改进点 5:边界检查与空指针

  • 问题
    • DockGlobalElementModel::moveItemgroupItemsByApp 都有检查 m_data.isEmpty() 或索引范围,这是很好的。
    • TaskManager::moveItem 中检查了 m_dockGlobalElementModel 是否为空。
  • 建议:目前的边界检查做得比较完善。唯一需要注意的是,在 groupItemsByApp 中,m_data.move(j, insertPos) 修改了容器,紧接着 ++insertPos。这依赖于 QList::move 的行为(移动到目标位置,原位置元素移除)。逻辑上是正确的,但建议添加注释说明这一点,因为这种原地修改容器的迭代器操作容易让人困惑。

4. QML 交互逻辑

改进点 6:QML 中的重复代码

  • 问题:在 TaskManager.qml 中,onDropped 和类似的拖拽处理函数里出现了重复的逻辑块:
    if(taskmanager.Applet.windowSplit) {
        taskmanager.Applet.moveItem(currentIndex, targetIndex)
    } else {
        visualModel.items.move(currentIndex, targetIndex)
    }
  • 建议:将这段逻辑提取为一个 JS 函数,例如 function handleItemMove(from, to) { ... },以减少代码重复,提高可维护性。

总结建议

  1. 性能关键:重构 groupItemsByApp,避免在循环中频繁触发模型更新信号。建议先计算最终顺序,再一次性更新模型。
  2. 模型同步:审查 DockItemModelrowsMoved 的处理,确保没有不必要的中间层信号转发。
  3. 代码整洁:提取 QML 中的重复逻辑。
  4. 注释:在 groupItemsByApp 中对复杂的容器修改逻辑添加详细注释。

优化后的 groupItemsByApp 示例思路(伪代码):

void DockGlobalElementModel::groupItemsByApp()
{
    if (m_data.isEmpty() || TaskManagerSettings::instance()->isWindowSplit())
        return;

    // 1. 创建一个临时列表用于存储分组后的数据
    QList<ItemType> newData;
    QSet<QString> processedIds;

    // 2. 遍历原始数据,按 ID 分组
    for (const auto &item : m_data) {
        const QString &id = std::get<0>(item);
        if (processedIds.contains(id)) continue;

        // 找到所有相同 ID 的项
        for (const auto &innerItem : m_data) {
            if (std::get<0>(innerItem) == id) {
                newData.append(innerItem);
            }
        }
        processedIds.insert(id);
    }

    // 3. 检查顺序是否真的改变了
    if (m_data == newData) return;

    // 4. 更新模型
    beginResetModel();
    m_data = newData;
    endResetModel();
}

注:使用 beginResetModel 会导致视图完全刷新,失去当前的滚动位置或选中状态。如果需要保留状态,则需要计算具体的 beginMoveRows 序列,但这会显著增加代码复杂度。鉴于分组通常是配置变更时的操作,ResetModel 通常是可接受的。

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.

2 participants