fix: 关闭弹窗时同时关闭启动器 #1397
Open
fix: 关闭弹窗时同时关闭启动器 #1397
Conversation
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
在 dock 空白区域右键时，启动器应该随其他弹窗一起关闭。 此修改为 dock 的 onRequestClosePopup 处理器添加了可选的启动器支持。 修改内容： - 添加 launcherControllerWrapper 属性以条件性支持启动器 - 在 Component.onCompleted 中使用 try-catch 初始化包装器以保证兼容性 - 在 onRequestClosePopup 中关闭启动器（如果包装器可用） - 移除延迟菜单定时器，关闭弹窗后直接打开菜单 - 修复 MenuHelper 正确断开 aboutToHide 处理器 PMS: BUG-323289
Reviewer's GuideAdds optional dde-launchpad integration so the launcher closes together with other dock popups, and fixes menu cleanup by correctly managing aboutToHide handlers while simplifying popup/menu timing. Sequence diagram for closing dock popups and optional launchersequenceDiagram
actor User
participant Dock as DockWindow
participant Popup as DockPopup
participant MenuHelper
participant LauncherWrapper as LauncherControllerWrapper
participant LauncherController
participant Menu
User->>Dock: rightClickBlankArea()
Dock->>Dock: onRequestClosePopup()
Dock->>Popup: DS.closeChildrenWindows(popup)
alt popup visible
Dock->>Popup: close()
end
alt launcherControllerWrapper exists
Dock->>LauncherWrapper: hide()
LauncherWrapper->>LauncherController: set visible = false
end
Dock->>MenuHelper: openMenu(contextMenu)
alt activeMenu exists
MenuHelper->>MenuHelper: activeMenu.close()
end
MenuHelper->>MenuHelper: disconnect previous aboutToHide handler for menu
MenuHelper->>MenuHelper: handler = function() { activeMenu = null }
MenuHelper->>Menu: menu.open()
MenuHelper->>Menu: menu.aboutToHide.connect(handler)
MenuHelper->>MenuHelper: activeMenu = menu
Updated class diagram for dock main window and MenuHelperclassDiagram
class DockWindow {
int dockRemainingSpaceForCenter
int dockPartSpacing
real dockItemIconSize
var launcherControllerWrapper
onRequestClosePopup()
onDockScreenChanged()
Component_onCompleted()
}
class LauncherControllerWrapper {
hide()
}
class LauncherController {
bool visible
}
class MenuHelper {
Menu activeMenu
var aboutToHideConnections
openMenu(menu)
closeMenu(menu)
}
class Menu {
open()
close()
aboutToHide
}
DockWindow --> LauncherControllerWrapper : optionally creates
LauncherControllerWrapper --> LauncherController : controls visibility
MenuHelper --> Menu : manages
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Using a plain JS object for
aboutToHideConnectionswithmenuobjects as keys will coerce the keys to strings (e.g.[object Object]), so different menu instances can collide; consider using aMapor attaching the handler directly as a property on themenuinstance to avoid incorrect disconnect behavior. aboutToHideConnectionsnever clears entries when a menu is closed or destroyed, so over time this can accumulate stale references and handlers; consider deleting the entry in theaboutToHidehandler or incloseMenuto keep the mapping in sync with live menus.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using a plain JS object for `aboutToHideConnections` with `menu` objects as keys will coerce the keys to strings (e.g. `[object Object]`), so different menu instances can collide; consider using a `Map` or attaching the handler directly as a property on the `menu` instance to avoid incorrect disconnect behavior.
- `aboutToHideConnections` never clears entries when a menu is closed or destroyed, so over time this can accumulate stale references and handlers; consider deleting the entry in the `aboutToHide` handler or in `closeMenu` to keep the mapping in sync with live menus.
## Individual Comments
### Comment 1
<location> `panels/dock/MenuHelper.qml:11` </location>
<code_context>
Item {
property Menu activeMenu: null
+ property var aboutToHideConnections: ({}) // Store connections by menu object
+
function openMenu(menu: Menu) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a plain JS object keyed by menu instances is unreliable because keys are coerced to strings.
Because QML/JS object keys are always strings, indexing with `aboutToHideConnections[menu]` will stringify `menu` (e.g. to `"[object Object]"`), so different `Menu` instances can collide and overwrite each other’s handlers. Instead, store the handler directly on the `menu` instance (e.g. `menu._aboutToHideHandler`) or use a small helper component that keeps the connection alongside the specific `Menu` object.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| Item { | ||
| property Menu activeMenu: null | ||
| property var aboutToHideConnections: ({}) // Store connections by menu object |
There was a problem hiding this comment.
issue (bug_risk): Using a plain JS object keyed by menu instances is unreliable because keys are coerced to strings.
Because QML/JS object keys are always strings, indexing with aboutToHideConnections[menu] will stringify menu (e.g. to "[object Object]"), so different Menu instances can collide and overwrite each other’s handlers. Instead, store the handler directly on the menu instance (e.g. menu._aboutToHideHandler) or use a small helper component that keeps the connection alongside the specific Menu object.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
在 dock 空白区域右键时，启动器应该随其他弹窗一起关闭。
此修改为 dock 的 onRequestClosePopup 处理器添加了可选的启动器支持。
修改内容：
PMS: BUG-323289
Summary by Sourcery
Ensure the dock closes the launcher along with other popups and improve menu cleanup handling.
Bug Fixes:
Enhancements: