Skip to content

fix: avoid icon overlap caused by ListView move and displace animation#1498

Merged
BLumia merged 1 commit intolinuxdeepin:masterfrom
BLumia:pms-308927-351825
Mar 14, 2026
Merged

fix: avoid icon overlap caused by ListView move and displace animation#1498
BLumia merged 1 commit intolinuxdeepin:masterfrom
BLumia:pms-308927-351825

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Mar 12, 2026

避免由于 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:

  • Resolve icon overlap and incorrect positioning when rearranging dock items caused by ListView displaced animations timing issues.

Enhancements:

  • Animate app item horizontal movement via delegate-level behavior instead of ListView displaced transitions to decouple layout from content animations.
  • Adjust drag-and-drop drop area stacking and logging to better track and handle item reordering interactions.

@BLumia BLumia requested a review from wjyrich March 12, 2026 12:41
@sourcery-ai
Copy link

sourcery-ai bot commented Mar 12, 2026

Reviewer's Guide

Refactors 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 transitions

sequenceDiagram
    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
Loading

Updated class diagram for TaskManager ListView delegate animation structure

classDiagram
    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
Loading

File-Level Changes

Change Details Files
Remove ListView displaced/move transitions so the view only handles removal fade/scale animations, avoiding overlap caused by QTBUG-133953.
  • Deleted onOptimalSingleTextWidthChanged handler that temporarily disables and re-enables displaced transitions.
  • Removed moveDisplaced, addDisplaced, and removeDisplaced Transition definitions and the ListView.move binding to moveDisplaced.
panels/dock/taskmanager/package/TaskManager.qml
Introduce a wrapper Rectangle that mirrors delegate geometry and handles x-position animation, with AppItem moved inside it.
  • Replaced direct AppItem delegate with a transparent Rectangle parented to appContainer whose x, y, width, height, and scale follow delegateRoot.
  • Added Behavior on x with a NumberAnimation to animate horizontal position changes of each item independently of ListView displaced transitions.
  • Nested AppItem inside the Rectangle, preserving prior bindings to delegateRoot properties and drag/drop wiring including ListView.delayRemove and dropFilesOnItem connection.
panels/dock/taskmanager/package/TaskManager.qml
Adjust DnD overlay and add debug logging for drag reordering diagnostics.
  • Set launcherDndDropArea.z to 3 so the drag/drop area sits above other content during operations.
  • Added console.log statements in onPositionChanged and onDropped to log launcherDndDesktopId, currentIndex, and targetIndex for debugging reordering behavior.
panels/dock/taskmanager/package/TaskManager.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-bot
Copy link

deepin-bot bot commented Mar 12, 2026

TAG Bot

New tag: 2.0.32
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1499

避免由于 ListView 动画时机问题导致图标相互遮挡的问题.

这个问题是 QTBUG-133953 导致的,官方提供的绕过方案实际不能解决我们场景
所对应的问题(只能解决或者说绕过添加时位置不对的情况). 此修复方式尽可能
规避使用 displaced 动画来对未被移除的图标展示位置变化动画,以使ListView
自身完全不需要关心自身内容的坐标动画(ListView只负责显示被移除元素的消
失动画,即scale缩小并淡出.

PMS: BUG-351826, BUG-308927
Log:
@BLumia BLumia force-pushed the pms-308927-351825 branch from aa6385c to 6ea3b2a Compare March 13, 2026 01:45
@deepin-ci-robot
Copy link

deepin pr auto review

这份代码修改主要涉及 QML 中任务栏管理器(TaskManager)的动画实现方式,从使用内置的 Transition 改为了显式的 Behavior。以下是对该 diff 的详细审查和改进建议:

1. 语法逻辑审查

  • 逻辑变更分析

    • 原代码:使用了 Qt Quick 提供的 moveDisplaced, addDisplaced, removeDisplaced 以及 move 属性来处理 ListView 中项目的移动和增删动画。这种方式是声明式的,由 ListView 的布局引擎自动触发。
    • 新代码:移除了上述 Transition 定义,转而在 Delegate 的根元素 Rectangle 上为 xy 属性添加了 Behavior。这意味着动画现在是由属性值的变化直接驱动的。
    • 结论:逻辑上是通顺的。通过 Behavior 可以更精细地控制动画,且在处理复杂的自定义布局或非标准布局(如非线性的网格布局)时,Behavior 往往比 Transition 更灵活。这里将 Rectangle 作为容器包裹 AppItem,并利用 Behavior 驱动其位置变化,实现了与原 Transition 类似的视觉效果。
  • 潜在问题 - 属性绑定

    • 新代码中 Rectangle 的位置属性直接绑定到了 delegateRoot
      x: delegateRoot.x
      y: delegateRoot.y
    • delegateRootLoader(或者类似的容器),其位置由 ListView 的布局逻辑控制。
    • 风险点:当 ListView 的布局发生改变(例如插入、删除、移动项目)时,delegateRootx/y 会瞬间改变。虽然 Behavior 会捕获这个变化并播放动画,但在某些极端情况下,如果 ListView 的布局逻辑频繁触发,可能会导致动画队列堆积或视觉上的抖动。原生的 Transition 机制通常能更好地处理这种布局冲突,因为它与视图的布局周期是同步的。

2. 代码质量审查

  • 调试代码残留

    • 代码中包含注释掉的调试代码:
      // kept for debug purpose
      // border.color: "red"
      // border.width: 1
    • 建议:在提交到生产代码库前,应该删除这些注释掉的调试代码,保持代码整洁。
  • 代码结构

    • 引入了一层额外的 Rectangle 包裹。虽然这有助于分离动画逻辑和业务逻辑(AppItem),但也增加了一层元素嵌套。
    • 建议:如果 AppItem 本身不需要复杂的背景绘制,可以考虑直接在 AppItem 上应用 Behavior,或者确认 Rectangle 是否必须存在(例如用于捕获特定区域的事件)。如果仅仅为了动画,这种嵌套是可以接受的,但需要确保不会因为层级过深影响渲染性能。
  • 代码删除

    • onOptimalSingleTextWidthChanged 中的逻辑被完全删除了。这段逻辑原本用于在文本宽度变化时临时禁用 addDisplacedremoveDisplaced 动画。
    • 风险点:删除这段代码意味着现在文本宽度变化时,Behavior 依然会触发 x/y 的动画。如果文本宽度变化导致项目重新布局(例如宽度变化引起换行或挤压),这可能会导致不必要的动画播放。需要确认这种变化是否是预期的行为。

3. 代码性能审查

  • 渲染性能

    • BehaviorTransition 在底层实现上通常都使用动画线程,性能开销差异不大。
    • 优化点:新代码使用了 Easing.OutCubic,原代码使用了 Easing.OutQuadOutCubic 的曲线更平滑,但计算量略大(虽然微乎其微)。这不是性能问题,而是视觉风格的选择。
  • 布局计算

    • 显式的 Behavior 可能会在布局频繁变化时(例如快速拖拽、快速添加/删除窗口)导致更多的 CPU 开销,因为每次属性变化都会触发动画评估。原生的 Transition 有时会对布局变化进行合并优化。
    • 建议:关注在快速操作场景下的帧率表现。

4. 代码安全审查

  • 安全性:此部分主要涉及 UI 交互,没有明显的安全漏洞(如注入攻击、权限提升等)。

总结与改进建议

  1. 清理代码:请删除 Rectangle 中注释掉的 border 调试代码。
  2. 确认动画逻辑:务必测试在快速连续操作(如快速打开/关闭多个窗口)下,新实现的 Behavior 是否能像之前的 Transition 一样流畅,避免出现布局错乱或动画“打架”的情况。
  3. 文本宽度变化的影响:由于删除了 onOptimalSingleTextWidthChanged 中的动画抑制逻辑,建议专门测试一下当应用标题文字长度频繁变化时,任务栏图标是否会出现不必要的抖动或频繁的位移动画。
  4. Z轴层级:修改中增加了 z: 3DropArea。这是一个好的改动,确保拖放区域始终在最上层,防止被其他元素遮挡导致拖放失效。

总体而言,这次修改将动画控制从视图层(ListView Transition)下沉到了代理层(Delegate Behavior),提供了更细粒度的控制,但也增加了维护布局同步逻辑的责任。代码本身没有严重错误,但需进行充分的边界测试。

@BLumia BLumia requested review from 18202781743 and wjyrich March 13, 2026 03:09
@deepin-ci-robot
Copy link

[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.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@BLumia BLumia merged commit 2b9ea74 into linuxdeepin:master Mar 14, 2026
9 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants