Skip to content

fix: correct context menu coordinate mapping#448

Open
mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
mhduiy:menu
Open

fix: correct context menu coordinate mapping#448
mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
mhduiy:menu

Conversation

@mhduiy
Copy link
Copy Markdown
Contributor

@mhduiy mhduiy commented Apr 10, 2026

Changed the DockContextMenu parent from 'this' to 'nullptr' in PluginItem constructor to fix coordinate mapping issues when right-click menu is shown. The issue occurred because using 'this' as parent caused the menu's coordinate system to be relative to the PluginItem widget, leading to incorrect position mapping when the menu was displayed. By setting parent to nullptr, the menu becomes a top-level widget with screen coordinates, ensuring proper positioning. Also added explicit cleanup in destructor to safely delete the menu pointer.

Log: Fixed incorrect right-click menu position for dock plugin items

fix: 修复上下文菜单坐标映射问题

将 PluginItem 构造函数中 DockContextMenu 的父对象从 'this' 改为 'nullptr',以修复显示右键菜单时的坐标映射问题。问题原因是使用 'this' 作
为父对象会导致菜单的坐标系相对于 PluginItem 小部件,从而在显示菜单时产
生错误的位置映射。通过将父对象设置为 nullptr,菜单成为使用屏幕坐标的顶
层小部件,确保正确定位。同时在析构函数中添加了显式清理逻辑以安全删除菜单
指针。

Log: 修复了任务栏插件项右键菜单位置不正确的问题

PMS: BUG-352159

Summary by Sourcery

Fix context menu positioning for dock plugin items by adjusting the menu’s ownership and lifecycle management.

Bug Fixes:

  • Correct right-click context menu coordinate mapping for dock plugin items so menus appear at the correct screen position.

Enhancements:

  • Update plugin item source file copyright year range to include 2026.

Changed the DockContextMenu parent from 'this' to 'nullptr' in
PluginItem constructor to fix coordinate mapping issues when right-click
menu is shown. The issue occurred because using 'this' as parent caused
the menu's coordinate system to be relative to the PluginItem widget,
leading to incorrect position mapping when the menu was displayed. By
setting parent to nullptr, the menu becomes a top-level widget with
screen coordinates, ensuring proper positioning. Also added explicit
cleanup in destructor to safely delete the menu pointer.

Log: Fixed incorrect right-click menu position for dock plugin items

fix: 修复上下文菜单坐标映射问题

将 PluginItem 构造函数中 DockContextMenu 的父对象从 'this' 改为
'nullptr',以修复显示右键菜单时的坐标映射问题。问题原因是使用 'this' 作
为父对象会导致菜单的坐标系相对于 PluginItem 小部件,从而在显示菜单时产
生错误的位置映射。通过将父对象设置为 nullptr,菜单成为使用屏幕坐标的顶
层小部件,确保正确定位。同时在析构函数中添加了显式清理逻辑以安全删除菜单
指针。

Log: 修复了任务栏插件项右键菜单位置不正确的问题

PMS: BUG-352159
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Apr 10, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the DockContextMenu lifetime and parenting for PluginItem so the context menu uses global screen coordinates instead of widget-relative coordinates, and adds explicit cleanup to avoid leaks after making it a top-level widget.

Sequence diagram for updated context menu coordinate mapping

sequenceDiagram
    actor User
    participant PluginItem
    participant DockContextMenu

    User->>PluginItem: rightClick(eventPosInWidget)
    PluginItem->>PluginItem: mapToGlobal(eventPosInWidget)
    PluginItem->>DockContextMenu: setPosition(globalPos)
    PluginItem->>DockContextMenu: exec()

    Note over DockContextMenu: Parent is nullptr, menu is top level
    DockContextMenu-->>User: showAt(globalPos)
Loading

Class diagram for PluginItem and DockContextMenu ownership changes

classDiagram
    class QWidget
    class DockContextMenu
    class PluginsItemInterface

    class PluginItem {
        -QString m_itemKey
        -PluginsItemInterface* m_pluginsItemInterface
        -DockContextMenu* m_menu
        -QTimer* m_tooltipTimer
        -QWidget* m_tipsWidget
        +PluginItem(PluginsItemInterface* pluginItemInterface, QString itemKey, QWidget* parent)
        +~PluginItem()
        +QWidget* itemPopupApplet()
    }

    PluginItem --|> QWidget
    PluginItem o-- DockContextMenu : owns
    PluginItem o-- PluginsItemInterface : uses

    DockContextMenu --|> QWidget

    note for PluginItem "m_menu is created with parent nullptr and explicitly deleted in destructor"
Loading

File-Level Changes

Change Details Files
Make DockContextMenu a top-level menu using screen coordinates instead of PluginItem-relative coordinates.
  • Change DockContextMenu parent in PluginItem constructor from the PluginItem instance to nullptr so the menu is created as a top-level widget using global screen coordinates.
  • Keep other constructor initializations (tooltip timer, tips widget, signals) intact while only adjusting the menu instantiation.
src/loader/pluginitem.cpp
Add explicit destruction logic for the DockContextMenu instance now that it is not parented by PluginItem.
  • Replace the defaulted PluginItem destructor with a custom destructor that deletes m_menu when non-null and then nulls the pointer.
  • Ensure that menu cleanup is explicit to prevent memory leaks once QObject parent-child ownership is removed.
src/loader/pluginitem.cpp
Update SPDX header metadata to reflect the extended copyright year range.
  • Extend SPDX-FileCopyrightText year from 2011 - 2022 to 2011 - 2026.
src/loader/pluginitem.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

Copy link
Copy Markdown

@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 left some high level feedback:

  • Since m_menu is always initialized and only deleted in the destructor, the null-check and setting it to nullptr after delete are unnecessary; you can safely simplify the destructor to just delete m_menu; for clarity.
  • Now that DockContextMenu is created with a nullptr parent and manually deleted, consider whether using a parented QObject (e.g., a lightweight wrapper QObject owned by PluginItem) or a smart pointer would better express ownership and reduce the risk of future lifetime/cleanup mistakes if the menu’s lifecycle gets more complex.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Since `m_menu` is always initialized and only deleted in the destructor, the null-check and setting it to `nullptr` after `delete` are unnecessary; you can safely simplify the destructor to just `delete m_menu;` for clarity.
- Now that `DockContextMenu` is created with a `nullptr` parent and manually deleted, consider whether using a parented QObject (e.g., a lightweight wrapper QObject owned by `PluginItem`) or a smart pointer would better express ownership and reduce the risk of future lifetime/cleanup mistakes if the menu’s lifecycle gets more complex.

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.

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

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