Skip to content

Conversation

@pengfeixx
Copy link
Contributor

@pengfeixx pengfeixx commented Feb 9, 2026

Fix the issue of the close button and shadow being cut off

Log: Fix the issue of the close button and shadow being cut off
pms: BUG-347237

Summary by Sourcery

Bug Fixes:

  • Allow notification list items to render beyond the list view bounds so the close button and drop shadow are no longer clipped.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 9, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the notification center list view so its content is no longer clipped, preventing the close button and drop shadow from being cut off.

File-Level Changes

Change Details Files
Allow the notification list view to render content outside its bounds so visual elements like the close button and shadow are not clipped.
  • Disabled clipping on the ListView used as the contentItem of the notification Control
  • Kept all other ListView configuration (spacing, snap mode, key navigation) unchanged to limit behavior changes to visual clipping
panels/notification/center/NotifyView.qml

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 left some high level feedback:

  • Disabling clip on the ListView may allow notifications to render outside the intended panel area; consider whether adjusting item padding/margins or the close button/shadow delegate layout could fix the cutoff while keeping clipping enabled.
  • Check the behavior when the list is long or scrolled: with clip: false, items or shadows might appear over adjacent UI or outside the notification center bounds, so validating the visual containment in edge cases would be useful.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Disabling `clip` on the `ListView` may allow notifications to render outside the intended panel area; consider whether adjusting item padding/margins or the close button/shadow delegate layout could fix the cutoff while keeping clipping enabled.
- Check the behavior when the list is long or scrolled: with `clip: false`, items or shadows might appear over adjacent UI or outside the notification center bounds, so validating the visual containment in edge cases would be useful.

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.

contentItem: ListView {
id: view
clip: true
clip: false
Copy link
Contributor

Choose a reason for hiding this comment

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

这个好像会导致之前修的那一个bug有问题

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Fix the issue of the close button and shadow being cut off

Log: Fix the issue of the close button and shadow being cut off
pms: BUG-347237
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查报告

经过对提供的 Git Diff 进行仔细审查,以下是从语法逻辑、代码质量、代码性能和代码安全四个方面的详细分析和改进建议:

1. 语法逻辑

  • 分析
    • 代码逻辑主要涉及将 NotifyCenter.qml 中 Header 的左边距和 NotifyView.qml 中 ListView 的左边距统一由 NotifyStyle.leftMargin 控制。
    • 同时,将 NotifyView.qml 中的 topMargin 从外层 NotifyCenter 移动到了内层 ListView 中。
    • main.qml 中的窗口宽度计算逻辑随之更新,加上了 NotifyStyle.leftMargin
  • 结论:逻辑正确。将布局边距从硬编码(Magic Number)改为集中管理的样式属性是正确的重构方向,且窗口宽度的计算公式也相应调整,确保了视觉布局的一致性。

2. 代码质量

  • 分析
    • 优点:消除了魔法数字。原本在 NotifyCenter.qml 中隐式存在的 topMargin: 10main.qml 中隐式假设的 contentPadding 作为左边距,现在被显式定义为 NotifyStyle.leftMargin。这提高了代码的可维护性,修改边距只需在一处进行。
    • 优点:引入了 import org.deepin.ds.notification,虽然在这个 Diff 中未直接显示其被引用,但考虑到后续可能需要访问该模块下的单例或类型,这是合理的准备。
    • 不足:在 panels/notification/center/package/main.qml 的第 66 行:
      width: NotifyStyle.contentItem.width + NotifyStyle.leftMargin +contentPadding * 2
      +contentPadding 前缺少空格,不符合 QML/Qt 的代码风格规范(通常操作符前后需加空格)。
  • 改进建议:修复格式问题,建议修改为:
    width: NotifyStyle.contentItem.width + NotifyStyle.leftMargin + contentPadding * 2

3. 代码性能

  • 分析
    • 布局属性的绑定从简单的数值变为属性绑定(如 NotifyStyle.leftMargin)。由于 NotifyStyle 是一个 QtObject,且其属性通常在初始化后不再改变,这种绑定的性能开销极小,几乎可以忽略不计。
    • ListViewanchorsmargin 变化会触发一次布局重计算,这在窗口初始化时发生,对运行时性能无影响。
  • 结论:性能表现良好,无明显隐患。

4. 代码安全

  • 分析
    • 新增的 NotifyStyle.leftMargin 是一个简单的整数属性,不涉及用户输入或外部数据注入,不存在安全风险。
    • 引入新的 import org.deepin.ds.notification 是安全的,前提是该模块是受信任的内部模块(从包名 org.deepin 看是可信的)。
  • 结论:代码安全,无风险。

总结与建议

这段代码是一次良好的 UI 布局重构,提升了代码的可维护性。唯一的小瑕疵是 main.qml 中的一处代码格式问题。

建议修改如下:

diff --git a/panels/notification/center/package/main.qml b/panels/notification/center/package/main.qml
index 11d5f8816..corrected_hash 100644
--- a/panels/notification/center/package/main.qml
+++ b/panels/notification/center/package/main.qml
@@ -63,7 +63,7 @@ Window {
     flags: Qt.Tool
 
     property int contentPadding: 20
-    width: NotifyStyle.contentItem.width + NotifyStyle.leftMargin +contentPadding * 2
+    width: NotifyStyle.contentItem.width + NotifyStyle.leftMargin + contentPadding * 2
     // height: 800
     DLayerShellWindow.layer: DLayerShellWindow.LayerOverlay
     DLayerShellWindow.anchors: DLayerShellWindow.AnchorRight | DLayerShellWindow.AnchorTop | DLayerShellWindow.AnchorBottom

除此之外,代码逻辑清晰,符合设计规范,可以合并。

@pengfeixx
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Feb 9, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit cc3bd20 into linuxdeepin:master Feb 9, 2026
8 of 11 checks passed
@pengfeixx pengfeixx deleted the fix-347237 branch February 9, 2026 07:53
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