fix(notification): fix invisible delegates after collapsing expanded …#1476
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 5, 2026
Merged
fix(notification): fix invisible delegates after collapsing expanded …#1476deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplaces a disruptive layoutChanged() emit with a lightweight ListView refresh mechanism by simulating a tiny scroll after collapsing/expanding notification app groups, ensuring displaced delegates are re-rendered without breaking animations. Sequence diagram for refreshed delegates after notification collapse/expandsequenceDiagram
actor User
participant NotifyViewDelegate
participant NotifyModel
participant NotifyView_ListView
User->>NotifyViewDelegate: clickCollapse(appGroup)
NotifyViewDelegate->>NotifyModel: collapseApp(index)
NotifyModel-->>NotifyViewDelegate: collapseCompleted
NotifyViewDelegate->>NotifyView_ListView: refreshAfterCollapse()
activate NotifyView_ListView
NotifyView_ListView->>NotifyView_ListView: Qt.callLater(callback)
Note over NotifyView_ListView: callback executes shortly after
NotifyView_ListView->>NotifyView_ListView: contentY++ (tiny scroll)
NotifyView_ListView->>NotifyView_ListView: re-render displaced delegates
deactivate NotifyView_ListView
User->>NotifyViewDelegate: clickExpand(appGroup)
NotifyViewDelegate->>NotifyModel: expandApp(index)
NotifyModel-->>NotifyViewDelegate: expandCompleted
NotifyViewDelegate->>NotifyView_ListView: refreshAfterCollapse()
activate NotifyView_ListView
NotifyView_ListView->>NotifyView_ListView: Qt.callLater(callback)
NotifyView_ListView->>NotifyView_ListView: contentY++ (tiny scroll)
NotifyView_ListView->>NotifyView_ListView: re-render displaced delegates
deactivate NotifyView_ListView
NotifyViewDelegate->>NotifyView_ListView: requestFocusOnExpand(targetIndex)
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}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
refreshAfterCollapseimplementation permanently incrementscontentYon every call; consider storing the current value and restoring it (or doing a small up/down adjustment in the samecallLater) to avoid cumulative drift and unexpected jumps for users who collapse/expand frequently. - Since
refreshAfterCollapseis also called after expand, you might want to rename it to something more generic (e.g.refreshAfterToggle) and document any assumptions aboutroot.view(that it directly exposescontentYand is aListView) to make the coupling clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `refreshAfterCollapse` implementation permanently increments `contentY` on every call; consider storing the current value and restoring it (or doing a small up/down adjustment in the same `callLater`) to avoid cumulative drift and unexpected jumps for users who collapse/expand frequently.
- Since `refreshAfterCollapse` is also called after expand, you might want to rename it to something more generic (e.g. `refreshAfterToggle`) and document any assumptions about `root.view` (that it directly exposes `contentY` and is a `ListView`) to make the coupling clearer.
## Individual Comments
### Comment 1
<location path="panels/notification/center/NotifyView.qml" line_range="87-89" />
<code_context>
+ // Refresh delegates after collapse/expand by simulating a tiny scroll
+ // This triggers ListView to re-render displaced delegates that may be invisible
+ // Called immediately after collapseApp/expandApp, before transition starts
+ function refreshAfterCollapse() {
+ Qt.callLater(function() {
+ contentY++
+ })
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** refreshAfterCollapse permanently shifts scroll position by 1 unit on every call
Because `contentY` is only incremented and never reset, each collapse/expand shifts the viewport down by 1 unit, gradually drifting the view from the user’s expected position. If you just need to trigger a re-render, use a net-zero scroll (e.g., `contentY += 1; contentY -= 1`, possibly via two `Qt.callLater` calls) or a more direct layout invalidation API so the visible scroll position stays stable.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
re2zero
force-pushed
the
bugfix
branch
4 times, most recently
from
0aa40d0 to
6b3cfd0
Compare
18202781743
reviewed
Mar 4, 2026
There was a problem hiding this comment.
Pull request overview
This PR fixes a UI issue in the notification center where delegates below a collapsed/expanded app group can become invisible, by avoiding a model layoutChanged() emission (which breaks animations) and adding a QML-side refresh mechanism to force the ListView to re-render displaced delegates.
Changes:
- Removed
emit layoutChanged()fromNotifyModel::collapseApp()to avoid breakingListViewtransitions/animations. - Added a
ListView.refreshAfterCollapse()helper in QML that simulates a tiny scroll to trigger delegate re-rendering. - Invoked the refresh helper after group collapse/expand actions in the delegate.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| panels/notification/center/notifymodel.cpp | Stops emitting layoutChanged() during collapse to preserve animations. |
| panels/notification/center/NotifyViewDelegate.qml | Calls the new view refresh hook after collapse/expand actions. |
| panels/notification/center/NotifyView.qml | Adds refreshAfterCollapse() implementation via small contentY nudge. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…notifications Remove emit layoutChanged() which breaks animations and add refresh mechanism to trigger ListView re-render of displaced delegates. 删除emit layoutChanged()避免破坏动画，通过模拟微小滚动触发ListView 重新渲染被移位的delegate。 PMS: BUG-351731 BUG-351729 Log: 修复收起超一屏应用通知时下方应用不显示的问题 Influence: 修复后收起应用通知下方应用正常显示，无需手动刷新
deepin pr auto review这段代码的修改主要涉及三个方面：版权年份更新、新增了一个强制ListView重绘的函数 以下是对代码的详细审查和改进建议： 1. 语法与逻辑审查
2. 代码质量
3. 代码性能
4. 代码安全
5. 改进建议
总结这段代码修改是一个优化性质的修复。它通过 QML 层的一个低开销 Hack 替代了 C++ 层的高开销全量刷新信号，解决了潜在的渲染卡顿或状态丢失问题，同时修复了视觉上的渲染错误。逻辑是自洽的，只要注意上述的边界情况和后续维护即可。 |
18202781743
approved these changes
Mar 5, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, re2zero 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 |
|
This pr force merged! (status: unstable) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
…notifications
Remove emit layoutChanged() which breaks animations and add refresh mechanism to trigger ListView re-render of displaced delegates.
删除emit layoutChanged()避免破坏动画，通过模拟微小滚动触发ListView重新渲染被移位的delegate。
PMS: BUG-351731 BUG-351729
Log: 修复收起超一屏应用通知时下方应用不显示的问题
Influence: 修复后收起应用通知下方应用正常显示，无需手动刷新
Summary by Sourcery
Bug Fixes: