fix: improve menu interaction and notification center handling#1514
Merged
18202781743 merged 1 commit intolinuxdeepin:masterfrom Mar 20, 2026
Merged
fix: improve menu interaction and notification center handling#151418202781743 merged 1 commit intolinuxdeepin:masterfrom
18202781743 merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideThis PR tightens focus and visibility handling around dock-related menus and tray popups by grabbing the keyboard for panel menus and auto-closing the notification center when opening dock menus, app context menus, and tray popups, while also updating copyright years. Sequence diagram for dock or tray menu opening with notification center handlingsequenceDiagram
actor User
participant DockOrTrayItem
participant MenuHelper
participant PanelMenuWindow
participant DS
participant NotificationCenterPanel
User->>DockOrTrayItem: click()
DockOrTrayItem->>DS: applet(org.deepin.ds.notificationcenter)
DS-->>DockOrTrayItem: notifyPanel
alt Notification center is visible
DockOrTrayItem->>NotificationCenterPanel: close()
end
DockOrTrayItem->>MenuHelper: openMenu(menuItem)
MenuHelper->>PanelMenuWindow: show()
MenuHelper->>PanelMenuWindow: setTitle(windowTitle)
MenuHelper->>DS: grabMouse(menuWindow)
MenuHelper->>DS: grabKeyboard(menuWindow)
User->>PanelMenuWindow: interact with menu (keyboard)
User->>PanelMenuWindow: close menu
PanelMenuWindow->>PanelMenuWindow: close()
PanelMenuWindow->>PanelMenuWindow: setCurrentItem(null)
PanelMenuWindow->>DS: grabKeyboard(menuWindow, false)
PanelMenuWindow->>DS: grabMouse(menuWindow, false)
Updated class diagram for dock menus, tray popups, and notification center focus handlingclassDiagram
class DS {
+applet(id)
+grabMouse(window)
+grabMouse(window, release)
+grabKeyboard(window)
+grabKeyboard(window, release)
}
class NotificationCenterPanel {
+visible bool
+close()
}
class MenuHelper {
+openMenu(menuItem)
}
class PanelMenuWindow {
+title string
+currentItem Item
+show()
+close()
}
class PanelMenu {
+windowTitle string
+menuWindow PanelMenuWindow
+showMenu()
+closeMenu()
}
class TrayItemSurfacePopup {
+popupMenu Item
+popupMenuContent Item
+openPopupMenu()
}
class SurfacePopup {
+menu Item
+openSurfaceMenu()
}
class AppItem {
+contextMenuLoader Item
+requestAppItemMenu()
}
DS --> NotificationCenterPanel : returns
PanelMenu --> PanelMenuWindow : owns
MenuHelper --> PanelMenuWindow : opens
TrayItemSurfacePopup --> DS : queries_applet
SurfacePopup --> DS : queries_applet
AppItem --> DS : queries_applet
TrayItemSurfacePopup --> NotificationCenterPanel : close_if_visible
SurfacePopup --> NotificationCenterPanel : close_if_visible
AppItem --> NotificationCenterPanel : close_if_visible
PanelMenu --> DS : grabMouse
PanelMenu --> DS : grabKeyboard
TrayItemSurfacePopup --> MenuHelper : indirect_menu_behavior
SurfacePopup --> MenuHelper : indirect_menu_behavior
AppItem --> MenuHelper : open_context_menu
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 left some high level feedback:
- The logic to fetch and close the notification center (
DS.applet("org.deepin.ds.notificationcenter")and visibility check) is duplicated in several places; consider extracting this into a shared helper (e.g., a function in a common utility QML) to keep behavior consistent and easier to maintain. - In
TrayItemSurfacePopup.qml,popupMenuContent.takeFocus()is called unconditionally; it may be safer to guard this with a null/visibility check (or useforceActiveFocus()on a known focusable item) to avoid runtime errors ifpopupMenuContentis not yet instantiated or focusable. - When adding
DS.grabKeyboard(menuWindow)inPanelMenu.qml, ensure that all paths that hide/destroy the menu window consistently go throughcloseMenu()(whereDS.grabKeyboard(menuWindow, false)is called), or otherwise release the keyboard grab to avoid lingering grabs in edge cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic to fetch and close the notification center (`DS.applet("org.deepin.ds.notificationcenter")` and visibility check) is duplicated in several places; consider extracting this into a shared helper (e.g., a function in a common utility QML) to keep behavior consistent and easier to maintain.
- In `TrayItemSurfacePopup.qml`, `popupMenuContent.takeFocus()` is called unconditionally; it may be safer to guard this with a null/visibility check (or use `forceActiveFocus()` on a known focusable item) to avoid runtime errors if `popupMenuContent` is not yet instantiated or focusable.
- When adding `DS.grabKeyboard(menuWindow)` in `PanelMenu.qml`, ensure that all paths that hide/destroy the menu window consistently go through `closeMenu()` (where `DS.grabKeyboard(menuWindow, false)` is called), or otherwise release the keyboard grab to avoid lingering grabs in edge cases.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
reviewed
Mar 19, 2026
panels/dock/package/main.qml
Outdated
| Panel.requestClosePopup() | ||
| viewDeactivated() | ||
| hideTimer.stop() | ||
| let notifyPanel = DS.applet("org.deepin.ds.notificationcenter") |
Contributor
There was a problem hiding this comment.
必须要这样跟任务栏通讯么，如果这样的话，那剪切板怎么处理呢？
能不能通过失去焦点来处理？
qxp930712
force-pushed
the
master
branch
2 times, most recently
from
c16a03a to
177fa1f
Compare
1. Add keyboard grab when showing panel menus to ensure proper input focus 2. Release keyboard grab when closing menus to prevent input conflicts 3. Close notification center panel when opening various dock menus (dock menu, app context menu, tray popups) to avoid overlapping UI elements 4. Add focus handling for tray item popups to ensure proper keyboard navigation 5. Update copyright years for consistency Log: Fixed menu interactions by ensuring proper focus management and preventing overlapping with notification center Influence: 1. Test opening dock menu while notification center is open - notification center should close 2. Verify app context menus close notification center when opened 3. Check tray popups properly close notification center 4. Test keyboard navigation in all menu types 5. Verify input focus is correctly managed between menus and other UI elements 6. Test menu interactions with multiple open applications PMS: BUG-284867
18202781743
approved these changes
Mar 20, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, qxp930712 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 |
deepin pr auto review这段代码主要涉及三个方面的修改：键盘焦点抓取、版权年份更新以及焦点获取。以下是对代码的详细审查和改进建议： 1. 代码逻辑审查修改点1：PanelMenu.qml 中添加键盘抓取 DS.grabKeyboard(menuWindow) // 打开菜单时抓取键盘
DS.grabKeyboard(menuWindow, false) // 关闭菜单时释放键盘
修改点2：TrayItemSurfacePopup.qml 中添加焦点获取 popupMenuContent.takeFocus()
2. 代码质量审查修改点3：版权年份更新 -// SPDX-FileCopyrightText: 2024 UnionTech Software Technology Co., Ltd.
+// SPDX-FileCopyrightText: 2024-2026 UnionTech Software Technology Co., Ltd.
3. 代码性能审查
4. 代码安全审查
5. 改进建议
6. 示例改进代码// PanelMenu.qml
Item {
function openMenu() {
menuWindow.title = windowTitle
menuWindow.show()
if (!DS.grabKeyboard(menuWindow)) {
console.warn("Failed to grab keyboard for menu window")
}
}
function closeMenu() {
menuWindow.close()
menuWindow.currentItem = null
if (!DS.ungrabKeyboard(menuWindow)) { // 假设存在 ungrabKeyboard
console.warn("Failed to release keyboard for menu window")
}
}
}
// TrayItemSurfacePopup.qml
Item {
function openPopupMenu() {
popupMenu.open()
// 确保键盘导航正常工作
if (!popupMenuContent.requestFocus()) { // 假设重命名为 requestFocus
console.warn("Failed to request focus for menu content")
}
}
}总结这段代码的修改逻辑是正确的，但可以通过更明确的 API 设计、错误处理和文档注释来提升代码质量和可维护性。建议在后续开发中逐步优化这些方面。 |
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
4 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.
Log: Fixed menu interactions by ensuring proper focus management and preventing overlapping with notification center
Influence:
PMS: BUG-284867
Summary by Sourcery
Improve panel and tray menu interaction by managing focus and avoiding conflicts with the notification center.
Bug Fixes:
Enhancements: