fix: improve notification center margin calculation#1363
fix: improve notification center margin calculation#1363deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideRefactors notification center margin handling to use dynamic, screen-aware dock geometry with device-pixel–aware calculations and stored margin properties, fixing positioning issues and flicker when the dock is shown/hidden or moved. Sequence diagram for dynamic dock geometry margin updatessequenceDiagram
actor User
participant DockApplet
participant NotificationCenterWindow as RootWindow
participant DLayerShellWindow
participant Screen
User->>DockApplet: Change dock visibility/position
DockApplet->>DockApplet: Update frontendWindowRect
DockApplet-->>NotificationCenterWindow: frontendWindowRectChanged(frontendWindowRect)
NotificationCenterWindow->>NotificationCenterWindow: updateMargins(frontendRect)
NotificationCenterWindow->>DockApplet: Read screenName, position, hideState, dockSize
NotificationCenterWindow->>Screen: Read name, devicePixelRatio, virtualX, virtualY, width, height
NotificationCenterWindow->>NotificationCenterWindow: Compute dockGeometry and screenGeometry
alt Dock on same screen and visible
alt DockPositionTop
NotificationCenterWindow->>NotificationCenterWindow: Set topMarginValue
else DockPositionRight
NotificationCenterWindow->>NotificationCenterWindow: Set rightMarginValue
else DockPositionBottom
NotificationCenterWindow->>DockApplet: Read hideState and dockSize
NotificationCenterWindow->>NotificationCenterWindow: Set bottomMarginValue with anti_flicker_logic
end
else Dock on other screen or not valid
NotificationCenterWindow->>NotificationCenterWindow: Reset all margin values to 0
end
NotificationCenterWindow-->>DLayerShellWindow: topMargin, rightMargin, bottomMargin bindings updated
DLayerShellWindow->>User: Notification center repositions without flicker
Class diagram for updated notification center margin handlingclassDiagram
class RootWindow {
+int topMarginValue
+int rightMarginValue
+int bottomMarginValue
+updateMargins(frontendRect)
+onRequestClosePopup()
+onFrontendWindowRectChanged(frontendWindowRect)
}
class DockApplet {
+string screenName
+int hideState
+int position
+int dockSize
+rect frontendWindowRect
+frontendWindowRectChanged(frontendWindowRect)
}
class Screen {
+string name
+double devicePixelRatio
+int virtualX
+int virtualY
+int width
+int height
}
class DLayerShellWindow {
+int topMargin
+int rightMargin
+int bottomMargin
+int exclusionZone
+int keyboardInteractivity
}
RootWindow --> DockApplet : reads_properties_and_listens_to_signal
RootWindow --> Screen : uses_for_geometry_and_dpr
RootWindow --> DLayerShellWindow : binds_margin_properties
DLayerShellWindow ..> Screen : positioned_on
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The
updateMarginsfunction repeats the pattern of resetting all margin values to 0 in several early-return branches; consider extracting this into a small helper (e.g.resetMargins()) to reduce duplication and make the intent clearer. - The
switch (dockPosition)uses raw numeric values (0/1/2) with comments forDOCK_TOP/RIGHT/BOTTOM; if the dock API exposes an enum or constants for these positions, using them instead of magic numbers would make the code more self-documenting and less error-prone. - For the bottom dock you explicitly gate the margin on
hideState === Dock.Hide, but for top and right positions the hide state is not considered (only geometry); it may be worth clarifying or aligning this behavior so that margin handling for hidden/auto-hidden docks is consistent across all positions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `updateMargins` function repeats the pattern of resetting all margin values to 0 in several early-return branches; consider extracting this into a small helper (e.g. `resetMargins()`) to reduce duplication and make the intent clearer.
- The `switch (dockPosition)` uses raw numeric values (0/1/2) with comments for `DOCK_TOP/RIGHT/BOTTOM`; if the dock API exposes an enum or constants for these positions, using them instead of magic numbers would make the code more self-documenting and less error-prone.
- For the bottom dock you explicitly gate the margin on `hideState === Dock.Hide`, but for top and right positions the hide state is not considered (only geometry); it may be worth clarifying or aligning this behavior so that margin handling for hidden/auto-hidden docks is consistent across all positions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
TAG Bot New tag: 2.0.21 |
There was a problem hiding this comment.
Pull request overview
This PR improves the notification center's margin calculation to properly account for the dock's position and visibility. The change replaces a simple position-based margin calculation with a comprehensive geometry-based approach that handles device pixel ratios and dynamic dock state changes.
Key changes:
- Replaced
windowMargin(position)function withupdateMargins(frontendRect)that uses dock geometry instead of simple position checks - Added property storage for margin values (topMarginValue, rightMarginValue, bottomMarginValue) to avoid recalculation on every access
- Implemented device pixel ratio conversion to translate dock geometry from device pixels to logical pixels for accurate positioning
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
TAG Bot New tag: 2.0.22 |
|
TAG Bot New tag: 2.0.23 |
|
TAG Bot New tag: 2.0.24 |
|
TAG Bot New tag: 2.0.25 |
|
TAG Bot New tag: 2.0.26 |
|
TAG Bot New tag: 2.0.27 |
|
TAG Bot New tag: 2.0.28 |
1c076fd to
4f2f176
Compare
62b019d to
4de5744
Compare
4de5744 to
11765c2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
11765c2 to
884c71d
Compare
884c71d to
3cbfc70
Compare
1. Fixed notification center window positioning logic to use frontendWindowRect for accurate dock geometry 2. Added special handling for bottom dock to avoid jitter by checking hideState 3. Improved margin calculation by considering actual visible dock area on screen edges 4. Added property binding for frontendRect updates when dock geometry changes Log: Fixed notification center positioning issues near dock edges Influence: 1. Test notification center positioning when dock is at top/right/bottom positions 2. Verify no jitter when dock is at bottom with hide/show states 3. Test positioning accuracy when dock spans multiple screens 4. Verify correct margin calculation when dock is partially off-screen 5. Test notification center behavior during dock position changes 6. Verify frontendRect updates properly when dock geometry changes fix: 修复通知中心窗口定位问题 1. 修复通知中心窗口定位逻辑,使用 frontendWindowRect 获取准确的 dock 几 何信息 2. 为底部 dock 添加特殊处理,通过检查 hideState 避免抖动问题 3. 改进边距计算,考虑屏幕边缘 dock 的实际可见区域 4. 添加 frontendRect 属性绑定,在 dock 几何信息变化时更新 Log: 修复通知中心在 dock 边缘的定位问题 Influence: 1. 测试通知中心在 dock 位于顶部/右侧/底部时的定位 2. 验证 dock 在底部隐藏/显示状态时无抖动 3. 测试 dock 跨多个屏幕时的定位准确性 4. 验证 dock 部分超出屏幕时的边距计算正确性 5. 测试 dock 位置变化时通知中心的行为 6. 验证 dock 几何信息变化时 frontendRect 正确更新 PMS: BUG-340795
3cbfc70 to
aa642a8
Compare
deepin pr auto review这段代码主要对 Dock 面板(DockPanel)的窗口矩形计算逻辑进行了重构,并修改了通知中心(Notification Center)中关于 Dock 边距的计算逻辑。 以下是针对语法逻辑、代码质量、代码性能和代码安全的详细审查意见: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进建议
总结:这次重构通过缓存计算结果提升了 C++ 端的性能,但引入了状态管理的复杂性(需要确保缓存始终有效)。QML 端的计算逻辑更加精细,但需注意边界情况和性能开销。最重要的是补全成员变量的初始化并修正 QML 中底部 Dock 的计算逻辑。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
|
/forcemerge |
|
This pr force merged! (status: behind) |
Log: Fixed notification center positioning issues when dock is visible
fix: 改进通知中心边距计算
Log: 修复任务栏可见时通知中心定位问题
PMS: BUG-340795
剪切板相关修复: #1308 linuxdeepin/dde-clipboard#215