feat: add tap handler to hide launcher#658
Merged
Merged
Conversation
Reviewer's GuideThe PR refactors the search results grid by wrapping the existing GridViewContainer in an Item, disables its built-in interactivity to fix click-through issues, and adds a custom MouseArea that captures wheel events and manually adjusts the scrollbar position with proper bounds checking, all while preserving the original look and behavior. Sequence diagram for custom scroll interaction in search result gridsequenceDiagram
participant User as actor User
participant MouseArea
participant ScrollBar
participant GridViewContainer
User->>MouseArea: Wheel event
MouseArea->>ScrollBar: Check if visible
alt ScrollBar visible
MouseArea->>ScrollBar: Adjust position (with bounds)
end
Class diagram for updated search result grid structureclassDiagram
class Item {
}
class GridViewContainer {
- interactive: bool
- vScrollBar: ScrollBar
}
class MouseArea {
+ onWheel(wheel)
}
class ScrollBar {
- position: float
- size: float
- visible: bool
- active: bool
}
Item "1" *-- "1" GridViewContainer
Item "1" *-- "1" MouseArea
GridViewContainer "1" *-- "1" ScrollBar
MouseArea ..> ScrollBar: adjusts position
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 there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `qml/FullscreenFrame.qml:643-651` </location>
<code_context>
+ if (!searchResultGridViewContainer.vScrollBar.visible) return
+
+ let delta = wheel.angleDelta.y
+ let scrollStep = 0.1
+
+ if (delta > 0) {
+ searchScrollBar.position = Math.max(0, searchScrollBar.position - scrollStep)
+ } else if (delta < 0) {
</code_context>
<issue_to_address>
**suggestion:** Fixed scrollStep value may not scale well for different grid sizes.
Hardcoding scrollStep to 0.1 can cause inconsistent scrolling across different grid sizes. Dynamically calculate scrollStep based on grid dimensions or item count for more consistent behavior.
```suggestion
let delta = wheel.angleDelta.y
// Dynamically calculate scrollStep based on grid size
// For example, scroll by one visible item per wheel event
let gridView = searchResultGridViewContainer
let visibleItems = Math.max(1, Math.floor(gridView.height / gridView.cellHeight))
let totalItems = gridView.count > 0 ? gridView.count : 1
let scrollStep = visibleItems / totalItems
if (delta > 0) {
searchScrollBar.position = Math.max(0, searchScrollBar.position - scrollStep)
} else if (delta < 0) {
let maxPosition = 1.0 - searchScrollBar.size
searchScrollBar.position = Math.min(maxPosition, searchScrollBar.position + scrollStep)
}
```
</issue_to_address>
### Comment 2
<location> `qml/FullscreenFrame.qml:649-650` </location>
<code_context>
+ if (delta > 0) {
+ searchScrollBar.position = Math.max(0, searchScrollBar.position - scrollStep)
+ } else if (delta < 0) {
+ let maxPosition = 1.0 - searchScrollBar.size
+ searchScrollBar.position = Math.min(maxPosition, searchScrollBar.position + scrollStep)
+ }
+ }
</code_context>
<issue_to_address>
**issue:** Potential edge case if searchScrollBar.size is undefined or zero.
If searchScrollBar.size is missing or zero, maxPosition may be incorrect, causing the scrollbar to exceed its intended range. Please validate that searchScrollBar.size is always set to a positive value when the scrollbar is shown.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+643
to
+651
| let delta = wheel.angleDelta.y | ||
| let scrollStep = 0.1 | ||
|
|
||
| if (delta > 0) { | ||
| searchScrollBar.position = Math.max(0, searchScrollBar.position - scrollStep) | ||
| } else if (delta < 0) { | ||
| let maxPosition = 1.0 - searchScrollBar.size | ||
| searchScrollBar.position = Math.min(maxPosition, searchScrollBar.position + scrollStep) | ||
| } |
There was a problem hiding this comment.
suggestion: Fixed scrollStep value may not scale well for different grid sizes.
Hardcoding scrollStep to 0.1 can cause inconsistent scrolling across different grid sizes. Dynamically calculate scrollStep based on grid dimensions or item count for more consistent behavior.
Suggested change
| let delta = wheel.angleDelta.y | |
| let scrollStep = 0.1 | |
| if (delta > 0) { | |
| searchScrollBar.position = Math.max(0, searchScrollBar.position - scrollStep) | |
| } else if (delta < 0) { | |
| let maxPosition = 1.0 - searchScrollBar.size | |
| searchScrollBar.position = Math.min(maxPosition, searchScrollBar.position + scrollStep) | |
| } | |
| let delta = wheel.angleDelta.y | |
| // Dynamically calculate scrollStep based on grid size | |
| // For example, scroll by one visible item per wheel event | |
| let gridView = searchResultGridViewContainer | |
| let visibleItems = Math.max(1, Math.floor(gridView.height / gridView.cellHeight)) | |
| let totalItems = gridView.count > 0 ? gridView.count : 1 | |
| let scrollStep = visibleItems / totalItems | |
| if (delta > 0) { | |
| searchScrollBar.position = Math.max(0, searchScrollBar.position - scrollStep) | |
| } else if (delta < 0) { | |
| let maxPosition = 1.0 - searchScrollBar.size | |
| searchScrollBar.position = Math.min(maxPosition, searchScrollBar.position + scrollStep) | |
| } |
Comment on lines
+649
to
+650
| let maxPosition = 1.0 - searchScrollBar.size | ||
| searchScrollBar.position = Math.min(maxPosition, searchScrollBar.position + scrollStep) |
There was a problem hiding this comment.
issue: Potential edge case if searchScrollBar.size is undefined or zero.
If searchScrollBar.size is missing or zero, maxPosition may be incorrect, causing the scrollbar to exceed its intended range. Please validate that searchScrollBar.size is always set to a positive value when the scrollbar is shown.
BLumia
reviewed
Oct 11, 2025
wjyrich
force-pushed
the
fix-bug-324421
branch
from
9823eee to
d90aa74
Compare
1. Add TapHandler to GridViewContainer to hide launcher when tapped 2. This provides better user experience by allowing users to dismiss the launcher with a simple tap 3. The handler sets LauncherController.visible to false when any area within the grid view is tapped 4. Improves navigation flow by giving users intuitive way to close the launcher interface feat: 添加点击处理器以隐藏启动器 1. 在 GridViewContainer 中添加 TapHandler 以便点击时隐藏启动器 2. 通过允许用户简单点击即可关闭启动器来提供更好的用户体验 3. 当网格视图内的任何区域被点击时，处理器将 LauncherController.visible 设置为 false 4. 通过为用户提供直观的关闭启动器界面方式来改进导航流程 PMS: BUG-324421
wjyrich
force-pushed
the
fix-bug-324421
branch
from
d90aa74 to
1648006
Compare
deepin pr auto review我对这段代码进行审查，并提出以下改进意见：
改进建议： TapHandler {
onTapped: {
// 添加防护措施，确保LauncherController存在
if (typeof(LauncherController) !== 'undefined' && LauncherController) {
// 只有在当前可见时才执行隐藏操作，避免不必要的操作
if (LauncherController.visible) {
LauncherController.visible = false;
}
}
}
}或者，如果项目使用了Qt 5.15+，可以考虑使用更现代的事件处理方式： TapHandler {
target: parent
onTapped: {
if (LauncherController?.visible) {
LauncherController.visible = false;
}
}
}这些改进可以增加代码的健壮性，避免潜在的错误，同时保持原有的功能不变。 |
wjyrich
changed the title
18202781743
approved these changes
Oct 11, 2025
| interactive: false | ||
| vScrollBar: ScrollBar { | ||
| id: searchScrollBar | ||
| visible: parent.model.count > 4 * 8 |
Contributor
There was a problem hiding this comment.
这样处理后，控制中心设置的滚动条策略是不是有问题了，
| active: parent.model.count > 4 * 8 | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
可以直接给GridView挂一个TapHandler，来接收点击事件吧，
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, wjyrich 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 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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
4 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.
feat: add tap handler to hide launcher
launcher with a simple tap
within the grid view is tapped
launcher interface
feat: 添加点击处理器以隐藏启动器
设置为 false
PMS: BUG-324421