Skip to content

fix(notification): hide close button when mouse leaves center after e…#1427

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
re2zero:bugfix
Feb 7, 2026
Merged

fix(notification): hide close button when mouse leaves center after e…#1427
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
re2zero:bugfix

Conversation

@re2zero
Copy link
Contributor

@re2zero re2zero commented Feb 4, 2026

…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:

  • Prevent notification close buttons from remaining visible after expanding a notification when the mouse moves outside the notification center area while focus is still active.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 4, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds 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 visibility

sequenceDiagram
    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
Loading

Class diagram for updated notification center hover tracking

classDiagram
    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)
Loading

File-Level Changes

Change Details Files
Track whether the mouse is inside the notification center and expose it as a property.
  • Introduce a readonly mouseInCenter property on the NotifyCenter root that reflects the HoverHandler.hovered state.
  • Add a HoverHandler to the notificationCenter Item to capture hover for the full center region.
panels/notification/center/NotifyCenter.qml
Propagate the notification center hover state down to notification items.
  • Add a centerHovered property to NotifyView and wire it from the NotifyCenter mouseInCenter property.
  • Require and pass the centerHovered property through NotifyViewDelegate to NormalNotify.
  • Add a centerHovered property to NormalNotify to receive the propagated state.
panels/notification/center/NotifyView.qml
panels/notification/center/NotifyViewDelegate.qml
panels/notification/center/NormalNotify.qml
Gate focused-state hover behavior on whether the mouse is inside the notification center so the close button hides when the pointer leaves.
  • Change parentHovered on NormalNotify implementation to impl.hovered

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • 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.
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.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@BLumia BLumia requested a review from 18202781743 February 4, 2026 03:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 centerHovered and shouldShowClose properties 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.

@deepin-bot
Copy link

deepin-bot bot commented Feb 5, 2026

TAG Bot

New tag: 2.0.29
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1436

@wyu71
Copy link
Contributor

wyu71 commented Feb 6, 2026

展开消息后需要考虑多次tab循环列表查看是否正常focus

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines 81 to 83
Keys.onBacktabPressed: function(event) {
root.gotoPrevItem()
event.accepted = true
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
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-ci-robot
Copy link

deepin pr auto review

Git Diff 代码审查报告

总体评价

这段代码主要是对通知中心(Notification Center)的键盘导航功能进行了重构,改进了焦点管理和Tab/Shift+Tab导航逻辑。整体上代码结构清晰,逻辑较为合理,但存在一些可以优化的地方。

详细审查意见

1. 语法逻辑

NormalNotify.qml

  • 问题: shouldShowClose属性初始值为false,但在Keys.onTabPressedKeys.onBacktabPressed中设置为true后,没有重置为false的机制
  • 建议: 考虑在失去焦点时重置shouldShowClose,可以添加onActiveFocusChanged处理:
    onActiveFocusChanged: {
        if (!activeFocus) {
            shouldShowClose = false
        }
    }

OverlapNotify.qml

  • 问题: clearButton属性定义为property var clearButton: null,没有类型检查
  • 建议: 使用更具体的类型定义,如property Item clearButton: null,并添加验证:
    onClearButtonChanged: {
        if (clearButton && !(clearButton instanceof Item)) {
            console.warn("clearButton must be an Item")
        }
    }

NotifyAction.qml

  • 问题: focusFirstButtonfocusLastButton函数中有重复逻辑
  • 建议: 提取公共逻辑为辅助函数:
    function focusActionAtIndex(index) {
        if (actions[index] && actions[index].enabled) {
            if (index === 0) {
                firstActionBtn.forceActiveFocus()
                return true
            } else if (index === 1 && secondActionLoader.item) {
                secondActionLoader.item.forceActiveFocus()
                return true
            } else if (index > 1 && moreActionsLoader.item) {
                moreActionsLoader.item.forceActiveFocus()
                return true
            }
        }
        return false
    }

NotifyItemContent.qml

  • 问题: focusFirstButtonfocusLastButton函数中检查actionLoader.item.enabledactionLoader.item.hasEnabledAction,但hasEnabledAction属性在NotifyAction.qml中是只读计算属性
  • 建议: 确保这个属性在所有情况下都能正确计算,特别是当actions数组为空时

2. 代码质量

GroupNotify.qml

  • 问题: focusLastButton函数直接调用groupClearBtn.forceActiveFocus(),但没有检查按钮是否存在或可用
  • 建议: 添加检查:
    function focusLastButton() {
        if (groupClearBtn && groupClearBtn.enabled && groupClearBtn.visible) {
            groupClearBtn.forceActiveFocus()
            return true
        }
        return false
    }

NotifyViewDelegate.qml

  • 问题: 代码中删除了调试用的Loader组件,但没有保留任何调试接口
  • 建议: 考虑保留一个条件编译或属性控制的调试接口,方便后续调试:
    Loader {
        anchors.fill: parent
        active: normalNotify.activeFocus && NotifyAccessor.debugging
        
        sourceComponent: FocusBoxBorder {
            radius: normalNotify.radius
            color: normalNotify.palette.highlight
        }
    }

3. 代码性能

NotifyAction.qml

  • 问题: hasEnabledAction属性每次访问都会遍历整个actions数组
  • 建议: 考虑在actions变化时缓存结果:
    property bool hasEnabledAction: false
    
    onActionsChanged: {
        hasEnabledAction = false
        for (let i = 0; i < actions.length; i++) {
            if (actions[i] && actions[i].enabled) {
                hasEnabledAction = true
                break
            }
        }
    }

NotifyItemContent.qml

  • 问题: focusFirstButtonfocusLastButton函数中有重复检查actionLoader.item.enabledactionLoader.item.hasEnabledAction
  • 建议: 可以缓存这些检查结果,避免多次访问

4. 代码安全

所有文件

  • 问题: 使用forceActiveFocus()可能导致焦点管理混乱,特别是在多个组件竞争焦点时
  • 建议: 考虑使用tryFocus()或类似的更安全的焦点设置方法,并处理焦点设置失败的情况:
    function safeForceActiveFocus(item) {
        if (item && item.enabled && item.visible) {
            item.forceActiveFocus()
            return item.activeFocus
        }
        return false
    }

NotifyAction.qml

  • 问题: 直接访问actions[i]可能导致类型错误
  • 建议: 添加类型检查:
    if (actions[i] && typeof actions[i] === 'object' && actions[i].enabled) {
        // ...
    }

5. 其他建议

键盘导航一致性

  • 问题: 不同类型的通知项(GroupNotify, NormalNotify, OverlapNotify)的键盘导航逻辑略有不同
  • 建议: 考虑创建一个通用的键盘导航接口或基类,确保所有通知项的行为一致

焦点顺序文档

  • 建议: 添加文档说明焦点顺序,例如:
    /**
     * Focus order for NormalNotify:
     * 1. Item itself (when navigating from previous item)
     * 2. Close button (if visible and enabled)
     * 3. Action buttons (from left to right)
     * 4. Next notification item
     */

总结

这段代码重构了通知中心的键盘导航功能,整体上改进了焦点管理和Tab/Shift+Tab导航逻辑。主要优点是:

  1. 将焦点管理逻辑集中到各个组件中,提高了代码的模块化
  2. 改进了键盘导航的用户体验,特别是对于禁用按钮的处理
  3. 增加了shouldShowClose属性,使关闭按钮的显示逻辑更加清晰

主要需要改进的地方:

  1. 添加对焦点状态的完整管理,特别是shouldShowClose属性的重置
  2. 增强类型检查和错误处理
  3. 优化性能,特别是hasEnabledAction属性的计算
  4. 提高代码复用性,减少重复逻辑
  5. 添加更多文档和注释,说明焦点顺序和键盘导航行为

建议在合并前进行充分的测试,特别是边缘情况,如所有按钮都禁用时的导航行为。

@re2zero
Copy link
Contributor Author

re2zero commented Feb 6, 2026

展开消息后需要考虑多次tab循环列表查看是否正常focus

瑜哥,测试了TAB,都正常。

  1. 全部折叠下按TAB切换,可以切换到每一个折叠的通知项,循环多次没有问题。
  2. 点开展开一个折叠通知(截图录屏36条),按TAB切换,此类通知最后一条通知后能切到下一个折叠通知,并能循环到通知最顶项,开始循环。循环5次以上,都正常。
    通知条TAB切换顺序:通知条 -> X 按钮 -> 通知内按钮 -> 下一条通知
    如果通知内没有按钮或按钮无效(置灰),则跳转到下一条通知。
    解决了当前如果通知内没按钮或按钮为灰,TAB再切不出来的情况。

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@re2zero re2zero requested a review from 18202781743 February 7, 2026 03:37
@deepin-ci-robot
Copy link

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@re2zero
Copy link
Contributor Author

re2zero commented Feb 7, 2026

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Feb 7, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 0d288ea into linuxdeepin:master Feb 7, 2026
16 of 17 checks passed
@re2zero re2zero deleted the bugfix branch February 7, 2026 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants