Skip to content

fix(ComboBox): fix highlight lingering after mouse leaves#585

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
electricface:swt/fix-bug306683
Mar 23, 2026
Merged

fix(ComboBox): fix highlight lingering after mouse leaves#585
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
electricface:swt/fix-bug306683

Conversation

@electricface
Copy link
Member

@electricface electricface commented Mar 10, 2026

  • Track the popup's hover state by adding a isInteractingWithContent property,
    and modify the highlighted property in the delegate.
  • The logic of the highlighted property now depends on
    isInteractingWithContent, automatically clearing the highlight when the
    mouse leaves.

修复鼠标移出后高亮项仍然残留的问题

  • 通过添加isInteractingWithContent属性跟踪popup悬停状态,修改delegate的highlighted
  • highlighted 属性逻辑依赖isInteractingWithContent,鼠标离开时自动清除高亮

Log: 修复ComboBox的菜单项鼠标离开后高亮残留问题
Influence: ComboBox菜单项
PMS: BUG-304991

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 10, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

ComboBox menu item highlighting is now gated on a new popup-level hover state so that the highlighted style clears when the mouse leaves the popup list area, implemented separately for Qt5 (MouseArea) and Qt6 (HoverHandler).

Class diagram for updated ComboBox popup and delegate highlighting (Qt5 and Qt6)

classDiagram
    class ComboBox {
        int currentIndex
        int highlightedIndex
        bool hoverEnabled
        Popup popup
    }

    class Popup {
        bool contentHovered
        int implicitWidth
        ArrowListView contentItem
    }

    class ArrowListView {
        int maxVisibleItems
        ListView view
    }

    class ListView {
        int currentIndex
        int highlightRangeMode
        int highlightMoveDuration
    }

    class DelegateItem {
        bool highlighted
        bool hoverEnabled
        bool autoExclusive
        bool checked
        +updateHighlighted(popupContentHovered, controlHighlightedIndex, index)
    }

    class MouseArea_Qt5 {
        bool hoverEnabled
        int acceptedButtons
        +onContainsMouseChanged(containsMouse)
    }

    class HoverHandler_Qt6 {
        +onHoveredChanged(hovered)
    }

    ComboBox --> Popup : owns
    Popup --> ArrowListView : contentItem
    ArrowListView --> ListView : view
    ArrowListView --> MouseArea_Qt5 : Qt5 only
    ArrowListView --> HoverHandler_Qt6 : Qt6 only
    ListView --> DelegateItem : provides delegates

    DelegateItem : highlighted = popup.contentHovered && control.highlightedIndex === index
    MouseArea_Qt5 : onContainsMouseChanged sets popup.contentHovered
    HoverHandler_Qt6 : onHoveredChanged sets popup.contentHovered
Loading

Flow diagram for ComboBox popup hover state driving delegate.highlighted

flowchart TD
    A[Mouse enters menu item area] --> B[Popup.contentHovered set to true]
    B --> C[Control.highlightedIndex updated for hovered item]
    C --> D[Delegate.highlighted evaluates popup.contentHovered && control.highlightedIndex === index]
    D --> E[Matching delegate shows highlighted style]

    F[Mouse leaves popup list area] --> G[Popup.contentHovered set to false]
    G --> H[Delegate.highlighted re-evaluates to false for all items]
    H --> I[All menu items clear highlighted style]
Loading

File-Level Changes

Change Details Files
Gate ComboBox delegate highlighting on popup list hover state to avoid lingering highlight after mouse exit.
  • Introduce popup.contentHovered boolean property to represent whether the mouse is inside the popup list area.
  • Update menu item delegate.highlighted binding to require both popup.contentHovered and control.highlightedIndex === index.
  • In the Qt5 ComboBox, add a full-area MouseArea with hoverEnabled and acceptedButtons: Qt.NoButton to keep popup.contentHovered in sync with pointer presence over the list content.
  • In the Qt6 ComboBox, add a HoverHandler in the list content to keep popup.contentHovered in sync with pointer hover state.
  • Update copyright year range in the Qt5 ComboBox QML file header.
src/qml/ComboBox.qml
qt6/src/qml/ComboBox.qml

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 found 1 issue, and left some high level feedback:

  • In both Qt5 and Qt6 versions, tying delegate.highlighted to popup.contentHovered && control.highlightedIndex === index means keyboard-based navigation will no longer visually highlight items when the mouse is outside the popup; consider whether contentHovered should be ignored (or handled differently) for focus/keyboard navigation to preserve accessibility and expected behavior.
  • For the Qt6 HoverHandler, instead of imperatively setting popup.contentHovered in onHoveredChanged, consider binding popup.contentHovered directly to the handler’s hovered property to avoid potential desynchronization if additional hover logic is added later.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In both Qt5 and Qt6 versions, tying `delegate.highlighted` to `popup.contentHovered && control.highlightedIndex === index` means keyboard-based navigation will no longer visually highlight items when the mouse is outside the popup; consider whether `contentHovered` should be ignored (or handled differently) for focus/keyboard navigation to preserve accessibility and expected behavior.
- For the Qt6 `HoverHandler`, instead of imperatively setting `popup.contentHovered` in `onHoveredChanged`, consider binding `popup.contentHovered` directly to the handler’s `hovered` property to avoid potential desynchronization if additional hover logic is added later.

## Individual Comments

### Comment 1
<location path="qt6/src/qml/ComboBox.qml" line_range="39-42" />
<code_context>
         text: control.textRole ? (Array.isArray(control.model) ? modelData[control.textRole] : (model[control.textRole] === undefined ? modelData[control.textRole] : model[control.textRole])) : modelData
         icon.name: (control.iconNameRole && model[control.iconNameRole] !== undefined) ? model[control.iconNameRole] : null
-        highlighted: control.highlightedIndex === index
+        highlighted: popup.contentHovered && control.highlightedIndex === index
         hoverEnabled: control.hoverEnabled
         autoExclusive: true
</code_context>
<issue_to_address>
**issue (bug_risk):** In Qt 6, `popup` is both missing an `id` and not visible from the delegate context.

`popup.contentHovered` will always fail here: the `Popup` has no `id`, so `popup` is undefined in this file, and even with `id: popup` the delegate can’t see it because the popup isn’t in the delegate’s context. Instead, expose the hover state via something the delegate can access (for example, a `control.popupContentHovered` property updated by the popup) and bind `highlighted` to that property.
</issue_to_address>

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.

text: control.textRole ? (Array.isArray(control.model) ? modelData[control.textRole] : (model[control.textRole] === undefined ? modelData[control.textRole] : model[control.textRole])) : modelData
icon.name: (control.iconNameRole && model[control.iconNameRole] !== undefined) ? model[control.iconNameRole] : null
highlighted: control.highlightedIndex === index
highlighted: popup.contentHovered && control.highlightedIndex === index
Copy link
Contributor

Choose a reason for hiding this comment

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

popup是可以从外部赋值的,不应该在ComboBox里访问popup,可以参照 https://github.com/linuxdeepin/dtkdeclarative/pull/569/changes

@electricface electricface force-pushed the swt/fix-bug306683 branch 4 times, most recently from 5edc15e to 8398b4e Compare March 11, 2026 03:51
@electricface electricface changed the title fix(ComboBox): fix the issue of highlighted state lingering after mouse leaves menu items fix(ComboBox): fix highlight lingering and add keyboard navigation in popup Mar 11, 2026
view.highlightRangeMode: ListView.ApplyRange
view.highlightMoveDuration: 0
Keys.priority: Keys.BeforeItem
Keys.onPressed: (event) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

这是手搓了个ComboBox的按键处理呀,无法借助qt里面的处理么,

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Mar 12, 2026

TAG Bot

New tag: 6.7.36
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #587

@electricface electricface changed the title fix(ComboBox): fix highlight lingering and add keyboard navigation in popup fix(ComboBox): fix highlight lingering after mouse leaves Mar 20, 2026
@electricface
Copy link
Member Author

Code review

Found 1 issue:

  1. Missing state reset when popup closes: The contentHovered property is initialized to false but never reset when the popup closes. Since the Popup object is persistent between open/close cycles, a stale true value from a previous session (e.g., from keyboard navigation) will persist when the popup is reopened, causing highlights to appear immediately without user interaction.

    Fix: Add an onClosed handler to reset the state:

    onClosed: {
        contentHovered = false
    }

    popup: Popup {
    id: popup
    leftMargin: DS.Style.popup.margin
    rightMargin: DS.Style.popup.margin
    palette: control.palette
    implicitWidth: control.flat ? Math.max(contentItem.implicitWidth, control.width) : control.width
    property bool contentHovered: false
    Connections {
    target: control
    function onHighlightedIndexChanged() {
    if (control.highlightedIndex >= 0)
    popup.contentHovered = true
    }
    }
    contentItem: ArrowListView {
    clip: true
    maxVisibleItems: control.maxVisibleItems
    view.model: control.delegateModel
    view.currentIndex: control.highlightedIndex
    view.highlightRangeMode: ListView.ApplyRange
    view.highlightMoveDuration: 0
    HoverHandler {
    onHoveredChanged: popup.contentHovered = hovered
    }
    }

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@electricface electricface force-pushed the swt/fix-bug306683 branch 2 times, most recently from 7766bca to dd67e9e Compare March 20, 2026 10:51
- Track the popup's hover state by adding a isInteractingWithContent property
  to ComboBox, and modify the highlighted property in the delegate.
- The logic of the highlighted property now depends on
  isInteractingWithContent, automatically clearing the highlight when the
  mouse leaves.

修复鼠标移出后高亮项仍然残留的问题

- 在ComboBox上添加isInteractingWithContent属性跟踪popup悬停状态,修改delegate的highlighted属性
- highlighted属性逻辑依赖isInteractingWithContent,鼠标离开时自动清除高亮

Log: 修复ComboBox的菜单项鼠标离开后高亮残留问题
Influence: ComboBox菜单项
PMS: BUG-304991
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这段代码主要在 ComboBox.qml 中增加了一个 isInteractingWithContent 属性,用于控制下拉列表项的高亮显示逻辑。以下是对该 diff 的审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面的改进建议:

1. 语法逻辑

  • 高亮逻辑修改

    • 原代码:highlighted: control.highlightedIndex === index
    • 新代码:highlighted: control.isInteractingWithContent ? control.highlightedIndex === index : false
    • 问题:新逻辑强制要求 isInteractingWithContenttrue 时才显示高亮。这改变了组件的默认行为。如果用户未进行任何交互(如鼠标悬停或键盘导航),高亮将完全消失。这可能会影响用户体验,特别是对于无障碍访问或键盘导航的用户,他们可能期望初始状态下有一个默认高亮项。
    • 建议:确认这是否是预期的交互行为。如果目的是仅在用户交互时高亮,那么逻辑是正确的。如果希望保留初始高亮(例如默认选中项),则需要调整逻辑。
  • 状态重置时机

    • onClosed: control.isInteractingWithContent = false 确保了关闭弹窗时重置状态,这是合理的。
    • onHighlightedIndexChanged 中,当索引有效时设置为 true,这能响应键盘导航。
    • 潜在冲突HoverHandleronHoveredChanged 会根据鼠标悬停状态直接修改 isInteractingWithContent
    • 问题:如果用户使用键盘导航(触发 onHighlightedIndexChanged),然后鼠标移入(触发 onHoveredChanged),再移出(触发 onHoveredChanged 设为 false),会导致高亮突然消失,即使当前可能仍然有选中的索引。这种鼠标和键盘操作的混合可能会导致状态闪烁。

2. 代码质量

  • 属性命名isInteractingWithContent 是一个布尔属性,命名符合 QML 规范,清晰易懂。
  • 状态管理:目前该属性由三个不同的地方控制(onClosedonHighlightedIndexChangedHoverHandler)。这种分散的状态管理容易导致逻辑冲突,如上所述。
  • 代码耦合HoverHandler 被添加到了 contentItem 的父级(即 popupbackground)。如果 popup 结构发生变化,这个 Handler 可能会失效或影响其他组件。

3. 代码性能

  • 属性绑定highlighted 属性现在依赖于 isInteractingWithContent。每次 isInteractingWithContent 变化时,所有列表项的 highlighted 状态都会重新计算。如果列表项非常多,可能会有轻微的性能开销,但在常规 UI 场景下通常可以忽略不计。
  • 频繁更新HoverHandler 会在鼠标移动时频繁触发事件,导致 isInteractingWithContent 频繁更新。虽然布尔赋值开销很小,但会触发属性绑定链。

4. 代码安全

  • 外部修改风险isInteractingWithContent 是一个普通属性,外部代码可以随意修改它,这可能导致内部逻辑混乱。
  • 建议:如果这是一个内部状态,建议将其设为 readonly 或者通过更严格的逻辑来保护它,防止外部意外修改。

5. 改进建议

为了解决上述逻辑冲突和状态管理问题,建议进行以下修改:

方案 A:优化状态逻辑(推荐)

明确交互的定义,避免鼠标移出时立即清除状态。可以将状态分为“键盘交互中”和“鼠标交互中”,或者仅在特定条件下清除状态。

// ... 其他代码
property bool isInteractingWithContent: false
// ... 其他代码

popup: T.Popup {
    // ... 其他配置
    
    onClosed: control.isInteractingWithContent = false
    
    Connections {
        target: control
        function onHighlightedIndexChanged() {
            // 键盘导航时保持交互状态
            if (control.highlightedIndex >= 0)
                control.isInteractingWithContent = true
        }
        function onActiveFocusChanged() {
            // 如果控件失去焦点,通常意味着交互结束
            if (!control.activeFocus) {
                control.isInteractingWithContent = false
            }
        }
    }

    HoverHandler {
        // 使用 target 指定具体的 ListView 或内容区域,而不是整个 Popup
        // 这样可以避免在 Popup 的边距区域移动鼠标时触发状态变化
        id: hoverHandler
        onHoveredChanged: {
            // 仅当鼠标悬停在内容上时才更新状态
            // 并且不直接设置为 false,以免覆盖键盘操作的状态
            if (hovered) {
                control.isInteractingWithContent = true
            }
            // 注意:这里移除了 !hovered 时设为 false 的逻辑
            // 依靠 onClosed 或 onActiveFocusChanged 来重置
        }
    }
    
    // ... 其他代码
}

方案 B:使用状态机(复杂场景)

如果交互逻辑非常复杂,建议使用 QML 的 StateTransition 或者简单的状态机逻辑来管理 isInteractingWithContent,而不是分散在各个信号处理器中。

总结

这段代码的初衷是为了改善高亮显示的交互体验,但目前的实现方式在鼠标和键盘混合操作时可能存在逻辑冲突。建议优化状态更新的逻辑,确保 isInteractingWithContent 的状态变化是确定且符合预期的,特别是在处理鼠标移出和弹窗关闭的时机上。此外,限制 HoverHandler 的作用域可以提高代码的健壮性。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, electricface

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

@electricface
Copy link
Member Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Mar 23, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 5d10663 into linuxdeepin:master Mar 23, 2026
20 of 21 checks passed
@electricface electricface deleted the swt/fix-bug306683 branch March 23, 2026 07:31
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.

3 participants