refactor(notification): move bubble overlay logic to QML delegate#1565
refactor(notification): move bubble overlay logic to QML delegate#1565deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideRefactors the notification bubble system so that overlay/folding visuals and disappearance animations are handled entirely in QML delegates, while the C++ model becomes a simple list with a max-keep limit and updated roles, and the QML UI is updated to use a unified Bubble component and smoother panel visibility behavior. Sequence diagram for bubble removal and panel hide with QML transitionssequenceDiagram
actor User
participant Applet
participant BubblePanel
participant BubbleModel
participant BubbleView as QML_BubbleView_ListView
participant BubbleDelegate
User->>BubbleDelegate: Dismiss or close bubble
BubbleDelegate->>Applet: close(bubble.index, NotifyItem.Dismissed or Closed)
Applet->>BubblePanel: onNotificationStateChanged(id, processedType)
BubblePanel->>BubbleModel: remove(index)
BubbleModel->>BubbleModel: takeAt(index), deleteLater()
BubbleModel->>BubbleView: model changed (row removed)
BubbleView->>BubbleDelegate: trigger remove and removeDisplaced transitions
BubbleDelegate->>BubbleDelegate: play opacity,y,scale animations
BubbleModel-->>BubblePanel: bubble count changed (model empty)
BubblePanel->>BubblePanel: onBubbleCountChanged()
alt model is empty
BubblePanel->>BubblePanel: QTimer::singleShot(500ms)
Note over BubbleDelegate,BubblePanel: remove transitions continue during delay
BubblePanel-->>BubblePanel: after 500ms, check items().isEmpty()
BubblePanel->>BubblePanel: setVisible(false)
else model not empty
BubblePanel->>BubblePanel: setVisible(enabled())
end
Updated class diagram for notification bubble model and itemclassDiagram
class BubbleItem {
+NotifyEntity m_entity
-int m_urgency
-QString m_timeTip
-bool m_enablePreview
+QString appName() const
+QString body() const
+QString summary() const
+QString appIcon() const
+QString bodyImagePath() const
+int urgency() const
+QVariantList actions() const
+void updateActions()
+QString timeTip() const
+void setTimeTip(QString timeTip)
+bool isValid() const
+void timeTipChanged()
}
class BubbleModel {
-QTimer* m_updateTimeTipTimer
-QList~BubbleItem*~ m_bubbles
-int m_maxKeep
-int m_contentRowCount
+BubbleModel(QObject* parent)
+int rowCount(QModelIndex parent) const
+QVariant data(QModelIndex index, int role) const
+QHash~int, QByteArray~ roleNames() const
+int displayRowCount() const
+void push(BubbleItem* bubble)
+void remove(int index)
+void remove(const BubbleItem* bubble)
+void clear()
+void clearInvalidBubbles()
-void updateBubbleCount(int count)
-int replaceBubbleIndex(const BubbleItem* bubble) const
-void updateBubbleTimeTip()
-void updateContentRowCount(int rowCount)
}
class BubblePanel {
-BubbleModel* m_bubbles
+void onBubbleCountChanged()
+void addBubble(qint64 id)
+void onNotificationStateChanged(qint64 id, int processedType)
+void setVisible(bool visible)
+bool enabled() const
}
BubbleModel "1" --> "*" BubbleItem : owns
BubblePanel "1" --> "1" BubbleModel : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The overlay retention logic is split between C++ (m_maxKeep = bubbleCount + 2) and QML (opacity/levelsFolded based on maxCount and hard-coded 2/3), which makes behavior harder to reason about; consider centralizing these constants or exposing a single role from the model so QML doesn’t have to duplicate the ‘+2’ and fold-depth assumptions.
- BubbleDelegate.qml’s realIndex calculation relies on ListView.view.count and index potentially being negative, which couples the delegate tightly to ListView internals; if you only target ListView, you can likely drop the negative-index adjustment and use index directly, or otherwise document this assumption to avoid fragile reuse.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The overlay retention logic is split between C++ (m_maxKeep = bubbleCount + 2) and QML (opacity/levelsFolded based on maxCount and hard-coded 2/3), which makes behavior harder to reason about; consider centralizing these constants or exposing a single role from the model so QML doesn’t have to duplicate the ‘+2’ and fold-depth assumptions.
- BubbleDelegate.qml’s realIndex calculation relies on ListView.view.count and index potentially being negative, which couples the delegate tightly to ListView internals; if you only target ListView, you can likely drop the negative-index adjustment and use index directly, or otherwise document this assumption to avoid fragile reuse.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
9f122b6 to
0c4074e
Compare
1. Remove level property and overlay count tracking from BubbleItem and BubbleModel 2. Introduce BubbleDelegate.qml with visual folding effects (scale, y-offset, opacity) based on index 3. Simplify BubbleModel to use m_maxKeep limit instead of separate display/overlay counts 4. Add remove and removeDisplaced transitions for smooth disappearance animations 5. Consolidate NormalBubble.qml and OverlayBubble.qml into single Bubble.qml component 6. Add delayed hide (500ms) in BubblePanel to allow QML animations to complete Log: Refactor notification bubble system to handle overlay visual effects in QML instead of C++ model refactor(notification): 将气泡折叠逻辑移至 QML 代理组件 1. 移除 BubbleItem 和 BubbleModel 中的 level 属性及折叠计数追踪 2. 新增 BubbleDelegate.qml,根据索引实现视觉折叠效果(缩放、Y 偏移、透明度) 3. 简化 BubbleModel,使用 m_maxKeep 限制替代分离的显示/折叠计数 4. 添加 remove 和 removeDisplaced 过渡动画实现平滑消失效果 5. 合并 NormalBubble.qml 和 OverlayBubble.qml 为单一 Bubble.qml 组件 6. BubblePanel 添加 500ms 延迟隐藏,确保 QML 动画播放完成 Log: 重构通知气泡系统,将折叠视觉效果从 C++ 模型移至 QML 处理 PMS: BUG-355029
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy 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 |
deepin pr auto review这份代码 diff 主要实现了三个方面的功能改进:
以下是详细的代码审查意见: 1. 语法逻辑审查1.1 输入区域实现优点:
问题:
// 建议添加 XCB 错误处理
xcb_generic_error_t *error = nullptr;
xcb_shape_rectangles(...);
if (error) {
qWarning() << "Failed to set input region:" << error->error_code;
free(error);
}
// 建议添加检查
auto applyInputRegion = [this, window]() {
if (!window || !window->window()) return;
window->window()->setMask(m_dlayerShellWindow->inputRegion());
window->waylandSurface()->commit();
};1.2 通知气泡模型优点:
问题:
// 如果在定时器触发前对象被删除,pendingBubbles 中的指针会悬空
// 建议在析构函数中已经处理了,但可以更明确
BubbleModel::~BubbleModel()
{
if (m_processPendingTimer) {
m_processPendingTimer->stop();
}
qDeleteAll(m_pendingBubbles); // 已有
m_pendingBubbles.clear();
qDeleteAll(m_bubbles); // 已有
m_bubbles.clear();
}
// 建议添加空指针检查
void BubbleModel::insertBubble(BubbleItem *bubble)
{
if (!bubble) return; // 添加空指针检查
// ... 其余代码
}2. 代码质量审查2.1 代码组织
// 建议定义为属性
property int maxCount: 3
property int spacing: 10
property int peekAmount: 8
property int maxFoldedLevels: 32.2 注释和文档
/**
* @brief 设置矩形输入区域
* @param x 矩形左上角X坐标
* @param y 矩形左上角Y坐标
* @param width 矩形宽度
* @param height 矩形高度
*/
void DLayerShellWindow::setInputRegionRect(int x, int y, int width, int height)2.3 代码重复
3. 代码性能审查3.1 输入区域更新
// 建议使用防抖(debounce)机制
function updateInputRegion() {
if (updateInputRegionTimer.running) return;
updateInputRegionTimer.start();
}
Timer {
id: updateInputRegionTimer
interval: 16 // ~60fps
onTriggered: {
root.DLayerShellWindow.setInputRegionRect(...);
}
}3.2 模型更新
// 建议使用 beginResetModel/endResetModel 批量处理
void BubbleModel::clearInvalidBubbles()
{
if (m_bubbles.isEmpty()) return;
beginResetModel();
// 批量删除无效气泡
m_bubbles.erase(std::remove_if(m_bubbles.begin(), m_bubbles.end(),
[](BubbleItem *item) { return !item->isValid(); }),
m_bubbles.end());
endResetModel();
}4. 代码安全审查4.1 输入验证
void DLayerShellWindow::setInputRegionRect(int x, int y, int width, int height)
{
// 添加参数验证
if (width <= 0 || height <= 0) {
qWarning() << "Invalid rectangle dimensions:" << width << height;
return;
}
setInputRegion(QRegion(x, y, width, height));
}4.2 内存安全
// 建议使用 QSharedPointer 或 QScopedPointer
QList<QSharedPointer<BubbleItem>> m_bubbles;
QQueue<QSharedPointer<BubbleItem>> m_pendingBubbles;4.3 线程安全
// 建议考虑使用 QtConcurrent 处理耗时操作
QtConcurrent::run([this]() {
// XCB 调用
QMetaObject::invokeMethod(this, [this]() {
// 更新 UI
}, Qt::QueuedConnection);
});5. 其他建议
总体而言,这次更新实现了重要的功能改进,但在错误处理、性能优化和代码健壮性方面还有提升空间。建议在合并前添加适当的错误处理和单元测试。 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Log: Refactor notification bubble system to handle overlay visual effects in QML instead of C++ model
refactor(notification): 将气泡折叠逻辑移至 QML 代理组件
Log: 重构通知气泡系统,将折叠视觉效果从 C++ 模型移至 QML 处理
PMS: BUG-355029
Summary by Sourcery
Refactor the notification bubble system so that overlay/folding behavior and animations are handled in QML delegates instead of the C++ model, while simplifying bubble count management.
New Features:
Bug Fixes:
Enhancements: