fix(notification): hide close button when mouse leaves center after e…#1427
fix(notification): hide close button when mouse leaves center after e…#1427deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds hover tracking for the entire notification center and threads that state through the view and notification items so that the close button only remains visible when either hovered or focused while the mouse is still within the notification center area. Sequence diagram for mouse leave handling and close button visibilitysequenceDiagram
actor User
participant Mouse as MousePointer
participant NotifyCenter
participant HoverHandler as centerHoverHandler
participant NotifyView
participant NormalNotify
participant Impl as ImplComponent
User->>Mouse: Move inside notification center
Mouse->>HoverHandler: hover enter
HoverHandler-->>NotifyCenter: hovered = true
NotifyCenter->>NotifyView: centerHovered = mouseInCenter(true)
NotifyView->>NormalNotify: centerHovered = true
NormalNotify->>Impl: parentHovered = impl.hovered || (activeFocus && centerHovered)
Impl-->>User: Close button visible
User->>Mouse: Move outside notification center
Mouse->>HoverHandler: hover leave
HoverHandler-->>NotifyCenter: hovered = false
NotifyCenter->>NotifyView: centerHovered = mouseInCenter(false)
NotifyView->>NormalNotify: centerHovered = false
NormalNotify->>Impl: recompute parentHovered
Impl-->>User: Close button hidden when only focus is active
Class diagram for updated notification center hover trackingclassDiagram
class NotifyCenter {
int maxViewHeight
int stagingViewCount
int viewCount
bool mouseInCenter
gotoStagingLast()
gotoStagingFirst()
}
class HoverHandler_center {
bool hovered
}
class NotifyView {
alias notifyModel
alias viewPanelShown
real viewHeight
int viewCount
bool centerHovered
gotoHeaderFirst()
gotoHeaderLast()
}
class NotifyViewDelegate {
NotifyModel notifyModel
Item view
bool centerHovered
setting(var pos, var params)
}
class NormalNotify {
int implicitWidth
int implicitHeight
bool centerHovered
gotoNextItem()
gotoPrevItem()
}
class ImplComponent {
bool hovered
bool strongInteractive
string contentIcon
int contentRowCount
bool parentHovered
}
NotifyCenter --> HoverHandler_center : contains
NotifyCenter --> NotifyView : contains
NotifyView --> NotifyViewDelegate : uses
NotifyViewDelegate --> NormalNotify : selects_as_delegate
NotifyView --> NotifyModel : uses
NotifyView --> Item : view
NormalNotify --> ImplComponent : contains
NotifyCenter : mouseInCenter = centerHoverHandler.hovered
NotifyView : centerHovered from NotifyCenter.mouseInCenter
NotifyViewDelegate : centerHovered required
NormalNotify : centerHovered from NotifyView.centerHovered
ImplComponent : parentHovered = impl.hovered || (root.activeFocus && root.centerHovered)
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:
- The same concept is referred to as
mouseInCenterinNotifyCenter.qmlandcenterHoveredin other components; consider standardizing on a single property name to make the data flow easier to follow. - You now gate
parentHoveredonroot.activeFocus && root.centerHovered; if keyboard focus alone (with the mouse outside the center) should still visually indicate hover/close affordances for accessibility, you may want to revisit this condition or add a separate visual state for keyboard focus.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The same concept is referred to as `mouseInCenter` in `NotifyCenter.qml` and `centerHovered` in other components; consider standardizing on a single property name to make the data flow easier to follow.
- You now gate `parentHovered` on `root.activeFocus && root.centerHovered`; if keyboard focus alone (with the mouse outside the center) should still visually indicate hover/close affordances for accessibility, you may want to revisit this condition or add a separate visual state for keyboard focus.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR aims to fix a bug where the close button remains visible after expanding a notification when the mouse leaves the notification center area. The fix introduces mouse position tracking through a centerHovered property and keyboard navigation state tracking via shouldShowClose to control when the close button should be displayed.
Changes:
- Refactored Tab/Backtab navigation to use parent Control handlers instead of item-level handlers
- Added
centerHoveredandshouldShowCloseproperties to track mouse and keyboard focus states - Improved focus management functions to properly handle disabled buttons and return success/failure status
- Introduced
focusFirstActionOnly()to avoid circular focus loops in OverlapNotify
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| NotifyItemContent.qml | Refactored focus navigation logic with improved return values and added focusFirstActionOnly() method |
| NotifyAction.qml | Enhanced button focus functions to handle disabled actions and return boolean success indicators |
| OverlapNotify.qml | Added centerHovered/shouldShowClose properties and external clearButton reference for focus management |
| NotifyViewDelegate.qml | Simplified delegate by removing item-level Tab handlers and passing centerHovered to child components |
| NormalNotify.qml | Added centerHovered/shouldShowClose properties and moved Tab handlers to internal Control |
| GroupNotify.qml | Added focusLastButton() function and moved Tab handlers to internal Control |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
TAG Bot New tag: 2.0.29 |
|
展开消息后需要考虑多次tab循环列表查看是否正常focus |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Keys.onBacktabPressed: function(event) { | ||
| root.gotoPrevItem() | ||
| event.accepted = true |
There was a problem hiding this comment.
Non-reversible tab order: Tab from the close button goes to action buttons, but Shift+Tab from action buttons skips the close button and goes to the previous item. This creates a non-reversible tab navigation where users cannot use Shift+Tab to return to the close button they just tabbed from. While this appears intentional based on the comment at line 334 in NotifyItemContent.qml, consider whether this provides a good user experience, as it violates the typical expectation that Shift+Tab reverses Tab navigation.
Refactor Tab/Shift+Tab navigation logic to properly handle disabled buttons and unify focus management across notification items. 改进通知中心键盘导航逻辑,正确处理禁用按钮的焦点跳转, 统一通知项和按钮的焦点管理。 Log: 改进通知中心键盘导航 PMS: BUG-347241 Influence: 使用键盘 Tab/Shift+Tab 浏览通知时,现在会正确跳过禁用的按钮,焦点顺序更符合预期;关闭按钮仅在键盘导航或鼠标悬停时显示,界面更简洁。
deepin pr auto reviewGit Diff 代码审查报告总体评价这段代码主要是对通知中心(Notification Center)的键盘导航功能进行了重构,改进了焦点管理和Tab/Shift+Tab导航逻辑。整体上代码结构清晰,逻辑较为合理,但存在一些可以优化的地方。 详细审查意见1. 语法逻辑NormalNotify.qml
OverlapNotify.qml
NotifyAction.qml
NotifyItemContent.qml
2. 代码质量GroupNotify.qml
NotifyViewDelegate.qml
3. 代码性能NotifyAction.qml
NotifyItemContent.qml
4. 代码安全所有文件
NotifyAction.qml
5. 其他建议键盘导航一致性
焦点顺序文档
总结这段代码重构了通知中心的键盘导航功能,整体上改进了焦点管理和Tab/Shift+Tab导航逻辑。主要优点是:
主要需要改进的地方:
建议在合并前进行充分的测试,特别是边缘情况,如所有按钮都禁用时的导航行为。 |
瑜哥,测试了TAB,都正常。
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, re2zero 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 |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
…xpand
Added mouse position tracking to notification center to prevent close button from staying visible after expand when mouse moves outside the center area.
在通知中心添加鼠标位置追踪,解决展开后鼠标移出中心时关闭按钮不隐藏的问题。
Log: 修复展开通知后鼠标移出中心关闭按钮不隐藏的问题
PMS: BUG-347241
Influence: 修复后,展开折叠通知并将鼠标移出通知中心区域时,展开的第一条信息右上角的关闭按钮会正确隐藏,提升用户体验。同时保留所有Tab键导航和键盘焦点功能。
Summary by Sourcery
Bug Fixes: