Skip to content

fix: remove bubble removal animation and simplify visibility logic#1496

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
qxp930712:master
Mar 12, 2026
Merged

fix: remove bubble removal animation and simplify visibility logic#1496
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
qxp930712:master

Conversation

@qxp930712
Copy link

@qxp930712 qxp930712 commented Mar 12, 2026

Removed the complex animation sequence when bubbles are removed from the notification panel and simplified the visibility control logic. The previous implementation had a delayed hide mechanism that caused unnecessary complexity and potential timing issues.

The bubble removal animation in QML was overly complex with sequential animations and property actions, which could lead to inconsistent behavior. The C++ visibility logic was simplified by removing the delayed hide timer, making the panel state management more straightforward and reliable.

Log: Improved notification bubble panel behavior with smoother transitions

Influence:

  1. Test adding multiple notification bubbles
  2. Verify bubbles are properly removed when dismissed
  3. Check that panel visibility updates correctly when last bubble is removed
  4. Test panel behavior when notifications come in rapid succession
  5. Verify no visual glitches during bubble transitions
  6. Test with different notification types and durations

PMS: BUG-284659

Summary by Sourcery

Simplify notification bubble removal behavior and visibility handling in the bubble panel.

Bug Fixes:

  • Remove delayed hide logic for the bubble panel to avoid timing-related visibility issues.

Enhancements:

  • Eliminate the complex QML removal animation for notification bubbles in favor of immediate removal.
  • Streamline bubble panel visibility updates to depend directly on whether bubbles exist and the panel is enabled.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: qxp930712

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

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 12, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Removes the custom ListView removal animation for notification bubbles in QML and simplifies the C++ bubble panel visibility logic by eliminating the delayed hide timer and directly tying visibility to bubble presence and panel enabled state.

Class diagram for simplified BubblePanel visibility logic

classDiagram
    class BubblePanel {
        - BubbleModel* m_bubbles
        + onNotificationStateChanged(qint64 id, int processedType) void
        + onBubbleCountChanged() void
        + addBubble(qint64 id) void
        + setVisible(bool visible) void
        + enabled() bool
    }

    class BubbleModel {
        + items() QList~BubbleItem~
    }

    class BubbleItem {
        + id qint64
        + content QString
    }

    BubblePanel --> BubbleModel : uses
    BubbleModel --> BubbleItem : contains

    %% onBubbleCountChanged now directly sets visibility based on items().isEmpty() and enabled() without delayed hide timer
Loading

File-Level Changes

Change Details Files
Remove custom bubble removal animation in the QML ListView delegate to rely on default removal behavior.
  • Drop the delegate item id used solely for the removal animation.
  • Delete the SequentialAnimation attached to ListView.onRemove that animated the bubble x-position and used ListView.delayRemove to defer model removal.
panels/notification/bubble/package/main.qml
Simplify bubble panel visibility management logic in C++ by removing delayed hide behavior.
  • Stop using a QTimer::singleShot delay when the bubble list becomes empty.
  • Always set visibility based on whether there are any bubbles and the panel is enabled, using setVisible(!isEmpty && enabled()).
panels/notification/bubble/bubblepanel.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

Copy link

@sourcery-ai sourcery-ai bot left a comment

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.

Removed the complex animation sequence when bubbles are removed from
the notification panel and simplified the visibility control logic.
The previous implementation had a delayed hide mechanism that caused
unnecessary complexity and potential timing issues.

The bubble removal animation in QML was overly complex with sequential
animations and property actions, which could lead to inconsistent
behavior. The C++ visibility logic was simplified by removing
the delayed hide timer, making the panel state management more
straightforward and reliable.

Log: Improved notification bubble panel behavior with smoother
transitions

Influence:
1. Test adding multiple notification bubbles
2. Verify bubbles are properly removed when dismissed
3. Check that panel visibility updates correctly when last bubble is
removed
4. Test panel behavior when notifications come in rapid succession
5. Verify no visual glitches during bubble transitions
6. Test with different notification types and durations

PMS: BUG-284659
@deepin-ci-robot
Copy link

deepin pr auto review

这份代码修改涉及通知气泡面板(BubblePanel)的显示逻辑和气泡移除动画的处理。以下是对这两处修改的详细审查意见,涵盖语法逻辑、代码质量、性能和安全方面。

1. 文件:panels/notification/bubble/bubblepanel.cpp

修改内容:
移除了 onBubbleCountChanged 函数中当气泡列表为空时的 400ms 延迟隐藏逻辑,改为直接根据状态设置可见性。

审查意见:

  • 逻辑与功能(主要问题):

    • 用户体验下降(UX): 原代码在 isEmpty 为 true 时使用了 QTimer::singleShot(400, ...)。这通常是为了实现一种"延迟隐藏"的效果,给用户留出时间看到最后一个气泡消失,或者是为了等待 UI 动画(如淡出、收缩)完成。移除这个延迟会导致面板在最后一个气泡消失的瞬间立即消失,视觉上可能会显得突兀或生硬。
    • 逻辑一致性: 原代码中,非空时立即显示(setVisible(true)),空时延迟隐藏。新代码统一为立即显示或隐藏。虽然逻辑更统一了,但丢失了上述的 UX 细节。
  • 代码质量:

    • 简化逻辑: 新代码减少了分支判断,逻辑更紧凑,符合 KISS 原则(Keep It Simple, Stupid),在代码可读性上有所提升。
  • 改进建议:

    • 保留动画过渡: 如果移除延迟是为了修复某个 Bug(例如面板闪烁),建议确认 QML 端是否有替代的动画方案。如果 QML 端也没有动画,建议恢复延迟或通过其他方式(如 QML 的 Opacity 动画)来平滑过渡,而不是生硬地 setVisible(false)

2. 文件:panels/notification/bubble/package/main.qml

修改内容:
移除了 ListView 的 delegate (Bubble) 中关于 ListView.onRemoveSequentialAnimation,该动画原本负责在气泡移除时将其向右滑动 360 像素。

审查意见:

  • 逻辑与功能(主要问题):

    • 移除关键交互反馈: 这是一个破坏性的修改。原代码定义了气泡被移除时的离场动画:气泡会向右滑动并伴随指数缓动(Easing.InExpo)。移除这段代码后,气泡将直接从列表中消失,没有任何过渡效果。
    • 与 C++ 修改的关联: 结合 C++ 的修改来看,C++ 端移除了隐藏面板的延迟,QML 端移除了气泡消失的动画。这表明开发者可能有意彻底简化或移除所有过渡动画以追求"即时响应"或解决性能问题。但代价是交互体验的显著降级。
  • 代码质量:

    • 代码精简: 删除了约 20 行代码,减少了 QML 文件的复杂度。
  • 性能:

    • 性能提升: 移除动画确实会减少 CPU/GPU 的渲染开销,特别是在低性能设备上或通知频繁弹出时。但这通常是以牺牲用户体验为代价的。

综合分析与改进建议

这两处修改共同构成了一个明显的趋势:移除所有与通知气泡消失相关的延迟和动画

  1. 语法逻辑: 修改后的代码在语法上没有错误,逻辑也是自洽的(气泡消失 -> 面板立即隐藏)。
  2. 代码质量: 代码变得更短、更简单,易于阅读。
  3. 代码性能: 性能会有微小的提升(减少了定时器管理和动画渲染计算)。
  4. 代码安全: 此修改不涉及内存安全或权限安全问题。

最终建议:

  • 如果是为了修复 Bug: 请确认是否存在动画导致的界面卡顿、闪烁或状态不同步问题。如果是,当前的修改是有效的"止损"方案。
  • 如果是为了优化性能: 建议评估用户体验的损失是否值得。现代 UI 系统通常依赖动画来引导用户视线,直接消失会让用户感到困惑。
  • 推荐做法(折中方案):
    • QML 端: 建议保留简单的淡出(Opacity)动画,而不是完全移除动画。淡出动画通常比位移动画性能开销更小,且视觉上更柔和。
    • C++ 端: 如果 QML 端保留了淡出动画(例如 300ms),那么 C++ 端的 QTimer::singleShot 延迟应该设置为略大于或等于动画时长(例如 300ms 或 350ms),以确保面板在动画播放完毕后才消失,避免动画被截断。

总结: 这段代码在技术上没有问题,但在用户体验(UX)方面存在倒退。除非有极其特殊的性能限制或 Bug 需要修复,否则不建议合并此改动,或者建议采用更轻量级的动画方案替代。

@18202781743
Copy link
Contributor

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Mar 12, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit c044407 into linuxdeepin:master Mar 12, 2026
11 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.

4 participants