fix: center dialog title with vertical offset#620
Conversation
|
Hi @MyLeeJiEun. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideCenters the dialog window title by replacing the plain title string with a styled D.Label in DialogTitleBar, applying horizontal centering, a vertical downward offset, and a max-width constraint to avoid overlap with side buttons. 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 found 2 issues, and left some high level feedback:
- The new D.Label references
titleBar.fontandtitleBar.palette, buttitleBaris not defined in this scope of DialogWindow.qml; consider binding to a property that is actually available here (e.g., fromcontrolorDialogTitleBar's exposed properties) to avoid runtime errors. - The hard-coded constants
verticalCenterOffset: 30andparent.width - 120are magic numbers that may not adapt well to different title bar layouts or DPI; it would be more robust to derive these offsets from the actual title bar height and side button widths or expose them as configurable properties.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new D.Label references `titleBar.font` and `titleBar.palette`, but `titleBar` is not defined in this scope of DialogWindow.qml; consider binding to a property that is actually available here (e.g., from `control` or `DialogTitleBar`'s exposed properties) to avoid runtime errors.
- The hard-coded constants `verticalCenterOffset: 30` and `parent.width - 120` are magic numbers that may not adapt well to different title bar layouts or DPI; it would be more robust to derive these offsets from the actual title bar height and side button widths or expose them as configurable properties.
## Individual Comments
### Comment 1
<location path="qt6/src/qml/DialogWindow.qml" line_range="76-77" />
<code_context>
+ anchors.verticalCenterOffset: 30 // 正数向下移,根据视觉效果调整
+
+ // --- 样式设置 ---
+ font: titleBar.font
+ palette: titleBar.palette
+
+ // 限制最大宽度,避免文字太长冲到左右两边的按钮或图标
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `titleBar` here is likely invalid, since ids are not visible across component boundaries.
Within `sourceComponent: DialogTitleBar { ... }`, the `titleBar` id defined inside `DialogTitleBar` is not in scope—ids are local to their own component. This will resolve to an undefined or incorrect object. To reuse the title bar styling, either bind to something in scope (e.g. `parent.font` / `parent.palette`) or have `DialogTitleBar` expose the needed values via properties (e.g. `property font titleFont`, `property palette titlePalette`) and bind to those instead.
</issue_to_address>
### Comment 2
<location path="qt6/src/qml/DialogWindow.qml" line_range="80" />
<code_context>
+ palette: titleBar.palette
+
+ // 限制最大宽度,避免文字太长冲到左右两边的按钮或图标
+ width: Math.min(implicitWidth, parent.width - 120)
+ elide: Text.ElideRight
+ horizontalAlignment: Text.AlignHCenter
</code_context>
<issue_to_address>
**issue (bug_risk):** Width calculation can become negative when `parent.width` is small.
When `parent.width < 120`, `parent.width - 120` becomes negative, so `width` can be set to a negative value, causing layout issues. Consider clamping the available width to zero, e.g. `width: Math.min(implicitWidth, Math.max(parent.width - 120, 0))`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
7f5bc69 to
4a7b979
Compare
2ca5dac to
4e16df3
Compare
Replace plain title text with a centered D.Label in DialogTitleBar Log: Center the dialog window title with downward offset fix: 对话框标题替换为 D.Label 组件 将 DialogTitleBar 中的纯文本标题替换为 D.Label 组件 Log: 将对话框窗口标题居中并设置下移偏移 PMS: BUG-361279
4e16df3 to
1c06ed0
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, MyLeeJiEun 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 |
Log: Center the dialog window title with downward offset and width limit
fix: 对话框标题居中并添加垂直偏移
Log: 将对话框窗口标题居中并设置下移偏移和宽度限制
PMS: BUG-361279
Summary by Sourcery
Bug Fixes: