fix: icons stuck together caused by listview add animation #1367
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
由于 QTBUG-133953, 任务栏添加 icon 时新元素的加入动画一定会导致其 布局计算得到的位置是一个错误的位置.原本的修复是通过延时强制重新触发 布局实现的,但延时重新触发不一定可靠,导致仍可能概率性发生. 尽管 QTBUG-133953 并未得以解决,此处通过将动画时机进行调整,使相关位置计算 问题不再发生. PMS: BUG-308927 Log:
BLumia
requested review from
18202781743,
robertkill,
wjyrich and
yixinshark
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the TaskManager QML taskbar layout workaround by removing the force-relayout timer hack and moving the add animation to a per-delegate ListView.onAdd animation, ensuring correct layout calculation while keeping add/remove animations. Sequence diagram for new ListView.onAdd animation and layout calculationsequenceDiagram
participant PanelRoot as PanelRootObject
participant TaskManager as TaskManager
participant Overflow as OverflowContainer
participant ListView as AppsListView
participant Delegate as AppDelegate
PanelRoot->>TaskManager: dockItemMaxSize changes / new app added
TaskManager->>Overflow: visualModel updated
Overflow->>ListView: count changes
ListView->>ListView: onCountChanged
ListView->>TaskManager: DS.singleShot(300, updateDynamicCharLimits)
Note over ListView,Delegate: New delegate is created and positioned using correct layout
ListView->>Delegate: ListView.onAdd triggered
Delegate->>Delegate: NumberAnimation(scale, opacity, 0 -> 1, 200ms)
Note over TaskManager: No forceRelayoutWorkaround timer or property
Note over TaskManager: implicitWidth/implicitHeight use appContainer.implicitWidth/Height directly
Class diagram for updated TaskManager QML structure and propertiesclassDiagram
class ContainmentItem {
}
class TaskManager {
<<QML Item>>
bool useColumnLayout
int dockOrder
int remainingSpacesForTaskManager
int remainingSpacesForSplitWindow
int appContainerWidth
int appContainerHeight
var dynamicCharLimits
int implicitWidth
int implicitHeight
findAppIndex(appId)
updateDynamicCharLimits()
}
class OverflowContainer {
<<QML Item>>
bool useColumnLayout
real spacing
}
class AppsListView {
<<QML ListView>>
int count
real cellWidth
onCountChanged()
}
class AppDelegate {
<<QML Item>>
ListView.onAdd()
}
ContainmentItem *-- TaskManager : rootItem
TaskManager *-- OverflowContainer : overflowContainer
OverflowContainer *-- AppsListView : appsListView
AppsListView *-- AppDelegate : delegate
%% Removed elements (for context)
class RemovedTimer {
<<QML Timer>>
int interval
bool repeat
onTriggered()
}
TaskManager .. RemovedTimer : removed
class RemovedProperty {
<<property>>
int forceRelayoutWorkaround
}
TaskManager .. RemovedProperty : removed
class RemovedAddTransition {
<<Transition>>
addAnimation(scale, opacity, 0 -> 1, 200ms)
}
OverflowContainer .. RemovedAddTransition : removed add transition
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这个diff进行仔细的审查：
建议：
总的来说，这是一个很好的代码改进，提高了代码的可维护性和性能，同时保持了原有的功能。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
18202781743
reviewed
Dec 15, 2025
| return false | ||
| } | ||
|
|
||
| ListView.onAdd: NumberAnimation { |
Contributor
There was a problem hiding this comment.
add是附加属性的信号，这里是响应信号的槽函数么，这样写是什么意思呀？
Member
Author
There was a problem hiding this comment.
就是在 onAdd 信号触发的时候触发动画。
（p.s. 这是 QTBUG-133953 里给我的建议修改方式）
There was a problem hiding this comment.
Pull request overview
This PR addresses an issue where taskbar icons get stuck together due to a ListView add animation causing incorrect layout positioning. The fix removes a timer-based workaround and attempts to move the add animation from the ListView transition level to individual delegate items.
Key Changes:
- Removed
forceRelayoutWorkaroundproperty and related timer-based workaround logic - Removed ListView
addtransition and replaced with per-delegateListView.onAddhandler - Simplified implicit width/height calculations by removing workaround references
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+291
to
+298
| ListView.onAdd: NumberAnimation { | ||
| target: delegateRoot | ||
| properties: "scale,opacity" | ||
| from: 0 | ||
| to: 1 | ||
| duration: 200 | ||
| } | ||
|
|
There was a problem hiding this comment.
The syntax ListView.onAdd: NumberAnimation { ... } appears non-standard. In QML, ListView.onAdd is an attached signal handler that typically expects JavaScript code, not a direct Animation assignment. The standard approach would be to define the animation separately and start it within the signal handler, or use the ListView's add transition (the original approach). This may not execute as intended. Additionally, this approach could conflict with the existing state-based PropertyChanges (lines 299-310) and Behaviors (lines 312-313) that also control the opacity and scale properties.
Suggested change
| ListView.onAdd: NumberAnimation { | |
| target: delegateRoot | |
| properties: "scale,opacity" | |
| from: 0 | |
| to: 1 | |
| duration: 200 | |
| } |
18202781743
approved these changes
Dec 15, 2025
|
[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 |
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.
由于 QTBUG-133953, 任务栏添加 icon 时新元素的加入动画一定会导致其
布局计算得到的位置是一个错误的位置.原本的修复是通过延时强制重新触发
布局实现的,但延时重新触发不一定可靠,导致仍可能概率性发生. 尽管
QTBUG-133953 并未得以解决,此处通过将动画时机进行调整,使相关位置计算
问题不再发生.
Summary by Sourcery
Adjust task manager task list animations and layout sizing to avoid incorrect icon positioning when new items are added.
Bug Fixes:
Enhancements: