-
Notifications
You must be signed in to change notification settings - Fork 33
fix: Fix abnormal animation for two-finger swipe #688
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds drag state tracking to the fullscreen list view to prevent unintended flick animations when a two-finger swipe begins, ensuring the content position stays in sync with the current index unless a real drag is in progress. Sequence diagram for two-finger swipe handling and flick cancellationsequenceDiagram
actor User
participant TouchSystem
participant FullscreenFrame
participant ListviewPage
User->>TouchSystem: two_finger_swipe_start
TouchSystem->>FullscreenFrame: touch_events
FullscreenFrame->>ListviewPage: process_touch_events
alt no_real_drag_in_progress
ListviewPage->>ListviewPage: onFlickStarted
ListviewPage->>ListviewPage: check isDragging == false
ListviewPage->>ListviewPage: cancelFlick()
ListviewPage->>ListviewPage: set contentX = currentIndex * width
else real_drag_in_progress
ListviewPage->>ListviewPage: onDragStarted
ListviewPage->>ListviewPage: set isDragging = true
ListviewPage->>ListviewPage: allow normal flick animation
end
User-->>TouchSystem: release_gesture
TouchSystem-->>FullscreenFrame: drag_end_event
FullscreenFrame->>ListviewPage: onDragEnded
ListviewPage->>ListviewPage: set isDragging = false
Updated class diagram for FullscreenFrame list view drag stateclassDiagram
class FullscreenFrame {
}
class Indicator {
int currentIndex
}
class ListviewPage {
bool interactive
int currentIndex
bool isDragging
int contentX
int width
void setCurrentIndex(index)
void onFlickStarted()
void onDragStarted()
void onDragEnded()
}
FullscreenFrame --> ListviewPage : contains
FullscreenFrame --> Indicator : contains
Indicator --> ListviewPage : syncs_currentIndex
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Consider using the built-in
ListViewdrag/flick state properties (e.g.,drag.activeorflicking) instead of maintaining a separateisDraggingflag to reduce the risk of state getting out of sync. - You may want to explicitly reset
isDraggingwhen the view becomes non-interactive or is reinitialized (e.g., oninteractivechanges or component destruction) to avoid a staletruestate after lifecycle changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using the built-in `ListView` drag/flick state properties (e.g., `drag.active` or `flicking`) instead of maintaining a separate `isDragging` flag to reduce the risk of state getting out of sync.
- You may want to explicitly reset `isDragging` when the view becomes non-interactive or is reinitialized (e.g., on `interactive` changes or component destruction) to avoid a stale `true` state after lifecycle changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fix abnormal animation for two-finger swipe Log: Fix abnormal animation for two-finger swipe pms: BUG-343525
815bc41 to
0073cd5
Compare
deepin pr auto review我来对这段代码的修改进行审查:
改进建议: property bool isDragging: false // 标记当前是否处于拖拽状态
onFlickStarted: {
// 如果不在拖拽状态,则取消快速滑动并重置到当前位置
if (!isDragging) {
cancelFlick()
// 确保contentX在有效范围内
contentX = Math.max(0, Math.min(currentIndex * width, contentWidth - width))
}
}
onDragStarted: {
isDragging = true
}
onMovementEnded: {
isDragging = false
}
function setCurrentIndex(index) {
// 确保索引在有效范围内
index = Math.max(0, Math.min(index, count - 1))
listviewPage.currentIndex = index
// 使用绑定保持与indicator同步
listviewPage.currentIndex = Qt.binding(function() { return indicator.currentIndex })
}主要改进:
这些改进可以提高代码的可维护性和健壮性,同时保持原有功能不变。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, 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 |
Fix abnormal animation for two-finger swipe
Log: Fix abnormal animation for two-finger swipe
pms: BUG-343525
Summary by Sourcery
Bug Fixes: