Skip to content

fix: fix bubble list layout update on height change#1664

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
52cyb:notification
Jul 15, 2026
Merged

fix: fix bubble list layout update on height change#1664
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
52cyb:notification

Conversation

@52cyb

@52cyb 52cyb commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Replace the remove/insert workaround with replace + dataChanged and handle
height recalculations properly by toggling displayMarginBeginning. The old
approach of removing and inserting items forced delegate recreation but caused
unnecessary overhead and potential visual glitches. The new approach uses
QList::replace() which is more efficient, but requires explicit handling of
height updates because the view's transition logic can prevent proper re-layout when an item's height changes during a running transition.

Influence:

  1. Test notification bubble replacement in normal scenarios
  2. Test bubble height changes when content updates (e.g., expanding/ collapsing)
  3. Verify no visual glitches during rapid notification updates
  4. Test with multiple simultaneous notifications and transitions
  5. Verify smooth scrolling behavior when bubble heights change
  6. Test edge cases with very long notification messages

fix: 修复气泡列表高度变化时布局更新问题

replace + dataChanged 替换移除/插入的变通方案,并通过切换
displayMarginBeginning 正确处理高度重新计算。旧方法通过移除和插入项目
强制
委托重建,但导致不必要的开销和潜在的视觉问题。新方法使用
QList::replace()
效率更高,但需要显式处理高度更新,因为视图的过渡逻辑可能会在运行过渡期间
阻止项目高度变化时的正确重新布局。

Influence:

  1. 测试正常场景下的通知气泡替换
  2. 测试内容更新时气泡高度的变化(如展开/折叠)
  3. 验证快速更新通知时无视觉闪烁
  4. 测试多个同时发生的通知和过渡
  5. 验证气泡高度变化时滚动行为流畅
  6. 测试超长通知消息的边界情况

PMS: BUG-367205

Summary by Sourcery

Improve notification bubble replacement to use in-place model updates and ensure list layout recalculates correctly when bubble heights change.

Bug Fixes:

  • Fix notification bubble list failing to update layout when bubble heights change during transitions.

Enhancements:

  • Use QList in-place replacement with dataChanged for bubble items instead of remove/insert to reduce overhead and avoid visual glitches during rapid updates.

@deepin-ci-robot

Copy link
Copy Markdown

Hi @52cyb. Thanks for your PR.

I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sourcery-ai

sourcery-ai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Reviewer's Guide

Switches bubble replacement logic from remove/insert to QList::replace + dataChanged, and adds an explicit height change handler in the QML delegate to force proper ListView relayout when bubble heights change during transitions.

Sequence diagram for bubble replacement and height relayout

sequenceDiagram
    participant BubbleModel
    participant m_bubbles as QList_BubbleItem
    participant View as ListView_view
    participant Delegate as BubbleDelegate

    BubbleModel->>BubbleModel: replaceBubble(bubble)
    BubbleModel->>m_bubbles: replace(replaceIndex, bubble)
    BubbleModel-->>View: dataChanged(index(replaceIndex), index(replaceIndex))

    View-->>Delegate: update delegate for replaceIndex

    alt [bubbleContent.height changes]
        Delegate-->>Delegate: onHeightChanged
        Delegate-->>View: read displayMarginBeginning
        Delegate->>View: set displayMarginBeginning(m + 1)
        Delegate->>View: set displayMarginBeginning(m)
        View-->>View: forceLayoutPolish() with updated contentHeight
    end
Loading

File-Level Changes

Change Details Files
Ensure ListView relayouts correctly when a bubble delegate’s height changes, even while transitions are running.
  • Bind delegate height directly to bubbleContent.height as before, but add an onHeightChanged handler to react to runtime height changes.
  • Early-return in the height change handler for invalid indices or missing ListView.view to avoid unnecessary work.
  • Force ListView to recompute contentHeight when item height changes by toggling displayMarginBeginning, bypassing transition-related early-exit in the layout polishing logic.
panels/notification/bubble/package/BubbleDelegate.qml
Replace the previous remove/insert workaround in the bubble model with QList::replace plus a dataChanged signal to update existing delegates efficiently.
  • Compute the replacement index and retrieve the old bubble item as before.
  • Use m_bubbles.replace() instead of removeAt/insert to update the bubble in-place.
  • Emit dataChanged for the replaced index to notify views so existing delegates update without recreation and with less overhead.
panels/notification/bubble/bubblemodel.cpp

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


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.


height: bubbleContent.height

onHeightChanged: {

@18202781743 18202781743 Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

这个能再优化下么,减少view变化的触发,另外这个逻辑放在main.qml里,listview里,

Replace the remove/insert workaround with `replace` + `dataChanged`
and handle
height recalculations properly by toggling displayMarginBeginning. The
old
approach of removing and inserting items forced delegate recreation
but caused
unnecessary overhead and potential visual glitches. The new approach
uses
QList::replace() which is more efficient, but requires explicit handling
of
height updates because the view's transition logic can prevent proper
re-layout when an item's height changes during a running transition.

Influence:
1. Test notification bubble replacement in normal scenarios
2. Test bubble height changes when content updates (e.g., expanding/
collapsing)
3. Verify no visual glitches during rapid notification updates
4. Test with multiple simultaneous notifications and transitions
5. Verify smooth scrolling behavior when bubble heights change
6. Test edge cases with very long notification messages

fix: 修复气泡列表高度变化时布局更新问题

用 `replace` + `dataChanged` 替换移除/插入的变通方案,并通过切换
displayMarginBeginning 正确处理高度重新计算。旧方法通过移除和插入项目
强制
委托重建,但导致不必要的开销和潜在的视觉问题。新方法使用
QList::replace()
效率更高,但需要显式处理高度更新,因为视图的过渡逻辑可能会在运行过渡期间
阻止项目高度变化时的正确重新布局。

Influence:
1. 测试正常场景下的通知气泡替换
2. 测试内容更新时气泡高度的变化(如展开/折叠)
3. 验证快速更新通知时无视觉闪烁
4. 测试多个同时发生的通知和过渡
5. 验证气泡高度变化时滚动行为流畅
6. 测试超长通知消息的边界情况

PMS: BUG-367205
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, 52cyb

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

@52cyb

52cyb commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit add1aee into linuxdeepin:master Jul 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants