fix: correct context menu coordinate mapping#448
Open
mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: correct context menu coordinate mapping#448mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
mhduiy wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts 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 mappingsequenceDiagram
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)
Class diagram for PluginItem and DockContextMenu ownership changesclassDiagram
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"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since
m_menuis always initialized and only deleted in the destructor, the null-check and setting it tonullptrafterdeleteare unnecessary; you can safely simplify the destructor to justdelete m_menu;for clarity. - Now that
DockContextMenuis created with anullptrparent and manually deleted, consider whether using a parented QObject (e.g., a lightweight wrapper QObject owned byPluginItem) 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.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
approved these changes
Apr 10, 2026
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
Enhancements: