fix: improve menu interaction and notification center handling#1514
Open
qxp930712 wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: improve menu interaction and notification center handling#1514qxp930712 wants to merge 1 commit intolinuxdeepin:masterfrom
qxp930712 wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
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
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
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
|
deepin pr auto review这段代码主要涉及QML界面的交互逻辑，特别是关于菜单弹出、焦点管理和通知中心关闭的处理。以下是对这段代码的详细审查和改进意见： 1. 语法逻辑问题 1：重复代码 let notifyPanel = DS.applet("org.deepin.ds.notificationcenter")
if (notifyPanel && notifyPanel.visible) {
notifyPanel.close()
}改进建议：建议将这段逻辑封装成一个独立的函数或工具类，例如在 问题 2：键盘抓取逻辑不一致 2. 代码质量问题 1：版权年份更新 问题 2：缺少错误处理 let notifyPanel = DS.applet("org.deepin.ds.notificationcenter")改进建议：虽然后面有 3. 代码性能问题 1：频繁调用 DS.applet property var notifyPanel: DS.applet("org.deepin.ds.notificationcenter")
// 在需要关闭的地方
if (notifyPanel && notifyPanel.visible) {
notifyPanel.close()
}4. 代码安全问题 1：潜在的竞态条件 问题 2：外部依赖 5. 其他建议建议 1：添加注释 建议 2：单元测试 总结这段代码的主要改进方向是：
通过以上改进，代码的可维护性、性能和安全性都会得到提升。 |
![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
| Panel.requestClosePopup() | ||
| viewDeactivated() | ||
| hideTimer.stop() | ||
| let notifyPanel = DS.applet("org.deepin.ds.notificationcenter") |
Contributor
There was a problem hiding this comment.
必须要这样跟任务栏通讯么，如果这样的话，那剪切板怎么处理呢？
能不能通过失去焦点来处理？
| menuWindow.title = windowTitle | ||
| menuWindow.show() | ||
| DS.grabMouse(menuWindow) | ||
| DS.grabKeyboard(menuWindow) |
Contributor
There was a problem hiding this comment.
测下grabKeyboard之后有没有其它的问题，之前好像是有这个，后来为了避免啥才删掉的，
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: