fix: Handle exceptions when hovering over up/down arrows #565
Conversation
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
log: Modify arrow scrolling logic to adjust only contentY (scroll by step size in pixels), completely bypassing currentIndex. Also add single-step scrolling when clicking arrows. pms: bug-335545
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the ArrowListView button scrolling to operate directly on contentY using a configurable pixel step size (item height), adds single-step scrolling on click, and wires the new step size through ArrowListView, reducing reliance on currentIndex-based scrolling and improving hover/arrow behavior. Sequence diagram for hover and click scrolling behavior in ArrowListViewButtonsequenceDiagram
actor User
participant ArrowListViewButton
participant ActionButton
participant itemsView
User->>ArrowListViewButton: hover over up/down arrow
ArrowListViewButton->>ActionButton: set hovered = true
ActionButton->>ActionButton: shouldAutoScroll = hovered && enabled
ActionButton->>ActionButton: start initialDelayTimer (300ms)
initialDelayTimer-->>ActionButton: onTriggered
ActionButton->>ActionButton: delayCompleted = true
ActionButton->>hoverScrollTimer: start (interval 100ms)
loop while hovered and enabled
hoverScrollTimer-->>ActionButton: onTriggered
ActionButton->>ActionButton: performScroll()
ActionButton->>itemsView: compute maxY, step, nextY
ActionButton->>itemsView: set contentY = clamp(nextY, 0, maxY)
end
User-->>ArrowListViewButton: stop hover / disable button
ArrowListViewButton->>ActionButton: shouldAutoScroll = false
ActionButton->>ActionButton: delayCompleted = false
User->>ActionButton: click up/down arrow
ActionButton->>ActionButton: onClicked
ActionButton->>ActionButton: performScroll()
ActionButton->>itemsView: set contentY = clamp(contentY ± step, 0, maxY)
Updated class diagram for ArrowListView and ArrowListViewButton QML componentsclassDiagram
class ArrowListViewButton {
+Item view
+int direction
+int stepSize
+bool active
+bool shouldAutoScroll
+bool delayCompleted
+performScroll()
+onShouldAutoScrollChanged()
+onClicked()
}
class ActionButton {
+bool hovered
+bool enabled
+iconWidth
+iconHeight
+onClicked()
}
class InitialDelayTimer {
+int interval = 300
+bool repeat = false
+start()
+stop()
+onTriggered()
}
class HoverScrollTimer {
+int interval = 100
+bool repeat = true
+start()
+stop()
+onTriggered()
}
class ArrowListView {
+Item itemsView
+Item control
+int itemHeight
}
class UpArrowButton {
+int direction
+int stepSize
}
class DownArrowButton {
+int direction
+int stepSize
}
ArrowListViewButton o-- ActionButton
ArrowListViewButton o-- InitialDelayTimer
ArrowListViewButton o-- HoverScrollTimer
ArrowListView o-- ArrowListViewButton : uses
ArrowListView o-- UpArrowButton
ArrowListView o-- DownArrowButton
UpArrowButton <|-- ArrowListViewButton
DownArrowButton <|-- ArrowListViewButton
ArrowListViewButton --> ArrowListView : view
ArrowListView --> ArrowListViewButton : stepSize=itemHeight
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}
Contributor
deepin pr auto review代码审查意见我已对提供的 1. 语法逻辑方面问题1：performScroll()函数中的边界检查逻辑function performScroll() {
if (!view)
return
var maxY = Math.max(0, view.contentHeight - view.height)
if (maxY <= 0)
return
var step = Math.max(1, stepSize)
var nextY = direction === ArrowListViewButton.UpButton ? (view.contentY - step)
: (view.contentY + step)
view.contentY = Math.max(0, Math.min(maxY, nextY))
}改进建议：
问题2：stepSize属性定义property int stepSize: DS.Style.arrowListView.itemHeight改进建议：
property int stepSize: DS.Style.arrowListView && DS.Style.arrowListView.itemHeight || 12. 代码质量方面问题1：代码注释
问题2：代码一致性
问题3：错误处理
function performScroll() {
if (!view || typeof view.contentY === 'undefined' || typeof view.contentHeight === 'undefined')
return
// 其余逻辑...
}3. 代码性能方面问题1：定时器管理Timer {
id: initialDelayTimer
interval: 300
running: shouldAutoScroll && !delayCompleted
onTriggered: delayCompleted = true
}
Timer {
id: hoverScrollTimer
interval: 100
running: shouldAutoScroll && delayCompleted
onTriggered: performScroll()
}改进建议：
问题2：滚动计算
property real maxY: Math.max(0, view.contentHeight - view.height)
Connections {
target: view
function onContentHeightChanged() { maxY = Math.max(0, view.contentHeight - view.height) }
function onHeightChanged() { maxY = Math.max(0, view.contentHeight - view.height) }
}4. 代码安全方面问题1：属性验证
property int stepSize: {
var size = DS.Style.arrowListView && DS.Style.arrowListView.itemHeight || 1
return Math.max(1, Math.min(size, 100)) // 限制最大步长为100
}问题2：边界值处理
function performScroll() {
if (!view || !view.flickable)
return
// 使用flickable的API而不是直接设置contentY
if (direction === ArrowListViewButton.UpButton) {
view.flickable.flick(0, stepSize)
} else {
view.flickable.flick(0, -stepSize)
}
}综合改进建议代码// ArrowListViewButton.qml 改进版
Loader {
required property Item view
property int direction
property int stepSize: {
var size = DS.Style.arrowListView && DS.Style.arrowListView.itemHeight || 1
return Math.max(1, Math.min(size, 100))
}
active: view && view.interactive
// 缓存最大滚动位置
property real maxY: view ? Math.max(0, view.contentHeight - view.height) : 0
Connections {
target: view
function onContentHeightChanged() { maxY = Math.max(0, view.contentHeight - view.height) }
function onHeightChanged() { maxY = Math.max(0, view.contentHeight - view.height) }
}
sourceComponent: ActionButton {
// 原有属性...
// 执行滚动操作，带有边界检查
function performScroll() {
if (!view || typeof view.contentY === 'undefined' || maxY <= 0)
return
var step = Math.max(1, stepSize)
var nextY = direction === ArrowListViewButton.UpButton
? (view.contentY - step)
: (view.contentY + step)
// 使用flickable的API进行更安全的滚动
if (view.flickable) {
if (direction === ArrowListViewButton.UpButton) {
view.flickable.flick(0, step)
} else {
view.flickable.flick(0, -step)
}
} else {
// 回退方案：直接设置contentY
view.contentY = Math.max(0, Math.min(maxY, nextY))
}
}
// 自动滚动控制
property bool shouldAutoScroll: hovered && enabled
property bool delayCompleted: false
Timer {
id: initialDelayTimer
interval: 300
running: shouldAutoScroll && !delayCompleted
onTriggered: delayCompleted = true
}
Timer {
id: hoverScrollTimer
interval: 100
running: shouldAutoScroll && delayCompleted
repeat: true
onTriggered: performScroll()
}
onShouldAutoScrollChanged: {
if (!shouldAutoScroll)
delayCompleted = false
}
onClicked: performScroll()
}
}这些改进建议旨在提高代码的健壮性、可维护性和性能，同时增强错误处理和边界条件检查。 |
18202781743
approved these changes
Feb 5, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, JWWTSL 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 |
Contributor
|
This pr force merged! (status: unstable) |
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.
log: Modify arrow scrolling logic to adjust only contentY (scroll by step size in pixels), completely bypassing currentIndex. Also add single-step scrolling when clicking arrows.
pms: bug-335545
Summary by Sourcery
Adjust arrow list view button scrolling to operate on view content offset with pixel step size and support click-based single-step scrolling.
Bug Fixes:
Enhancements: