Skip to content

fix: fix notification text being truncated after replace#1619

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
18202781743:master
Jun 5, 2026
Merged

fix: fix notification text being truncated after replace#1619
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743

@18202781743 18202781743 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Replace dataChanged signal with remove+insert row operations to force
ListView to recalculate delegate size, preventing notification content
from being clipped.

Log: Fixed notification text truncation issue after replacement

Influence:

  1. Test notification replacement with various text lengths
  2. Verify notification bubble height adapts to content
  3. Test multiple rapid replacements for stability
  4. Verify notification text is fully visible after each replace

fix: 修复通知替换后文字被截断问题

将 dataChanged 信号替换为移除+插入行操作,强制 ListView 重新计算委托大
小,防止通知内容被裁剪。

Log: 修复通知替换后文字截断问题

Influence:

  1. 测试不同文本长度下的通知替换功能
  2. 验证通知气泡高度能自适应内容
  3. 测试多次快速替换的稳定性
  4. 验证每次替换后通知文字完整可见

Summary by Sourcery

Bug Fixes:

  • Prevent notification text from being truncated after a notification item is replaced in the list view.

Replace dataChanged signal with remove+insert row operations to force
ListView to recalculate delegate size, preventing notification content
from being clipped.

Log: Fixed notification text truncation issue after replacement

Influence:
1. Test notification replacement with various text lengths
2. Verify notification bubble height adapts to content
3. Test multiple rapid replacements for stability
4. Verify notification text is fully visible after each replace

fix: 修复通知替换后文字被截断问题

将 dataChanged 信号替换为移除+插入行操作,强制 ListView 重新计算委托大
小,防止通知内容被裁剪。

Log: 修复通知替换后文字截断问题

Influence:
1. 测试不同文本长度下的通知替换功能
2. 验证通知气泡高度能自适应内容
3. 测试多次快速替换的稳定性
4. 验证每次替换后通知文字完整可见
@sourcery-ai

sourcery-ai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Reworks bubble replacement in the notification model to use remove/insert row operations instead of dataChanged so that the view recreates delegates and recalculates ListView item height, preventing notification text from being visually truncated after replacement.

Sequence diagram for notification bubble replacement using remove/insert rows

sequenceDiagram
    participant ListView
    participant BubbleModel

    ListView->>BubbleModel: replaceBubble(bubble)
    activate BubbleModel
    BubbleModel->>BubbleModel: beginRemoveRows(QModelIndex, replaceIndex, replaceIndex)
    BubbleModel->>BubbleModel: m_bubbles.removeAt(replaceIndex)
    BubbleModel-->>ListView: rowsRemoved
    BubbleModel->>BubbleModel: endRemoveRows()

    BubbleModel->>BubbleModel: beginInsertRows(QModelIndex, replaceIndex, replaceIndex)
    BubbleModel->>BubbleModel: m_bubbles.insert(replaceIndex, bubble)
    BubbleModel-->>ListView: rowsInserted
    BubbleModel->>BubbleModel: endInsertRows()
    deactivate BubbleModel

    rect rgba(200,200,255,0.2)
        Note over ListView: ListView recreates delegates and recalculates item height
    end

    %% Previous behavior (now removed)
    %% ListView->>BubbleModel: replaceBubble(bubble)
    %% BubbleModel->>BubbleModel: m_bubbles.replace(replaceIndex, bubble)
    %% BubbleModel-->>ListView: dataChanged(index, index)
Loading

File-Level Changes

Change Details Files
Force ListView to recreate notification delegates on replacement to fix text truncation.
  • Replace in-place model update via m_bubbles.replace(...) and dataChanged() with explicit beginRemoveRows/removeAt/endRemoveRows followed by beginInsertRows/insert/endInsertRows for the same index
  • Retain and return the previous BubbleItem pointer so existing ownership semantics for the replaced bubble are unchanged
  • Document the rationale in-code that remove+insert is used so the view recalculates delegate height and avoids clipping notification content
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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff 代码。这次修改的核心目的是通过 remove + insert 操作强制视图重新创建委托,从而解决 ListView 无法正确重新计算高度的问题。

整体思路上,使用 beginRemoveRows/endRemoveRowsbeginInsertRows/endInsertRows 来代替单纯的 dataChanged 是 Qt Model/View 架构中处理此类 UI 刷新问题的标准做法。但是,这段代码在内存管理(逻辑与安全)和代码性能上存在一些需要改进的地方。

以下是详细的审查意见和改进建议:

1. 语法与逻辑 (内存管理隐患 - 严重)

在原代码中,m_bubbles.replace(replaceIndex, bubble) 会返回被替换的 oldBubble 指针,函数末尾也 return oldBubble;。但在修改后的代码中,使用了 removeAt,它不会返回被移除的对象,而是直接从 QList 中删除。

问题:
如果 m_bubbles 是一个管理内存的容器(比如在析构时遍历删除内部指针,或者 oldBubble 的所有权转移给了调用者),那么 removeAt 之后再 insert,会导致 oldBubble 指针虽然被返回,但其语义变得混乱。更严重的是,如果调用者没有正确处理 oldBubble 的内存释放,或者 m_bubbles 的析构函数依然试图删除它,就会导致内存泄漏双重释放

改进建议:
确保在移除之前提取 oldBubble,并明确其所有权。如果 BubbleItem 继承自 QObject 并且有指定的 parent,Qt 的对象树会管理内存;否则必须手动处理。

2. 代码性能 (视图刷新开销)

使用 remove + insert 会触发视图的两次完整更新流程(销毁旧委托,创建新委托),并且可能会导致 ListView 产生不必要的动画(移除动画+插入动画),这在通知列表这种频繁更新的场景中可能会引起视觉闪烁和性能损耗。

改进建议:
虽然你提到需要强制重新计算高度,但 Qt 提供了更轻量级的机制来通知尺寸变化。可以考虑以下两种性能更优的方案:

  • 方案 A (推荐):保留 dataChanged,但额外触发尺寸变化信号。
    如果你的模型继承自 QAbstractItemModel,可以在发射 dataChanged 时,同时通知 SizeHintRole 发生了变化。对于 Qt Quick 的 ListView,这通常足以让它重新请求并计算高度,而无需销毁和重建委托。
  • 方案 B:使用 beginResetModel / endResetModel
    如果数据结构变化较大,直接重置模型比手动 remove + insert 更安全,且不会产生奇怪的插入/删除动画。

3. 代码安全 (空指针与越界风险)

代码中 const auto oldBubble = m_bubbles[replaceIndex]; 没有对 replaceIndex 的有效性进行二次确认。虽然开头有 Q_ASSERT(isReplaceBubble(bubble));,但 Q_ASSERT 在 Release 版本中会被编译器移除,如果 replaceBubbleIndex 返回了无效索引(例如 -1),m_bubbles[replaceIndex] 将导致越界访问崩溃。

改进建议:
增加防御性编程检查,确保索引有效。


改进后的代码示例

结合以上分析,我为你提供两个版本的改进代码:

版本一:基于你当前的 remove + insert 思路(修复了逻辑和安全问题)

BubbleItem *BubbleModel::replaceBubble(BubbleItem *bubble)
{
    Q_ASSERT(isReplaceBubble(bubble));
    const auto replaceIndex = replaceBubbleIndex(bubble);
    
    // 防御性编程:防止 Release 模式下越界崩溃
    if (replaceIndex < 0 || replaceIndex >= m_bubbles.size()) {
        return nullptr; 
    }

    // 先取出旧指针,避免 removeAt 后丢失引用导致内存管理混乱
    const auto oldBubble = m_bubbles[replaceIndex];

    // Use remove + insert instead of dataChanged to force the view
    // to recreate the delegate, so ListView recalculates its height.
    beginRemoveRows(QModelIndex(), replaceIndex, replaceIndex);
    m_bubbles.removeAt(replaceIndex);
    endRemoveRows();

    beginInsertRows(QModelIndex(), replaceIndex, replaceIndex);
    m_bubbles.insert(replaceIndex, bubble);
    endInsertRows();

    // 注意:确保 oldBubble 的内存由调用者正确处理,或者被 Qt 对象树接管
    return oldBubble;
}

版本二:性能更优的 dataChanged + 尺寸通知方案(推荐尝试)

如果你的目的是让 ListView 重新计算高度,通常只需要告诉它尺寸角色发生了变化,不需要销毁重建委托:

BubbleItem *BubbleModel::replaceBubble(BubbleItem *bubble)
{
    Q_ASSERT(isReplaceBubble(bubble));
    const auto replaceIndex = replaceBubbleIndex(bubble);
    
    if (replaceIndex < 0 || replaceIndex >= m_bubbles.size()) {
        return nullptr; 
    }

    const auto oldBubble = m_bubbles[replaceIndex];
    // 直接替换数据
    m_bubbles.replace(replaceIndex, bubble);

    // 发出数据改变信号,并明确指出尺寸角色可能发生了变化
    // QVector<int> roles = {BubbleModel::DisplayRole, BubbleModel::SizeHintRole}; // 根据你的实际 Role 定义调整
    // Q_EMIT dataChanged(index(replaceIndex), index(replaceIndex), roles);
    
    // 如果简单的 dataChanged 依然无法触发高度重算,可以强制刷新该 item 的尺寸:
    // 在 QML ListView 中,可能需要配合 delegate 的 Component.onCompleted 或者 
    // 强制发送 sizeHintRole 改变的信号
    
    return oldBubble;
}

总结: 建议优先采用版本一确保当前逻辑的内存安全和越界安全;如果在实际运行中发现 remove + insert 导致列表闪烁或卡顿,请尝试版本二并结合 QML 端的 ListView 强制更新高度机制。

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

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, fly602

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

@18202781743

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit e25ffed into linuxdeepin:master Jun 5, 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.

3 participants