Repository navigation
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: zqq-dora 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 |
Reviewer's GuideUpdates Qt5 and Qt6 FloatingMessage implementations to correctly size notifications at 30px without a close button or 40px with one, expand for implicit content size, and eliminate invalid QML bindings. The change also centralizes the no-button height, applies capped system window-radius mapping, stabilizes Qt5 blur radius, standardizes icon sizing, and makes the Qt6 panelHeight override effective. Flow diagram for FloatingMessage sizingflowchart TD
Message[FloatingMessage]
Button{closeButton.active}
MinHeight[minimumHeight: 40]
MinNoButton[minimumHeightWithoutButton: 30]
Implicit[implicitHeight]
Panel[FloatingPanel.panelHeight]
Message --> Button
Button -->|true| MinHeight
Button -->|false| MinNoButton
MinHeight --> Panel
MinNoButton --> Panel
Implicit --> Panel
Panel -->|Math.max| Panel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi @zqq-dora. 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. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/qml/FloatingMessage.qml" line_range="40" />
<code_context>
contentItem: RowLayout {
- height: DS.Style.floatingMessage.minimumHeight
- width: Math.min(DS.Style.floatingMessage.maximumWidth, children.width + floatingPanel.leftPadding - floatingPanel.rightPadding)
+ height: Math.max(closeButton.active ? DS.Style.floatingMessage.minimumHeight : DS.Style.floatingMessage.minimumHeightWithoutButton, implicitHeight)
+ width: Math.min(DS.Style.floatingMessage.maximumWidth, implicitWidth + floatingPanel.leftPadding - floatingPanel.rightPadding)
spacing: 10
</code_context>
<issue_to_address>
**Message text elides too early**
When a Qt5 FloatingMessage uses content near its available width, `RowLayout.width` resolves to `implicitWidth - 2` because the 8 px left padding is offset by the 10 px right padding, leaving the loader two pixels short and making its label elide text earlier than necessary.
Size the row against the panel’s content area without letting the unequal left and right padding reduce its implicit width.
</issue_to_address>| contentItem: RowLayout { | ||
| height: DS.Style.floatingMessage.minimumHeight | ||
| width: Math.min(DS.Style.floatingMessage.maximumWidth, children.width + floatingPanel.leftPadding - floatingPanel.rightPadding) | ||
| height: Math.max(closeButton.active ? DS.Style.floatingMessage.minimumHeight : DS.Style.floatingMessage.minimumHeightWithoutButton, implicitHeight) |
There was a problem hiding this comment.
⚪ Low · Message text elides too early
When a Qt5 FloatingMessage uses content near its available width, RowLayout.width resolves to implicitWidth - 2 because the 8 px left padding is offset by the 10 px right padding, leaving the loader two pixels short and making its label elide text earlier than necessary.
Size the row against the panel’s content area without letting the unequal left and right padding reduce its implicit width.
Prompt for AI agents
In `src/qml/FloatingMessage.qml` at line 40:
**Message text elides too early**
When a Qt5 FloatingMessage uses content near its available width, `RowLayout.width` resolves to `implicitWidth - 2` because the 8 px left padding is offset by the 10 px right padding, leaving the loader two pixels short and making its label elide text earlier than necessary.
Size the row against the panel’s content area without letting the unequal left and right padding reduce its implicit width.
302e879 to
b8f44ff
Compare
| id: floatingPanel | ||
| property bool animationFinished: false | ||
| implicitWidth: DS.Style.control.contentImplicitWidth(floatingPanel) | ||
| panelHeight: Math.max(closeButton.active ? DS.Style.floatingMessage.minimumHeight : DS.Style.floatingMessage.minimumHeightWithoutButton, contentItem.height) |
There was a problem hiding this comment.
这个panelHeight是不是就是FloatingPanel这个控件的高度呀，
There was a problem hiding this comment.
是的，panelHeight 就是 FloatingPanel 这个控件的高度。之前 Qt6 的 FloatingPanel 没有暴露 panelHeight 属性，implicitHeight 直接写死了 DS.Style.floatingPanel.height（40px），所以 FloatingMessage 里对 panelHeight 赋值不生效。这次补上 panelHeight 属性并接到 InWindowBlur.implicitHeight，这样无按钮时 30px 的高度覆盖才能真正生效。
There was a problem hiding this comment.
那直接用Control提供的height是不是就行了，不需要新增panelHeight这个属性，
There was a problem hiding this comment.
好的，已改为直接使用 Control 提供的 height 属性，去掉了新增的 panelHeight。FloatingMessage 里现在直接设 height: Math.max(...)，FloatingPanel 的 InWindowBlur.implicitHeight 恢复为默认的 DS.Style.floatingPanel.height 作为隐式值，被 height 覆盖时自动生效。
| id: floatingPanel | ||
| implicitWidth: DS.Style.control.contentImplicitWidth(floatingPanel) | ||
| leftPadding: 10 | ||
| panelHeight: Math.max(closeButton.active ? DS.Style.floatingMessage.minimumHeight : DS.Style.floatingMessage.minimumHeightWithoutButton, contentItem.height) |
There was a problem hiding this comment.
不用维护qt5的qml代码样式，只用修改qt6下面的qml，
There was a problem hiding this comment.
好的，已经撤掉了 Qt5（src/qml/）下的所有改动，现在只保留 Qt6（qt6/src/qml/）的修改。
b8f44ff to
474fbfc
Compare
…fix invalid bindings The in-app notification (FloatingMessage) had several issues: 1. Height was always fixed at 40px regardless of whether a close button was present. Inline hardcoded 30 was used in three places without extracting it to a Style token. 2. children.height and children.width bindings in RowLayout were invalid (QML list property scalar access returns first child or 0), causing Math.max to always fall back to the constant with runtime warnings. 3. Floating panel corner radius was hardcoded to 14, not following the system window-radius setting unlike menu.radius. 4. Qt6 FloatingPanel lacked panelHeight property, so the height override in FloatingMessage was a no-op. 5. Icon sourceSize was 20 on Qt6 but 16 on Qt5. Changes: - Add minimumHeightWithoutButton (30) style token, use it for no-button notifications (40px with button). - Adapt floatingPanel.radius to system windowRadius: none->0, small->8, medium->12, large->12 (capped). - Replace children.height/width with implicitHeight/implicitWidth. - Unify icon sourceSize to 16x16. - Use Control height instead of custom panelHeight property so the height override takes effect. Only Qt6 QML is modified per reviewer feedback; Qt5 code is left unchanged. Also update SPDX copyright year to 2026.
474fbfc to
6bc8fde
Compare
Background
The in-app notification (
FloatingMessage) had several issues:minimumHeight(40px) regardless of whether a close button was present. A previous attempt hardcoded30inline in three places without extracting it to a Style token.children.heightand onechildren.widthbindings in theRowLayoutwere invalid: accessing a scalar property on a QML list (children) only returns the first child's value (or 0), so theMath.max(..., children.height)always fell back to the constant, and a runtime binding-loop warning was emitted.14and did not follow the system window-radius setting, unlikemenu.radiuswhich already usedD.DTK.platformTheme.windowRadius.FloatingPanellacked apanelHeightproperty, so thepanelHeightoverride inFloatingMessagewas a no-op dynamic property — the panel height always stayed atDS.Style.floatingPanel.height(40px).FloatingPanel.blurRadiuswas bound tofloatingPanel.radius, so making the radius dynamic would cause the blur amount to change with the system setting.sourceSizewas20on Qt6 but16on Qt5, an inconsistency.Changes
src/qml/FlowStyle.qml & qt6/src/qml/FlowStyle.qml
property int minimumHeightWithoutButton: 30tofloatingMessage, replacing the inline hardcoded30.floatingPanel.radiusfrom14toD.DTK.platformTheme.windowRadius < 0 ? 12 : Math.min(D.DTK.platformTheme.windowRadius, 12), adapting to the system window-radius setting with the mapping: none(0)->0, small(8)->8, medium(12)->12, large(18)->12 (capped).src/qml/FloatingMessage.qml & qt6/src/qml/FloatingMessage.qml
FloatingPanel.panelHeight,RowLayout.height,contentLoader.Layout.preferredHeight) now useMath.max(closeButton.active ? minimumHeight : minimumHeightWithoutButton, implicitHeight), so no-button notifications are 30px and with-button notifications are 40px, expanding for taller content.children.heightwithimplicitHeightandchildren.widthwithimplicitWidthin theRowLayout.sourceSizeto16x16on both Qt5 and Qt6.panelHeightassignment toFloatingPanel.qt6/src/qml/FloatingPanel.qml
property int panelHeight: DS.Style.floatingPanel.heightand wiredInWindowBlur.implicitHeighttocontrol.panelHeight, mirroring the Qt5 version so the height override takes effect.src/qml/FloatingPanel.qml
blurRadiusfrom the now-dynamicfloatingPanel.radius, pinning it to a fixed14to prevent blur jitter when the system radius changes.Effect
duration >= 0) are now 30px tall; notifications with a close button (persistent,duration < 0) remain 40px. Taller content expands the panel as expected.children.height/children.widthbindings no longer emit runtime warnings; the panel correctly sizes itself usingimplicitHeight/implicitWidth.背景
应用内通知（
FloatingMessage）存在以下问题：minimumHeight（40px），不区分是否有关闭按钮。此前有一处改动在三处内联硬编码了30，但没有提取为样式常量。RowLayout中的两处children.height和一处children.width绑定无效：对 QML 列表属性（children）取标量只会返回第一个子项的值（或 0），导致Math.max(..., children.height)始终退回常量，并产生运行时绑定告警。14，未跟随系统窗口圆角设置，而menu.radius已经使用了D.DTK.platformTheme.windowRadius。FloatingPanel缺少panelHeight属性，导致FloatingMessage中对panelHeight的赋值是不生效的动态属性，面板高度始终为DS.Style.floatingPanel.height（40px）。FloatingPanel.blurRadius绑定到floatingPanel.radius，圆角改为动态后模糊量会随系统设置变化。sourceSize为20，而 Qt5 为16，两版不一致。改动
src/qml/FlowStyle.qml 与 qt6/src/qml/FlowStyle.qml
floatingMessage中新增property int minimumHeightWithoutButton: 30，替换内联硬编码的30。floatingPanel.radius从14改为D.DTK.platformTheme.windowRadius < 0 ? 12 : Math.min(D.DTK.platformTheme.windowRadius, 12)，适配系统窗口圆角设置，映射关系为：无(0)->0，小(8)->8，中(12)->12，大(18)->12（封顶）。src/qml/FloatingMessage.qml 与 qt6/src/qml/FloatingMessage.qml
FloatingPanel.panelHeight、RowLayout.height、contentLoader.Layout.preferredHeight）改为Math.max(closeButton.active ? minimumHeight : minimumHeightWithoutButton, implicitHeight)，无按钮通知 30px、有按钮通知 40px，内容更高时自动撑开。children.height替换为implicitHeight、children.width替换为implicitWidth。sourceSize统一为16x16。FloatingPanel的panelHeight赋值。qt6/src/qml/FloatingPanel.qml
property int panelHeight: DS.Style.floatingPanel.height，InWindowBlur.implicitHeight改为control.panelHeight，与 Qt5 版本对齐，使高度覆盖生效。src/qml/FloatingPanel.qml
blurRadius与动态floatingPanel.radius解耦，固定为14，避免圆角变化时模糊量抖动。效果
duration >= 0）高度为 30px；有关闭按钮的通知（常驻，duration < 0）保持 40px。内容更高时面板自动撑开。children.height/children.width绑定不再产生运行时告警，面板通过implicitHeight/implicitWidth正确计算自身尺寸。Summary by Sourcery
Fix FloatingMessage sizing, layout bindings, and system-radius integration across Qt5 and Qt6.
Bug Fixes:
Enhancements: