refactor: restructure search item event handling and loader#315
Conversation
1. Move Timer, HoverHandler, and MouseArea from inside Image to direct children of AppletItem for better component hierarchy 2. Merge TapHandler (left click) and MouseArea (right click) into a single MouseArea handling both buttons via onPressed and onClicked 3. Change platformMenuLoader active from false to true to load context menu eagerly (previously lazy loaded) 4. Remove TapHandler and right-click MouseArea, simplifying event handling logic Log: Changed context menu loading strategy from lazy to eager; combined left and right click handlers Influence: 1. Test left click on search icon triggers Grand Search toggle 2. Test right click on search icon opens context menu 3. Verify tooltip appears on hover and disappears on unhover 4. Verify context menu items (if any) function correctly 5. Test quick successive clicks (left/right) do not cause unexpected behavior 6. Verify search icon still displays correctly and scales as expected refactor: 重构搜索项事件处理和加载器 1. 将Timer、HoverHandler和MouseArea从Image内部移至AppletItem的直接子级, 优化组件层次结构 2. 将左键点击的TapHandler和右键点击的MouseArea合并为单个MouseArea,通过 onPressed和onClicked处理两种按钮 3. 将platformMenuLoader的active从false改为true,使上下文菜单从惰性加载变 为主动加载 4. 移除TapHandler和独立的右键MouseArea,简化事件处理逻辑 Log: 将菜单加载策略从惰性改为主动;合并左右键点击处理器 Influence: 1. 测试左键点击搜索图标触发全局搜索切换 2. 测试右键点击搜索图标弹出上下文菜单 3. 验证悬停时显示提示,移出后提示消失 4. 验证上下文菜单项(如果有)功能正常 5. 测试快速连续点击(左/右键)不导致意外行为 6. 验证搜索图标正常显示且缩放符合预期 BUG: https://pms.uniontech.com/bug-view-328025.html
Reviewer's GuideRefactors the search item QML to move interaction handlers out of the Image into the AppletItem, merge left/right click handling into a single MouseArea, and eagerly load the context menu via an always-active Loader, while preserving tooltip and search toggle behavior. Sequence diagram for unified search item mouse interactionssequenceDiagram
actor User
participant MouseArea
participant Applet
participant platformMenuLoader
participant toolTipShowTimer
participant toolTip
rect rgb(230,230,255)
User->>MouseArea: hover enter
MouseArea->>toolTipShowTimer: start()
toolTipShowTimer->>toolTip: open()
end
rect rgb(230,255,230)
User->>MouseArea: hover leave
MouseArea->>toolTipShowTimer: stop()
MouseArea->>toolTip: close()
end
rect rgb(255,230,230)
alt left click
User->>MouseArea: onClicked(Qt.LeftButton)
MouseArea->>Applet: toggleGrandSearch()
MouseArea->>toolTip: close()
else right click
User->>MouseArea: onPressed(Qt.RightButton)
MouseArea->>platformMenuLoader: item.open()
end
end
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 found 1 issue, and left some high level feedback:
- By moving MouseArea and HoverHandler from the Image to the AppletItem and using
anchors.fill: parent, left/right clicks and hover now apply to the entire applet instead of just the icon; if this wider hit area is not intentional, consider anchoring to the Image or a dedicated wrapper. - When calling
platformMenuLoader.item.open()inonPressed, consider guarding with aplatformMenuLoader.status === Loader.Ready(or a null check onitem) to avoid potential issues if the loader isn’t fully initialized at the moment of the first right-click.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- By moving MouseArea and HoverHandler from the Image to the AppletItem and using `anchors.fill: parent`, left/right clicks and hover now apply to the entire applet instead of just the icon; if this wider hit area is not intentional, consider anchoring to the Image or a dedicated wrapper.
- When calling `platformMenuLoader.item.open()` in `onPressed`, consider guarding with a `platformMenuLoader.status === Loader.Ready` (or a null check on `item`) to avoid potential issues if the loader isn’t fully initialized at the moment of the first right-click.
## Individual Comments
### Comment 1
<location path="src/grand-search-shell-plugin/package/searchitem.qml" line_range="79-70" />
<code_context>
+ acceptedButtons: Qt.LeftButton | Qt.RightButton
+ onPressed: function(mouse) {
+ if (mouse.button === Qt.RightButton) {
+ mouse.accepted = true
+ platformMenuLoader.item.open()
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against `platformMenuLoader.item` being null before calling `open()`.
`active: true` doesn’t fully guarantee `platformMenuLoader.item` is available or that the load succeeded. Please guard the call with a null/status check (e.g. `if (platformMenuLoader.item)` or `platformMenuLoader.status === Loader.Ready`) before calling `open()` to avoid potential runtime errors.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| toolTipShowTimer.stop() | ||
| } | ||
|
|
||
| toolTip.close() |
There was a problem hiding this comment.
issue (bug_risk): Guard against platformMenuLoader.item being null before calling open().
active: true doesn’t fully guarantee platformMenuLoader.item is available or that the load succeeded. Please guard the call with a null/status check (e.g. if (platformMenuLoader.item) or platformMenuLoader.status === Loader.Ready) before calling open() to avoid potential runtime errors.
Fix the toggleGrandSearch function to switch visibility based on current state instead of always setting it to true. Add D-Bus signal connection for VisibleChanged to keep internal state synchronized with external events. Log: Changed Grand Search toggle behavior from always open to switchable Influence: 1. Test clicking the Grand Search toggle button repeatedly and verify visibility toggles correctly 2. Verify the D-Bus VisibleChanged signal is received and updates internal state 3. Test with other components that control Grand Search visibility to ensure consistency 4. Check that no regression occurs for the config toggle function fix: toggleGrandSearch 现在切换可见性 修复 toggleGrandSearch 函数,使其基于当前状态切换可见性,而不是始终设置 为 true。添加对 D-Bus VisibleChanged 信号的连接,确保内部状态与外部事件 同步。 Log: 修改 Grand Search 切换行为,从始终打开变为可切换 Influence: 1. 反复点击 Grand Search 切换按钮,验证可见性正确切换 2. 验证 D-Bus VisibleChanged 信号被接收并更新内部状态 3. 与其他控制 Grand Search 可见性的组件协同测试,确保一致性 4. 检查配置切换功能没有出现回归问题 BUG: https://pms.uniontech.com/bug-view-303917.html
deepin pr auto review你好!我是CodeGeeX,你的智能编程助手。我已经仔细审查了你提供的 Git Diff。 这次代码变更主要对 DDE Dock 上的全局搜索插件进行了重构和功能增强,包括:版权年份更新、QML 事件处理逻辑的合并与重构、右键菜单加载策略的更改、C++ 端 D-Bus 信号监听的引入,以及搜索显隐逻辑的修复。 以下是针对语法逻辑、代码质量、代码性能和代码安全四个方面的详细审查意见及改进建议: 一、 语法与逻辑
二、 代码质量
三、 代码性能
四、 代码安全
总结最核心需要修改的是:
希望这些审查意见对你有所帮助!如果有任何疑问,欢迎随时提问。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Johnson-zs, Kakueeen 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: blocked) |
children of AppletItem for better component hierarchy
single MouseArea handling both buttons via onPressed and onClicked
menu eagerly (previously lazy loaded)
handling logic
Log: Changed context menu loading strategy from lazy to eager; combined
left and right click handlers
Influence:
behavior
refactor: 重构搜索项事件处理和加载器
优化组件层次结构
onPressed和onClicked处理两种按钮
为主动加载
Log: 将菜单加载策略从惰性改为主动;合并左右键点击处理器
Influence:
BUG: https://pms.uniontech.com/bug-view-328025.html
Summary by Sourcery
Simplify search item event handling and adjust context menu loading behavior.
Enhancements: