Skip to content

chore: implement dock item menus for DockGlobalElementModel#1229

Merged
BLumia merged 1 commit intolinuxdeepin:masterfrom
BLumia:new-dock-menu-actions
Aug 26, 2025
Merged

chore: implement dock item menus for DockGlobalElementModel#1229
BLumia merged 1 commit intolinuxdeepin:masterfrom
BLumia:new-dock-menu-actions

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Aug 25, 2025

This patch is to continue prepare for #1201, by add previously missing changed signal for Docked and Menu role when docked state is changed, and implementing actions for menu entries.

Summary by Sourcery

Enable dock item menus by wiring up dock/undock, force quit, and close-all actions and ensure UI roles update on docking state changes

New Features:

  • Introduce toggleDockedElement in TaskManagerSettings to support toggling docked state via menu actions
  • Add menu actions in DockGlobalElementModel for docking/undocking, force quit, and close all

Enhancements:

  • Rename appendDockedElements/removeDockedElements to singular method names and update their usages
  • Emit dataChanged notifications for both DockedRole and MenusRole when the docked elements list is reloaded

@BLumia BLumia requested a review from wjyrich August 25, 2025 09:06
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要是关于Dock任务管理器的停靠状态管理,我来对代码进行审查并提出改进意见:

  1. 代码质量:
  • 函数命名不一致:原代码中appendDockedElementsremoveDockedElements使用了复数形式,而新的appendDockedElementremoveDockedElement使用单数形式,建议统一使用单数形式,因为每次操作只处理一个元素。
  • 新增的toggleDockedElement函数设计良好,将切换逻辑封装在一个函数中,提高了代码的可读性和可维护性。
  1. 语法逻辑:
  • DockGlobalElementModel::requestNewInstance函数中的逻辑处理更加清晰,但可以进一步优化:
    • 对于DOCK_ACTION_DOCK分支,可以提取id的获取逻辑,避免重复代码。
    • requestClose的调用参数不一致,一个传入true,一个没有,建议明确参数含义。
  1. 性能优化:
  • loadDockedElements函数中,每次数据变更都触发整个模型的数据更新(dataChanged),即使只有部分数据发生变化。可以考虑只更新实际变化的项目,以提高性能。
  • m_dockedElements.removeAll操作在列表较大时可能会有性能问题,建议使用QSet来存储停靠元素,以提高查找和删除效率。
  1. 代码安全:
  • requestNewInstance函数中,直接使用index.row()访问数据没有进行边界检查,可能导致越界访问。建议添加适当的边界检查。
  • TaskManagerSettings类中的m_dockedElements成员变量没有看到访问控制,建议将其设为私有,并通过接口访问。
  1. 改进建议:
  • 考虑为停靠状态添加枚举类型,而不是使用字符串常量(如DOCK_ACTION_DOCK),这样可以提高类型安全性。
  • 可以添加日志记录,特别是对于状态变更操作,便于调试和问题追踪。
  • 建议为toggleDockedElement函数添加信号,以便在状态切换时通知相关方。
  • 考虑将DockGlobalElementModel中的数据更新逻辑进一步封装,减少重复代码。

总体而言,这段代码的功能实现是合理的,主要改进点在于代码的一致性、性能优化和安全性增强。通过上述改进,可以提高代码的可维护性和可靠性。

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 there - I've reviewed your changes - here's some feedback:

  • Extract the "desktop/" element string formatting into a shared helper to avoid duplicating that logic in the model and parser.
  • Guard the dataChanged emission in loadDockedElements against empty m_data to ensure you’re not emitting invalid index ranges.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the "desktop/<id>" element string formatting into a shared helper to avoid duplicating that logic in the model and parser.
- Guard the dataChanged emission in loadDockedElements against empty m_data to ensure you’re not emitting invalid index ranges.

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.

@sourcery-ai
Copy link

sourcery-ai bot commented Aug 25, 2025

Reviewer's Guide

This PR centralizes dock state management by introducing a toggle API and renaming singular dock methods, ensures the UI model emits updated data for both docked and menu roles after loading, and wires up context‐menu actions (dock, force‐quit, close‐all) to these new APIs.

File-Level Changes

Change Details Files
Add toggleDockedElement API and rename singular dock methods
  • Rename appendDockedElements to appendDockedElement
  • Rename removeDockedElements to removeDockedElement
  • Introduce toggleDockedElement to add/remove based on current state
  • Update header declarations accordingly
panels/dock/taskmanager/taskmanagersettings.cpp
panels/dock/taskmanager/taskmanagersettings.h
Emit dataChanged for DockedRole and MenusRole after loading
  • After updating m_dockedElements, emit dataChanged covering both DockedRole and MenusRole
  • Guard emission on non-empty data set
panels/dock/taskmanager/dockglobalelementmodel.cpp
Implement context‐menu actions for dock toggling and closing
  • Handle DOCK_ACTION_DOCK via toggleDockedElement
  • Route DOCK_ACTION_FORCEQUIT and DOCK_ACTION_CLOSEALL to requestClose
panels/dock/taskmanager/dockglobalelementmodel.cpp
Update parser to use new singular dock methods
  • Replace calls to appendDockedElements with appendDockedElement
  • Replace calls to removeDockedElements with removeDockedElement
panels/dock/taskmanager/desktopfileabstractparser.cpp

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

@BLumia BLumia requested a review from 18202781743 August 25, 2025 09:17
This patch is to continue prepare for linuxdeepin#1201,
by add previously missing changed signal for Docked and Menu role
when docked state is changed, and implementing actions for menu
entries.

This patch also make DnD on dock item works without using the
legacy ItemModel class by implementing requestOpenUrls().

Log:
@BLumia BLumia force-pushed the new-dock-menu-actions branch from 5a2fb99 to e80a6c5 Compare August 26, 2025 03:24
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

@BLumia BLumia merged commit 986ac11 into linuxdeepin:master Aug 26, 2025
9 of 10 checks passed
@BLumia BLumia deleted the new-dock-menu-actions branch August 26, 2025 03:54
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