-
Notifications
You must be signed in to change notification settings - Fork 60
fix: Fix the issue of the close button and shadow being cut off #1438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts 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
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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
clipon theListViewmay 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.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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个好像会导致之前修的那一个bug有问题
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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 pr auto review代码审查报告经过对提供的 Git Diff 进行仔细审查,以下是从语法逻辑、代码质量、代码性能和代码安全四个方面的详细分析和改进建议: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与建议这段代码是一次良好的 UI 布局重构,提升了代码的可维护性。唯一的小瑕疵是 建议修改如下: 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除此之外,代码逻辑清晰,符合设计规范,可以合并。 |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
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: