fix: avoid icon overlap caused by ListView move and displace animation#1498
fix: avoid icon overlap caused by ListView move and displace animation#1498BLumia merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideRefactors the TaskManager ListView item animation strategy to avoid icon overlap by removing ListView displaced/move transitions and instead driving horizontal animation via an external wrapper Rectangle bound to delegate geometry, plus minor z-order and logging tweaks for drag-and-drop. Sequence diagram for updated dock icon drag-reorder animation without ListView displaced transitionssequenceDiagram
actor User
participant DockTaskManager
participant LauncherDndDropArea
participant AppListView
participant VisualModel
participant DelegateRoot
participant DelegateWrapperRectangle
participant AppItem
User->>AppItem: start drag
AppItem->>LauncherDndDropArea: dragEntered
User->>LauncherDndDropArea: drag move
LauncherDndDropArea->>LauncherDndDropArea: onPositionChanged(drag)
LauncherDndDropArea->>AppListView: indexAt(drag.x, drag.y)
LauncherDndDropArea->>DockTaskManager: findAppIndex(appId)
LauncherDndDropArea->>VisualModel: items.move(currentIndex, targetIndex)
VisualModel->>AppListView: update delegate order
AppListView->>DelegateRoot: update x, y, width, height, scale
DelegateRoot->>DelegateWrapperRectangle: propagate x, y, width, height, scale via bindings
DelegateWrapperRectangle->>DelegateWrapperRectangle: Behavior on x NumberAnimation
DelegateWrapperRectangle->>AppItem: keep anchors.fill parent
User->>LauncherDndDropArea: drop
LauncherDndDropArea->>LauncherDndDropArea: onDropped(drop)
LauncherDndDropArea->>VisualModel: items.move(currentIndex, targetIndex) if needed
VisualModel->>AppListView: finalize delegate positions
DelegateWrapperRectangle->>DelegateWrapperRectangle: finish x animation
Updated class diagram for TaskManager ListView delegate animation structureclassDiagram
class TaskManagerListView {
+int count
+int indexAt(int x, int y)
+VisualModel visualModel
-Transition moveDisplacedRemoved
-Transition addDisplacedRemoved
-Transition removeDisplacedRemoved
}
class VisualModel {
+list items
+void move(int fromIndex, int toIndex)
}
class DelegateRoot {
+int visualIndex
+var modelIndex
+real x
+real y
+real width
+real height
+real scale
+bool active
+bool attention
+string itemId
+string name
+string iconName
+var menus
+var windows
+real blendOpacity
+string title
+bool enableTitle
+int appTitleSpacing
}
class DelegateWrapperRectangle {
+real x
+real y
+real width
+real height
+real scale
+string color
+Behavior xBehavior
}
class Behavior {
+NumberAnimation animation
}
class NumberAnimation {
+int duration
+string easingType
}
class AppItem {
+string displayMode
+string colorTheme
+bool active
+bool attention
+string itemId
+string name
+string iconName
+var menus
+var windows
+int visualIndex
+var modelIndex
+real blendOpacity
+string title
+bool enableTitle
+int appTitleSpacing
+bool ListView_delayRemove
+onDragFinished()
}
class LauncherDndDropArea {
+int z
+string launcherDndDesktopId
+string launcherDndDragSource
+onPositionChanged(var drag)
+onDropped(var drop)
}
TaskManagerListView o-- VisualModel
VisualModel o-- DelegateRoot
DelegateRoot o-- DelegateWrapperRectangle
DelegateWrapperRectangle *-- AppItem
DelegateWrapperRectangle *-- Behavior
Behavior *-- NumberAnimation
TaskManagerListView .. LauncherDndDropArea
LauncherDndDropArea .. VisualModel
LauncherDndDropArea .. TaskManagerListView
DelegateRoot .. AppItem
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:
- The added
console.logstatements in the drag handlers look like temporary debugging output; consider removing them or guarding them behind a debug flag before merging. - The new
Behavior on xonly animates horizontal movement; if this view can ever be vertical or reflow vertically, you may also want to animateyto avoid sudden jumps during layout changes. - Now that the delegate
Rectangleis parented directly toappContainer, it might be safer to extract the animation duration/easing into a shared constant or property so it stays consistent with any other list or removal animations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The added `console.log` statements in the drag handlers look like temporary debugging output; consider removing them or guarding them behind a debug flag before merging.
- The new `Behavior on x` only animates horizontal movement; if this view can ever be vertical or reflow vertically, you may also want to animate `y` to avoid sudden jumps during layout changes.
- Now that the delegate `Rectangle` is parented directly to `appContainer`, it might be safer to extract the animation duration/easing into a shared constant or property so it stays consistent with any other list or removal animations.
## Individual Comments
### Comment 1
<location path="panels/dock/taskmanager/package/TaskManager.qml" line_range="217-222" />
<code_context>
}
onPositionChanged: function(drag) {
+ console.log("qml: launcherDndDesktopId " + launcherDndDesktopId);
if (launcherDndDesktopId === "") return
let targetIndex = appContainer.indexAt(drag.x, drag.y)
let appId = taskmanager.Applet.desktopIdToAppId(launcherDndDesktopId)
let currentIndex = taskmanager.Applet.windowSplit ? taskmanager.findAppIndexByWindow(appId, launcherDndWinId) : taskmanager.findAppIndex(appId)
+ console.log("qml: onPositionChanged " + currentIndex + " " + targetIndex);
if (currentIndex !== -1 && targetIndex !== -1 && currentIndex !== targetIndex) {
visualModel.items.move(currentIndex, targetIndex)
</code_context>
<issue_to_address>
**issue (performance):** Unconditional console.log calls in drag handlers can be noisy and impact performance.
Since these fire on every drag update, they can flood the console and degrade responsiveness with fast pointer movement. If they’re only for debugging, please either guard them behind a debug flag or remove them before merging to keep runtime logging lightweight.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
TAG Bot New tag: 2.0.32 |
避免由于 ListView 动画时机问题导致图标相互遮挡的问题. 这个问题是 QTBUG-133953 导致的,官方提供的绕过方案实际不能解决我们场景 所对应的问题(只能解决或者说绕过添加时位置不对的情况). 此修复方式尽可能 规避使用 displaced 动画来对未被移除的图标展示位置变化动画,以使ListView 自身完全不需要关心自身内容的坐标动画(ListView只负责显示被移除元素的消 失动画,即scale缩小并淡出. PMS: BUG-351826, BUG-308927 Log:
aa6385c to
6ea3b2a
Compare
deepin pr auto review这份代码修改主要涉及 QML 中任务栏管理器(TaskManager)的动画实现方式,从使用内置的 1. 语法逻辑审查
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
总结与改进建议
总体而言,这次修改将动画控制从视图层(ListView Transition)下沉到了代理层(Delegate Behavior),提供了更细粒度的控制,但也增加了维护布局同步逻辑的责任。代码本身没有严重错误,但需进行充分的边界测试。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
避免由于 ListView 动画时机问题导致图标相互遮挡的问题.
这个问题是 QTBUG-133953 导致的,官方提供的绕过方案实际不能解决我们场景
所对应的问题(只能解决或者说绕过添加时位置不对的情况). 此修复方式尽可能
规避使用 displaced 动画来对未被移除的图标展示位置变化动画,以使ListView
自身完全不需要关心自身内容的坐标动画(ListView只负责显示被移除元素的消
失动画,即scale缩小并淡出.
PMS: BUG-351826, BUG-308927
Log:
Summary by Sourcery
Prevent dock task manager icons from overlapping during ListView reordering animations by shifting position animations out of ListView’s displaced transitions and into the delegates themselves.
Bug Fixes:
Enhancements: