Skip to content

fix: Fix the issue where no space is reserved after dragging out the …#1486

Merged
BLumia merged 1 commit intolinuxdeepin:masterfrom
pengfeixx:fix-288727
Mar 7, 2026
Merged

fix: Fix the issue where no space is reserved after dragging out the …#1486
BLumia merged 1 commit intolinuxdeepin:masterfrom
pengfeixx:fix-288727

Conversation

@pengfeixx
Copy link
Contributor

@pengfeixx pengfeixx commented Mar 7, 2026

…tray icon

Fix the issue where no space is reserved after dragging out the tray icon

Log: Fix the issue where no space is reserved after dragging out the tray icon
pms: BUG-288727

Summary by Sourcery

Improve tray icon drag-and-drop behavior by introducing staged drop support and ensuring layout space is correctly reserved during reordering.

New Features:

  • Add staged drop state to the tray sort order model to preview tray icon positions before committing a drop.
  • Expose staged drop QML-invokable methods for staging, committing, and clearing drag positions in the tray.

Bug Fixes:

  • Ensure space is correctly reserved and item sizes reset when dragging tray icons out of or between areas so placeholders no longer inherit the previous item size.
  • Prevent drag-and-drop from being accepted on disallowed tray sections and handle stash vs non-stash drops more robustly.

Enhancements:

  • Refine visual index updates in the tray model to account for staged drop positions via a helper that reserves space in the layout.
  • Extend the tray item position manager with an API to clear registered item sizes and notify listeners when sizes change.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 7, 2026

Reviewer's Guide

Implements a staged drop mechanism and size reset for the tray to ensure space is correctly reserved and previewed when dragging items from the stash to the dock tray, adjusting both model logic and QML drag handling.

Sequence diagram for staged tray drop from stash to dock tray

sequenceDiagram
    actor User
    participant TrayContainerQML
    participant TraySortOrderModel
    participant TrayItemPositionManager

    User->>TrayContainerQML: drag item from stash over tray
    TrayContainerQML->>TrayItemPositionManager: itemIndexByPoint(point)
    TrayItemPositionManager-->>TrayContainerQML: index,isBefore

    TrayContainerQML->>TraySortOrderModel: stageDropPosition(surfaceId, visualIndex)
    TraySortOrderModel->>TraySortOrderModel: set m_stagedSurfaceId,m_stagedVisualIndex
    TraySortOrderModel-->>TrayContainerQML: stagedDropChanged

    TraySortOrderModel->>TraySortOrderModel: updateVisualIndexes()
    TraySortOrderModel->>TrayItemPositionManager: clearRegisteredSizes()
    TrayItemPositionManager->>TrayItemPositionManager: m_registeredItemsSize.clear()
    TrayItemPositionManager-->>TraySortOrderModel: visualItemSizeChanged

    loop assign visual indexes
        TraySortOrderModel->>TraySortOrderModel: reserveStagedDropSpace(currentVisualIndex)
        TraySortOrderModel->>TraySortOrderModel: skip index when currentVisualIndex == m_stagedVisualIndex
    end

    User->>TrayContainerQML: drop item in tray
    TrayContainerQML->>TrayItemPositionManager: itemIndexByPoint(point)
    TrayItemPositionManager-->>TrayContainerQML: index,isBefore
    TrayContainerQML->>TraySortOrderModel: commitStagedDrop()

    TraySortOrderModel->>TraySortOrderModel: dropToDockTray(m_stagedSurfaceId,m_stagedVisualIndex,true)
    TraySortOrderModel->>TraySortOrderModel: clear m_stagedSurfaceId,m_stagedVisualIndex
    TraySortOrderModel-->>TrayContainerQML: stagedDropChanged

    TraySortOrderModel->>TraySortOrderModel: updateVisualIndexes()
Loading

Class diagram for updated tray sort and position management

classDiagram
    class TraySortOrderModel {
        // properties
        int m_visualItemCount
        bool m_collapsed
        bool m_actionsAlwaysVisible
        bool m_isUpdating
        QStringList m_sectionTray
        QStringList m_sectionStash
        QStringList m_sectionFixed
        QStringList m_disabledIds
        QStringList m_hiddenIds
        QStringList m_dockHiddenIds
        QString m_stagedSurfaceId
        int m_stagedVisualIndex

        // Q_PROPERTIES
        bool actionsAlwaysVisible
        bool isUpdating
        QList~QVariantMap~ availableSurfaces
        QString stagedSurfaceId
        int stagedVisualIndex

        // invokable methods
        void dropToDockTray(QString surfaceId, int visualIndex, bool isBefore)
        void setDockVisible(QString surfaceId, bool visible)
        bool isDockVisible(QString surfaceId) const
        QModelIndex getModelIndexByVisualIndex(int visualIndex) const
        void stageDropPosition(QString surfaceId, int visualIndex)
        void commitStagedDrop()
        void clearStagedDrop()

        // internal helpers
        void updateVisualIndexes()
        void reserveStagedDropSpace(int& currentVisualIndex)
        QStandardItem* findItemByVisualIndex(int visualIndex, VisualSections visualSection) const
        QStringList* getSection(QString sectionType)
        QString findSection(QString surfaceId, QString fallback, QStringList forbiddenSections, int pluginFlags)
        void registerToSection(QString surfaceId, QString sectionType)
        QStandardItem* createTrayItem(QString name, QString surfaceId, QString sectionType)

        // signals
        void collapsedChanged(bool)
        void actionsAlwaysVisibleChanged(bool)
        void isUpdatingChanged(bool)
        void visualItemCountChanged(int)
        void availableSurfacesChanged(QList~QVariantMap~)
        void stagedDropChanged()
    }

    class TrayItemPositionManager {
        // properties
        QHash~QString,QSize~ m_registeredItemsSize
        Qt::Orientation m_orientation
        int m_dockHeight

        // invokable methods
        int itemIndexByPoint(QPoint point)
        Qt::Orientation orientation() const
        int dockHeight() const
        void layoutHealthCheck(int delayMs = 200)
        void clearRegisteredSizes()

        // signals
        void orientationChanged(Qt::Orientation)
        void dockHeightChanged(int)
        void visualItemSizeChanged()
    }

    TraySortOrderModel --> TrayItemPositionManager : uses
    TraySortOrderModel ..> QStandardItemModel : inherits
    TrayItemPositionManager ..> QObject : inherits
Loading

File-Level Changes

Change Details Files
Add staged drop state and logic in TraySortOrderModel to reserve and preview space during drag, including integrating it into visual index calculation and drop handling.
  • Include TrayItemPositionManager to allow clearing registered item sizes before recomputing visual indexes.
  • Clear registered tray item sizes at the start of updateVisualIndexes to avoid reusing outdated item sizes for placeholders.
  • Introduce staged drop properties (surface ID and visual index) and expose them as Q_PROPERTY values with a stagedDropChanged signal.
  • Add Q_INVOKABLE methods to stage, commit, and clear a staged drop, each updating visual indexes to reflect a preview or clear it.
  • Add a reserveStagedDropSpace helper and call it at key points in updateVisualIndexes to skip the staged index and thus reserve a slot for the dragged item.
  • Ensure commitStagedDrop reuses existing dropToDockTray logic with isBefore=true for consistency.
panels/dock/tray/traysortordermodel.cpp
panels/dock/tray/traysortordermodel.h
Update TrayContainer.qml drag-and-drop behavior to use the staged drop preview for items dragged from the stash while keeping immediate drop for in-tray drags, and to clear staged state appropriately.
  • Normalize spacing in model data access for SectionTypeRole.
  • Replace the previous stash-specific guard with logic that always evaluates whether a drop is allowed based on showStashActionVisible and dropHoverIndex.
  • For stash-origin drags, call stageDropPosition on drag move to preview and reserve space at the prospective drop index instead of performing the drop immediately.
  • For non-stash drags, keep timer-based delayed drop behavior using dropToDockTray when conditions allow.
  • On drop, decide between commitStagedDrop for stash drags and direct dropToDockTray for others, logging the stash state for debugging.
  • On drag exit, clear any staged drop state via clearStagedDrop and stop the timer for undropped drags from quickPanel.
panels/dock/tray/package/TrayContainer.qml
Extend TrayItemPositionManager with an API to clear registered item sizes so layout can recalculate correctly when visual indexes change.
  • Add a clearRegisteredSizes method that clears the internal item size cache and emits visualItemSizeChanged.
  • Expose clearRegisteredSizes as a Q_INVOKABLE method for use from C++/QML.
  • Use clearRegisteredSizes in TraySortOrderModel before recomputing visual indexes to ensure placeholders use default sizing.
panels/dock/tray/trayitempositionmanager.cpp
panels/dock/tray/trayitempositionmanager.h

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

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.

Hey - I've found 2 issues, and left some high level feedback:

  • Calling TrayItemPositionManager::clearRegisteredSizes() at the start of every updateVisualIndexes() (which is now also invoked on every staged drop change) may cause unnecessary relayouts during drag-hover; consider restricting this to cases where the layout actually changes (e.g. only when items are added/removed) or debouncing it for drag previews.
  • The staged-drop flow currently assumes isBefore is always true in commitStagedDrop; if users can hover between items with a before/after distinction, consider threading the isBefore semantics through the staging APIs to keep behavior consistent with direct drops.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Calling `TrayItemPositionManager::clearRegisteredSizes()` at the start of every `updateVisualIndexes()` (which is now also invoked on every staged drop change) may cause unnecessary relayouts during drag-hover; consider restricting this to cases where the layout actually changes (e.g. only when items are added/removed) or debouncing it for drag previews.
- The staged-drop flow currently assumes `isBefore` is always `true` in `commitStagedDrop`; if users can hover between items with a before/after distinction, consider threading the `isBefore` semantics through the staging APIs to keep behavior consistent with direct drops.

## Individual Comments

### Comment 1
<location path="panels/dock/tray/traysortordermodel.cpp" line_range="650-658" />
<code_context>
+    emit stagedDropChanged();
+}
+
+void TraySortOrderModel::clearStagedDrop()
+{
+    m_stagedSurfaceId.clear();
+    m_stagedVisualIndex = -1;
+    emit stagedDropChanged();
+    
+    // Update visual indexes to remove preview
+    updateVisualIndexes();
+}
+
</code_context>
<issue_to_address>
**suggestion (performance):** Guard clearStagedDrop to avoid redundant stagedDropChanged + updateVisualIndexes when nothing is staged.

Since this can be called when no staged drop is active (e.g. from onExited in QML), we end up emitting stagedDropChanged and running updateVisualIndexes unnecessarily. Consider an early return when m_stagedSurfaceId is empty and m_stagedVisualIndex < 0 to skip redundant work.

```suggestion
void TraySortOrderModel::clearStagedDrop()
{
    // Avoid redundant work if there is no staged drop
    if (m_stagedSurfaceId.isEmpty() && m_stagedVisualIndex < 0) {
        return;
    }

    m_stagedSurfaceId.clear();
    m_stagedVisualIndex = -1;
    emit stagedDropChanged();

    // Update visual indexes to remove preview
    updateVisualIndexes();
}
```
</issue_to_address>

### Comment 2
<location path="panels/dock/tray/trayitempositionmanager.cpp" line_range="122-126" />
<code_context>
     qDebug() << "layout health check scheduled!";
 }

+void TrayItemPositionManager::clearRegisteredSizes()
+{
+    m_registeredItemsSize.clear();
+    emit visualItemSizeChanged();
+}
+
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid emitting visualItemSizeChanged if registered sizes are already empty.

clearRegisteredSizes always clears m_registeredItemsSize and emits visualItemSizeChanged, even when the map is already empty. Since updateVisualIndexes now calls this unconditionally, that can create many redundant signals. Add a guard (e.g. if (!m_registeredItemsSize.isEmpty())) before clearing and emitting to avoid unnecessary downstream recalculations.

```suggestion
void TrayItemPositionManager::clearRegisteredSizes()
{
    if (m_registeredItemsSize.isEmpty()) {
        return;
    }

    m_registeredItemsSize.clear();
    emit visualItemSizeChanged();
}
```
</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.

…tray icon

Fix the issue where no space is reserved after dragging out the tray icon

Log: Fix the issue where no space is reserved after dragging out the tray icon
pms: BUG-288727
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

1. 整体评估

这段代码实现了托盘区域(Tray)的拖放功能改进,特别是针对暂存区域(stash area)的拖放操作进行了优化。代码引入了"暂存拖放"(staged drop)机制,允许在拖放过程中预览位置,并在实际放置前预留空间。

2. 语法逻辑分析

优点

  1. 代码结构清晰,功能模块划分合理
  2. 使用了QML与C++结合的方式实现功能,符合Qt框架的最佳实践
  3. 通过信号/槽机制实现了组件间通信

问题与改进建议

TrayContainer.qml

  1. 拖放逻辑问题:

    // 在onPositionChanged中
    if (isStash && shouldAllowDrop) {
        DDT.TraySortOrderModel.stageDropPosition(surfaceId, Math.floor(currentItemIndex))
    } else if (!isStash && shouldAllowDrop) {
        dropTrayTimer.handleDrop = function() {
            if (isDropped || dragExited) return
            DDT.TraySortOrderModel.dropToDockTray(surfaceId, Math.floor(currentItemIndex), isBefore)
        }
        dropTrayTimer.start()
    }

    建议: 考虑将拖放逻辑统一处理,减少代码重复。可以创建一个统一的拖放处理函数,根据isStash标志选择不同的处理方式。

  2. 错误处理缺失:

    let modelIndex = DDT.TraySortOrderModel.getModelIndexByVisualIndex(currentItemIndex)
    let sectionType = root.model.data(modelIndex, DDT.TraySortOrderModel.SectionTypeRole)

    建议: 添加对modelIndex有效性的检查,防止无效索引导致错误。

traysortordermodel.cpp

  1. 性能问题:

    void TraySortOrderModel::stageDropPosition(const QString &surfaceId, int visualIndex)
    {
        // ...
        updateVisualIndexes(); // 每次拖动都会更新整个视图
    }

    建议: 考虑优化updateVisualIndexes()的调用频率,可以使用防抖(debounce)机制减少不必要的更新。

  2. 状态管理问题:

    void TraySortOrderModel::commitStagedDrop()
    {
        if (m_stagedSurfaceId.isEmpty() || m_stagedVisualIndex < 0) {
            return;
        }
        
        // Reuse dropToDockTray logic for consistency
        dropToDockTray(m_stagedSurfaceId, m_stagedVisualIndex, true);
        
        // Clear staged state (dropToDockTray already called updateVisualIndexes)
        m_stagedSurfaceId.clear();
        m_stagedVisualIndex = -1;
        emit stagedDropChanged();
    }

    建议: 在dropToDockTray失败时,应该有错误处理机制,避免状态不一致。

3. 代码质量分析

优点

  1. 代码注释清晰,解释了关键逻辑
  2. 命名规范,变量和函数名具有描述性
  3. 使用了常量定义,避免魔法数字

问题与改进建议

  1. 代码重复:

    // 在多个地方重复出现
    reserveStagedDropSpace(currentVisualIndex);
    results[0]->setData(currentVisualIndex, TraySortOrderModel::VisualIndexRole);
    currentVisualIndex++;

    建议: 可以提取为公共函数,减少重复代码。

  2. 资源管理:

    void TrayItemPositionManager::clearRegisteredSizes()
    {
        if (m_registeredItemsSize.isEmpty()) {
            return;
        }
        
        m_registeredItemsSize.clear();
        emit visualItemSizeChanged();
    }

    建议: 考虑添加RAII风格的资源管理,确保资源正确释放。

4. 代码性能分析

问题与改进建议

  1. 频繁更新视图:

    void TraySortOrderModel::stageDropPosition(const QString &surfaceId, int visualIndex)
    {
        // ...
        updateVisualIndexes(); // 每次拖动都会更新整个视图
    }

    建议: 实现增量更新机制,只更新受影响的部分,而不是整个视图。

  2. 内存分配:

    void TraySortOrderModel::updateVisualIndexes()
    {
        // ...
        for (int i = 0; i < rowCount(); i++) {
            item(i)->setData(-1, TraySortOrderModel::VisualIndexRole);
        }
        // ...
    }

    建议: 考虑预分配内存或使用更高效的数据结构。

5. 代码安全分析

问题与改进建议

  1. 输入验证不足:

    void TraySortOrderModel::stageDropPosition(const QString &surfaceId, int visualIndex)
    {
        // 没有对surfaceId和visualIndex进行充分验证
        m_stagedSurfaceId = surfaceId;
        m_stagedVisualIndex = visualIndex;
    }

    建议: 添加对输入参数的验证,特别是surfaceId的格式和visualIndex的范围检查。

  2. 状态一致性:

    void TraySortOrderModel::commitStagedDrop()
    {
        // ...
        dropToDockTray(m_stagedSurfaceId, m_stagedVisualIndex, true);
        
        // 如果dropToDockTray失败,状态可能不一致
        m_stagedSurfaceId.clear();
        m_stagedVisualIndex = -1;
        emit stagedDropChanged();
    }

    建议: 添加状态检查和恢复机制,确保在操作失败时状态保持一致。

  3. 并发安全:
    建议: 考虑添加适当的同步机制,防止多线程访问导致的数据竞争。

6. 其他建议

  1. 单元测试:
    建议: 为新增的拖放功能添加单元测试,特别是边界情况和错误处理。

  2. 文档:
    建议: 更新API文档,特别是新增的stageDropPosition、commitStagedDrop和clearStagedDrop方法。

  3. 日志:
    建议: 添加更详细的日志记录,便于调试和问题追踪。

总结

这段代码实现了托盘区域拖放功能的改进,整体设计合理,但在错误处理、性能优化和状态管理方面还有改进空间。建议在后续版本中逐步完善这些方面,以提高代码的健壮性和性能。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, pengfeixx

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

@BLumia BLumia merged commit d12b2dd into linuxdeepin:master Mar 7, 2026
10 of 12 checks passed
@pengfeixx pengfeixx deleted the fix-288727 branch March 7, 2026 07:49
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