Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Aug 5, 2025

使 ItemModel 使用新的任务栏驻留配置,并确保任务栏驻留顺序的保存位置也使用
新的任务栏驻留配置.

此提交旨在进一步与 ItemModel 解耦,便于后续模型切换重构分支的合入.

Summary by Sourcery

Refactor TaskManager to migrate docked items configuration from the deprecated TASKMANAGER_DOCKEDITEMS_KEY to the new TASKMANAGER_DOCKEDELEMENTS_KEY, unify docking APIs around element ID strings, and update drag-and-drop reordering logic accordingly.

New Features:

  • Introduce saveDockElementsOrder invokable in TaskManager to persist user reordering of docked elements.

Enhancements:

  • Migrate legacy docked items from YAML-based config to a string list under TASKMANAGER_DOCKEDELEMENTS_KEY with automatic data migration and deprecation warning for the old key.
  • Replace TaskManagerSettings’ dockedDesktopFiles API with set/append/remove methods and an isDocked query using element ID strings.
  • Remove ItemModel.moveTo and shift item reordering logic into TaskManager and QML, adjusting drag-and-drop to operate on element IDs.
  • Update DesktopfileAbstractParser to use the new string-based docking API instead of JSON objects.

@sourcery-ai
Copy link

sourcery-ai bot commented Aug 5, 2025

Reviewer's Guide

This PR refactors the docked-items persistence in TaskManagerSettings to use a new TASKMANAGER_DOCKEDELEMENTS_KEY, removes legacy JSON-based APIs, introduces string-based helpers and migration logic, updates Desktopfile parsing, eliminates the old ItemModel moveTo method, and adapts TaskManager and its QML UI to handle ordering and saving via the new API.

Sequence diagram for saving docked elements order from QML drag-and-drop

sequenceDiagram
    participant User as actor User
    participant QML as TaskManager.qml
    participant TaskManager as TaskManager
    participant TaskManagerSettings as TaskManagerSettings

    User->>QML: Drag and drop docked app icon
    QML->>QML: visualModel.items.move(currentIndex, targetIndex)
    QML->>QML: Collect new appIds order
    QML->>TaskManager: saveDockElementsOrder(appIds)
    TaskManager->>TaskManagerSettings: setDockedElements(newDockedElements)
    TaskManagerSettings->>TaskManagerSettings: saveDockedElements()
    TaskManagerSettings-->>TaskManager: (order persisted)
    TaskManager-->>QML: (optional: update UI)
Loading

Class diagram for TaskManagerSettings refactor to TASKMANAGER_DOCKEDELEMENTS_KEY

classDiagram
    class TaskManagerSettings {
        +setDockedElements(elements: QStringList)
        +appendDockedElements(element: QString)
        +removeDockedElements(element: QString)
        +dockedElements() const: QStringList
        +isDocked(elementId: QString) const: bool
        -migrateFromDockedItems()
        -toDockedElementsStrings(items: QJsonArray) const: QStringList
        -toDockedElementsString(item: QJsonObject) const: QString
        -saveDockedElements()
    }

    class DesktopfileAbstractParser {
        +isDocked(): bool
        +setDocked(docked: bool)
    }

    TaskManagerSettings <.. DesktopfileAbstractParser : uses

    class TaskManager {
        +saveDockElementsOrder(appIds: QStringList)
        +loadDockedAppItems()
    }

    TaskManager --> TaskManagerSettings : uses

    class ItemModel {
        +dumpDockedItems() const: QJsonArray
    }
Loading

Class diagram for removed legacy docked items API

classDiagram
    class TaskManagerSettings {
        -setDockedDesktopFiles(desktopfiles: QJsonArray)
        -appnedDockedDesktopfiles(desktopfile: QJsonObject)
        -removeDockedDesktopfile(desktopfile: QJsonObject)
        -dockedDesktopFiles(): QJsonArray
        -dockedItemsPersisted()
        -loadDockedItems()
    }
    %% These methods and members have been removed in favor of the new string-based API.
Loading

File-Level Changes

Change Details Files
Migrate and deprecate old TASKMANAGER_DOCKEDITEMS_KEY handling
  • Removed loadDockedItems calls and related signals for the deprecated key
  • Added a qDebug warning when TASKMANAGER_DOCKEDITEMS_KEY is encountered
  • In constructor: call migrateFromDockedItems and emit dockedElementsChanged
panels/dock/taskmanager/taskmanagersettings.cpp
Overhaul TaskManagerSettings API for docked elements
  • Deleted old dockedItemsPersisted, loadDockedItems and QJsonArray API
  • Added setDockedElements, appendDockedElements, removeDockedElements, isDocked methods
  • Introduced toDockedElementsStrings, toDockedElementsString helpers and saveDockedElements
  • Changed dockedElements() signature to const QStringList&
panels/dock/taskmanager/taskmanagersettings.h
panels/dock/taskmanager/taskmanagersettings.cpp
Adapt DesktopfileAbstractParser to new dockedElements API
  • Replaced QJsonObject construction with isDocked("desktop/") call
  • Swapped appned/removeDockedDesktopfile calls for append/removeDockedElements
panels/dock/taskmanager/desktopfileabstractparser.cpp
Remove legacy moveTo from ItemModel
  • Deleted ItemModel::moveTo implementation and its Q_INVOKABLE declaration
panels/dock/taskmanager/itemmodel.cpp
panels/dock/taskmanager/itemmodel.h
Update TaskManager and QML to use new ordering and saving logic
  • Changed loadDockedAppItems to iterate TaskManagerSettings::dockedElements
  • Added TaskManager::saveDockElementsOrder and exposed it to QML
  • In TaskManager.qml: added findAppIndex, conditional visualModel.items.move, collected appIds after drop, and invoked saveDockElementsOrder
panels/dock/taskmanager/taskmanager.cpp
panels/dock/taskmanager/taskmanager.h
panels/dock/taskmanager/package/TaskManager.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

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 @BLumia - 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> `panels/dock/taskmanager/taskmanagersettings.cpp:162` </location>
<code_context>
+
+void TaskManagerSettings::appendDockedElements(const QString &element)
+{
+    m_dockedElements.append(element);
+    Q_EMIT dockedElementsChanged();
+    saveDockedElements();
 }
</code_context>

<issue_to_address>
Appending elements without duplicate check may cause repeated entries.

This could lead to UI or logic errors if unique docked elements are expected.
</issue_to_address>

### Comment 2
<location> `panels/dock/taskmanager/package/TaskManager.qml:192` </location>
<code_context>
                 let appId = taskmanager.Applet.desktopIdToAppId(launcherDndDesktopId)
-                taskmanager.Applet.dataModel.moveTo(appId, targetIndex)
+                let currentIndex = taskmanager.findAppIndex(appId)
+                if (currentIndex !== -1 && currentIndex !== targetIndex && targetIndex < visualModel.items.count) {
+                    visualModel.items.move(currentIndex, targetIndex)
+                }
             }
</code_context>

<issue_to_address>
No handling for out-of-bounds or negative targetIndex.

Also check that targetIndex is not negative to avoid errors.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
                let currentIndex = taskmanager.findAppIndex(appId)
                if (currentIndex !== -1 && currentIndex !== targetIndex && targetIndex < visualModel.items.count) {
                    visualModel.items.move(currentIndex, targetIndex)
                }
=======
                let currentIndex = taskmanager.findAppIndex(appId)
                if (
                    currentIndex !== -1 &&
                    currentIndex !== targetIndex &&
                    targetIndex >= 0 &&
                    targetIndex < visualModel.items.count
                ) {
                    visualModel.items.move(currentIndex, targetIndex)
                }
>>>>>>> REPLACE

</suggested_fix>

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.

Q_EMIT dockedElementsChanged();
qDebug() << "TASKMANAGER_DOCKEDITEMS_KEY is deprecated, please use TASKMANAGER_DOCKEDELEMENTS_KEY instead";
} else if (TASKMANAGER_DOCKEDELEMENTS_KEY == key) {
m_dockedElements = m_taskManagerDconfig->value(TASKMANAGER_DOCKEDELEMENTS_KEY, {}).toStringList();
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): Appending elements without duplicate check may cause repeated entries.

This could lead to UI or logic errors if unique docked elements are expected.

@BLumia BLumia force-pushed the dockedelemets_key branch from d19a40d to 2e66369 Compare August 5, 2025 07:10
@BLumia BLumia force-pushed the dockedelemets_key branch from 2e66369 to 5a3823e Compare August 5, 2025 07:52
使 ItemModel 使用新的任务栏驻留配置,并确保任务栏驻留顺序的保存位置也使用
新的任务栏驻留配置.

此提交旨在进一步与 ItemModel 解耦,便于后续模型切换重构分支的合入.

Log:
@BLumia BLumia force-pushed the dockedelemets_key branch from 5a3823e to 971ef6f Compare August 5, 2025 08:09
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码注释

    • DesktopfileAbstractParser::isDockedDesktopfileAbstractParser::setDocked函数中,注释应该更详细地解释为什么需要使用QStringLiteral而不是直接使用字符串字面量。
  2. 函数命名

    • DesktopfileAbstractParser::setDocked函数中的appnedDockedDesktopfilesremoveDockedDesktopfile方法名应该更明确地反映其功能,例如appendDockedDesktopFilesremoveDockedDesktopFiles
  3. 代码重构

    • ItemModel::dumpDockedItems函数中,result变量应该初始化为QJsonArray(),以避免潜在的未初始化使用。
  4. 错误处理

    • TaskManager.qml文件中,findAppIndex函数在找不到匹配的appId时返回-1,这是一个好的做法,但是应该添加注释说明为什么返回-1
  5. 日志记录

    • TaskManager::handleWindowAdded函数中,日志记录的字符串应该使用QString格式化,而不是直接拼接字符串。
  6. 代码风格

    • TaskManager::loadDockedAppItems函数中,for循环的缩进应该与foreach循环保持一致。
  7. 性能优化

    • TaskManagerSettings::migrateFromDockedItems函数中,foreach循环可以替换为for循环,以提高性能。
  8. 安全性

    • TaskManagerSettings::migrateFromDockedItems函数中,应该检查dockedDesktopFilesStrList中的每个元素是否为有效的JSON对象,以防止潜在的解析错误。
  9. 代码重复

    • TaskManager::saveDockElementsOrder函数中,for循环用于构建appIds列表,可以提取为一个独立的函数,以减少代码重复。
  10. 未使用的代码

    • ItemModel::moveTo函数在ItemModel.h中已被注释掉,应该完全移除,以保持代码的整洁。
  11. 未处理的异常

    • TaskManagerSettings::migrateFromDockedItems函数中,如果YAML::Load失败,应该有异常处理机制,以防止程序崩溃。
  12. 代码文档

    • TaskManagerSettings::setDockedElementsTaskManagerSettings::appendDockedElements函数的文档注释应该更详细地说明这些函数的用途和参数。

以上是针对代码提交的详细审查意见,旨在提高代码质量、可读性和可维护性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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 d84eb17 into linuxdeepin:master Aug 5, 2025
8 of 10 checks passed
@BLumia BLumia deleted the dockedelemets_key branch August 5, 2025 08:39
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