fix: Fix the issue where removing an item from the dropdown list does not cancel the highlight by pengfeixx · Pull Request #563 · linuxdeepin/dtkdeclarative

pengfeixx · 2026-02-02T05:45:47Z

Fix the issue where removing an item from the dropdown list does not cancel the highlight

Log: Fix the issue where removing an item from the dropdown list does not cancel the highlight
pms: BUG-306683
BUG-304991

sourcery-ai

Sorry @pengfeixx, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

18202781743 · 2026-02-02T06:05:35Z

qt6/src/qml/ComboBox.qml

+                }
+            }
+
+            ArrowListView {


这里Item用的是MenuItem，是不是应该从MenuItem里去做这种颜色的变化呀，

这里Item用的是MenuItem，是不是应该从MenuItem里去做这种颜色的变化呀，

高亮还是在menuitem里处理的，但是捕获鼠标移入移出还是要在外面listview里来做

deepin-ci-robot · 2026-02-02T07:54:11Z

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

Details

Needs approval from an approver in each of these files:

debian/deepin/OWNERS

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

18202781743 · 2026-02-02T08:10:16Z

qt6/src/qml/ComboBox.qml

        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
+        highlightEnabled: arrowListView.hovered || (control.highlightedIndex === index && subMenu && subMenu.visible)


那还需要这个highlightEnabled么？能直接用highlight么，

那还需要这个highlightEnabled么？能直接用highlight么，

已修改

18202781743 · 2026-02-02T08:10:27Z

qt6/src/qml/ArrowListView.qml

+    HoverHandler {
+        target: itemsView
+        onHoveredChanged: {
+            control.hovered = hovered


这种应该可以直接绑定，

这种应该可以直接绑定，

已修改

… not cancel the highlight Fix the issue where removing an item from the dropdown list does not cancel the highlight Log: Fix the issue where removing an item from the dropdown list does not cancel the highlight pms: BUG-306683 BUG-304991

deepin-ci-robot · 2026-02-02T09:04:53Z

deepin pr auto review

这份代码审查主要针对提供的git diff，对ArrowListView.qml、ComboBox.qml和Menu.qml三个文件的修改进行分析。

总体评价

这段代码的目的是增强ComboBox和Menu组件的交互体验，主要是通过引入HoverHandler来处理鼠标悬停状态，从而更精确地控制列表项的高亮显示和当前索引的清除。代码逻辑基本符合QML的语法规范，但在代码质量和性能方面有一些可以优化的地方。

详细审查

1. 语法与逻辑

ArrowListView.qml:
- 新增属性: property bool hovered: hoverHandler.hovered
  - 意见: 语法正确。将内部状态暴露为属性供外部使用是合理的。
- HoverHandler: target: itemsView
  - 意见: 逻辑上，将itemsView作为悬停目标意味着只有鼠标在列表视图内部移动时才会触发。如果itemsView有边距或装饰元素未被覆盖，鼠标在这些区域移动时可能不会触发hovered。如果这是预期行为，则无需修改；否则应考虑将target设为parent或更广泛的区域。
ComboBox.qml:
- 导入: import QtQuick.Controls
  - 意见: 引入QtQuick.Controls是为了使用HoverHandler（自Qt 5.15起属于Qt Quick模块，但在某些版本或配置下可能属于Controls）。如果项目依赖较新的Qt版本（如Qt 6），这是标准的。
- 高亮逻辑修改: highlighted: (control.highlightedIndex === index) && (arrowListView.hovered || (subMenu && subMenu.visible))
  - 意见: 逻辑上，这修复了一个常见的UX问题：当鼠标移出下拉列表但未点击时，高亮状态应该消失（除非有子菜单打开）。这是一个很好的改进。
  - 潜在风险: arrowListView是在contentItem中定义的。在delegate中直接访问contentItem的id（arrowListView）依赖于QML的作用域链。虽然通常可行，但这种隐式依赖降低了代码的可维护性。如果contentItem被替换或重构，这里会出问题。建议通过control属性显式传递这个状态，或者确认ArrowListView本身就是control的一部分且结构稳定。
Menu.qml:
- HoverHandler: target: viewLayout
  - 意见: 与ArrowListView类似，确保viewLayout覆盖了所有需要响应鼠标的区域。
- 逻辑逻辑: onHoveredChanged中检查子菜单状态。
  - 意见: 逻辑是合理的：只有当鼠标离开菜单且没有打开的子菜单时，才清除当前选中项（currentIndex = -1）。这防止了在导航到子菜单时父菜单的高亮丢失。
- 代码风格: 使用了var声明变量。
  - 意见: 在QML/JS中，var会根据赋值改变类型。对于currentItem，如果itemAt返回的是QObject派生类，使用var是可以的。但对于hasOpenSubMenu，明确使用bool类型或直接在条件中判断会更清晰。

2. 代码质量

硬编码与耦合:
- 在ComboBox.qml的delegate中直接引用arrowListView（即control.contentItem）的id构成了紧耦合。更好的做法是在T.ComboBox中定义一个property bool isHovered，然后在ArrowListView中绑定这个属性，或者通过信号传递。
- 不过，考虑到这是模板（Template）的实现，内部耦合在一定程度上是可以接受的，只要外部接口保持整洁。
变量声明:
- Menu.qml中的var currentItem和var hasOpenSubMenu。建议改为具体的类型（如果已知）或直接内联逻辑，减少不必要的临时变量，提高代码可读性。

3. 代码性能

绑定频率:
- ArrowListView.qml中的property bool hovered: hoverHandler.hovered是一个绑定。每当鼠标移动时，这个属性会更新。如果这个属性被大量其他属性绑定（例如每个delegate的highlighted状态），在列表项很多时可能会造成频繁的重绘。
- 建议: 目前的实现对于常规UI（几十项以内）性能影响微乎其微。但在极端情况下（如几百上千项），频繁的hovered变化传播给所有delegate可能会引起卡顿。目前的场景是ComboBox和Menu，通常项数不多，因此性能是可以接受的。
查找开销:
- Menu.qml中的control.itemAt(control.currentIndex)。这涉及一次模型查找。在onHoveredChanged中调用是合理的，因为它不是高频事件（只在进出时触发），不会造成性能问题。

4. 代码安全

空指针检查:
- Menu.qml中已经做了if (currentItem)检查，这是很好的习惯，防止了currentItem为空时的崩溃。
类型安全:
- 使用var降低了类型安全性，但在QML的JS环境中是常见的妥协。确保itemAt返回的对象确实有subMenu属性。

改进建议

优化变量声明 (Menu.qml):
建议简化onHoveredChanged中的逻辑，去掉中间变量，使代码更紧凑。

onHoveredChanged: {
    if (!hovered) {
        var currentItem = control.itemAt(control.currentIndex)
        // 只有当没有子菜单，或者子菜单不可见时，才清除索引
        if (!currentItem || !currentItem.subMenu || !currentItem.subMenu.visible) {
            control.currentIndex = -1
        }
    }
}

解耦 ComboBox.qml 中的依赖:
虽然直接访问id方便，但更健壮的做法是让ArrowListView通过属性向上通知，或者T.ComboBox向下传递状态。不过，为了保持diff的简洁性，如果当前结构稳定，可以保留现状，但建议添加注释说明依赖关系。
```
// 在 ComboBox.qml 的 contentItem: ArrowListView 中
// 确保 ArrowListView 暴露 hovered 属性供 delegate 使用
```
HoverHandler 目标范围:
检查itemsView和viewLayout是否真的覆盖了所有视觉区域。如果有padding或背景色在Layout之外，可能需要调整target。
导入模块:
确认import QtQuick.Controls是否必须。在Qt 6中，HoverHandler属于QtQuick。如果项目允许，尽量减少不必要的导入以减小编译后的二进制体积（尽管QML是解释/即时编译的，但减少依赖总是好的）。如果确实需要Controls中的其他类型，则保留。

总结

这段代码逻辑清晰，能够有效解决ComboBox和Menu在鼠标交互时的状态保持问题。主要改进点在于减少中间变量、注意组件间的耦合度以及确认HoverHandler的目标范围。代码安全性方面做得较好，有空指针检查。性能方面对于常规UI场景没有问题。

18202781743 · 2026-02-03T07:34:05Z

qt6/src/qml/ComboBox.qml

        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.highlightedIndex === index) && (arrowListView.hovered || (subMenu && subMenu.visible))


在外面不要访问contentItem里的arrowListView，因为contentItem可能被外面复写，

18202781743 · 2026-02-03T07:35:22Z

qt6/src/qml/ComboBox.qml

        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.highlightedIndex === index) && (arrowListView.hovered || (subMenu && subMenu.visible))


能不能只去判断MenuItem的hover呀，而不是通过外面的那个Popup有没有hover去判断，因为现在Popup可能存在margin，这样的话，在hover时，MenuItem也应该失去highlight的，不显示background，

18202781743 · 2026-02-03T07:37:33Z

qt6/src/qml/Menu.qml

+                    }
+
+                    if (!hasOpenSubMenu) {
+                        control.currentIndex = -1


能不修改currentIndex么，因为currentIndex是对外暴露的，外面可能监听了这个属性，如果我们改变了currentIndex，会导致调用方的业务逻辑发生变化，

deepin-bot · 2026-02-05T09:48:36Z

TAG Bot

New tag: 6.7.33
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #566

sourcery-ai bot reviewed Feb 2, 2026

View reviewed changes

18202781743 reviewed Feb 2, 2026

View reviewed changes

pengfeixx force-pushed the fix-306683 branch from 8c8735d to 9bae01f Compare February 2, 2026 07:54

pengfeixx force-pushed the fix-306683 branch from 9bae01f to fc8431a Compare February 2, 2026 07:55

18202781743 reviewed Feb 2, 2026

View reviewed changes

pengfeixx force-pushed the fix-306683 branch from fc8431a to 813e96c Compare February 2, 2026 08:57

pengfeixx force-pushed the fix-306683 branch from 813e96c to 57f56cc Compare February 2, 2026 09:03

18202781743 reviewed Feb 3, 2026

View reviewed changes

Conversation

pengfeixx commented Feb 2, 2026

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented Feb 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented Feb 2, 2026

deepin pr auto review

总体评价

详细审查

1. 语法与逻辑

2. 代码质量

3. 代码性能

4. 代码安全

改进建议

总结

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepin-bot bot commented Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants