fix: Fix the issue where the dropdown remains highlighted after being…#569
Open
pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: Fix the issue where the dropdown remains highlighted after being…#569pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pengfeixx 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 guide (collapsed on small PRs)Reviewer's GuideUpdates ComboBox dropdown item highlighting logic so items only appear highlighted while the popup content is hovered or a submenu is open, fixing a bug where the dropdown remained highlighted after the cursor left the popup. Sequence diagram for ComboBox dropdown hover-based highlightingsequenceDiagram
actor User
participant ComboBox
participant PopupContent
participant HoverHandler
participant ListViewDelegate
User->>ComboBox: open()
ComboBox->>PopupContent: create_and_show()
loop for_each_item
PopupContent->>ListViewDelegate: instantiate_delegate(index)
ListViewDelegate->>ComboBox: bind_highlighted_to(mouseInPopupContent_or_subMenu_visible_and_highlightedIndex)
end
User->>PopupContent: move_mouse_inside()
PopupContent->>HoverHandler: hover_event(hovered_true)
HoverHandler->>ComboBox: set_mouseInPopupContent(true)
ComboBox->>ComboBox: highlightedIndex_changed()
ComboBox->>ListViewDelegate: reevaluate_highlighted_binding()
ListViewDelegate->>User: show_item_highlight()
User->>PopupContent: move_mouse_outside()
PopupContent->>HoverHandler: hover_event(hovered_false)
HoverHandler->>ComboBox: set_mouseInPopupContent(false)
ComboBox->>ListViewDelegate: reevaluate_highlighted_binding()
ListViewDelegate->>User: remove_item_highlight()
User->>ListViewDelegate: open_subMenu()
ListViewDelegate->>ListViewDelegate: subMenu_visible_true
ListViewDelegate->>User: keep_item_highlighted_while_subMenu_visible()
User->>ListViewDelegate: close_subMenu()
ListViewDelegate->>ListViewDelegate: subMenu_visible_false
ListViewDelegate->>ComboBox: reevaluate_highlighted_binding()
ComboBox->>ListViewDelegate: item_not_highlighted_when_mouse_not_in_popup_content()
ListViewDelegate->>User: remove_item_highlight()
Class diagram for updated ComboBox highlighting properties and handlersclassDiagram
class ComboBox {
int maxVisibleItems
Palette separatorColor
var horizontalAlignment
bool mouseInPopupContent
int highlightedIndex
onHighlightedIndexChanged()
}
class PopupContent {
ListView view
HoverHandler hoverHandler
}
class HoverHandler {
bool hovered
onHoveredChanged()
}
class ListView {
int currentIndex
int highlightRangeMode
int highlightMoveDuration
}
class ListViewDelegate {
int index
bool highlighted
bool hoverEnabled
bool autoExclusive
bool checked
bool subMenu_visible
}
ComboBox --> PopupContent : owns
PopupContent --> HoverHandler : contains
PopupContent --> ListView : contains
ListView --> ListViewDelegate : creates_delegates
ComboBox --> ListViewDelegate : controls_highlightedIndex
HoverHandler --> ComboBox : updates_mouseInPopupContent
ComboBox --> ListViewDelegate : controls_highlighted_via_mouseInPopupContent_and_subMenu_visible
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:
- Consider resetting
mouseInPopupContentwhen the popup closes (e.g., in the popup’sonVisibleChangedhandler) so that keyboard-only closing or programmatic closing does not leave the internal state stuck intrue. - Using
onHighlightedIndexChangedto setmouseInPopupContent = trueconflates keyboard-driven highlighting with mouse hover; if the intent is to represent actual mouse presence, it might be clearer to derive this solely from pointer/hover events.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider resetting `mouseInPopupContent` when the popup closes (e.g., in the popup’s `onVisibleChanged` handler) so that keyboard-only closing or programmatic closing does not leave the internal state stuck in `true`.
- Using `onHighlightedIndexChanged` to set `mouseInPopupContent = true` conflates keyboard-driven highlighting with mouse hover; if the intent is to represent actual mouse presence, it might be clearer to derive this solely from pointer/hover events.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
reviewed
Feb 6, 2026
qt6/src/qml/ComboBox.qml
Outdated
| 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: (control.mouseInPopupContent || (subMenu && subMenu.visible)) && control.highlightedIndex === index |
Contributor
There was a problem hiding this comment.
Combobox，没有subMenu这个场景吧，
Contributor
There was a problem hiding this comment.
这个highlighted去掉复制，直接用Menu里的写法，可以么？
… moved out Fix the issue where the dropdown remains highlighted after being moved out Log: Fix the issue where the dropdown remains highlighted after being moved out pms: BUG-304991
Contributor
deepin pr auto review这段代码的修改主要是在 1. 语法逻辑审查
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
改进建议代码T.ComboBox {
// ... 其他属性 ...
// 新增属性，用于标记鼠标是否在弹出层内容区域
// 建议添加注释说明用途
property bool mouseInPopupContent: false
// ... 其他代码 ...
delegate: ItemDelegate {
// ... 其他属性 ...
// 只有当鼠标在弹出层内且当前项是高亮索引时，才高亮显示
highlighted: control.mouseInPopupContent && control.highlightedIndex === index
// ... 其他属性 ...
}
// 当高亮索引改变时，标记鼠标在弹出层内
// 这通常意味着用户正在通过键盘或鼠标与弹出层交互
onHighlightedIndexChanged: {
mouseInPopupContent = true
}
// ... 其他代码 ...
popup: Popup {
// ... 其他代码 ...
// 监听弹出层关闭事件，重置鼠标状态标志
onClosed: {
control.mouseInPopupContent = false
}
contentItem: ListView {
// ... 其他属性 ...
// 使用 HoverHandler 监听鼠标悬停状态
HoverHandler {
onHoveredChanged: {
control.mouseInPopupContent = hovered
}
}
}
}
}改进点总结
这些改进将使代码更加健壮、可维护，并减少潜在的 Bug。 |
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
3 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.
… moved out
Fix the issue where the dropdown remains highlighted after being moved out
Log: Fix the issue where the dropdown remains highlighted after being moved out
pms: BUG-304991
Summary by Sourcery
Bug Fixes: