fix(notification): Button text is truncated when hovering over "Clear all" in English environment with 20 font size#1431
Conversation
Reviewer's GuideImplements a fade-out opacity mask for the notification center header title so the text appears smoothly truncated when constrained by right-side buttons, only activating the effect when the text actually overflows. Flow diagram for enabling opacity mask on notification titleflowchart LR
A[Header layout created] --> B[Compute titleText.implicitWidth]
B --> C[Compute titleContainer.width]
C --> D{titleText.implicitWidth > titleContainer.width + 5}
D -- Yes --> E[Enable layer on titleContainer]
E --> F[Apply OpacityMask effect]
F --> G[Use Rectangle as maskSource]
G --> H[Apply horizontal gradient
0.0: opaque
0.9: opaque
1.0: transparent]
D -- No --> I[Disable layer on titleContainer]
I --> J[Render full title without fade]
H --> K[Render title with smooth right-side fade]
K --> L[Clear_all button remains fully visible]
J --> L
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 left some high level feedback:
- Consider avoiding the hard-coded
+ 5width threshold and the0.9gradient stop magic numbers by extracting them into named constants or properties to make the intent clearer and easier to tweak in future layout changes. - Using
OpacityMaskwithlayer.enabledcan be relatively expensive; if this header is used in a highly dynamic context, it may be worth profiling and, if needed, constraining whenlayer.enabledcan toggle (e.g., only on width/text changes) or exploring a lighter-weight fade implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider avoiding the hard-coded `+ 5` width threshold and the `0.9` gradient stop magic numbers by extracting them into named constants or properties to make the intent clearer and easier to tweak in future layout changes.
- Using `OpacityMask` with `layer.enabled` can be relatively expensive; if this header is used in a highly dynamic context, it may be worth profiling and, if needed, constraining when `layer.enabled` can toggle (e.g., only on width/text changes) or exploring a lighter-weight fade implementation.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
9222f66 to
d84deba
Compare
"Clear all" in English environment with 20 font size - Added OpacityMask to the title container for a smooth fade-out effect on the right side - Enabled gradient effect only when text overflows (using width + 5px threshold) - Made the last 10% of the text fade to transparent for a natural visual transition --- fix(notification): 英文环境且字体为20号,展开通知面板后,鼠标 hover到“清除全部”,此按钮未全部显示。 - 为标题容器添加 OpacityMask 实现右侧平滑淡出 - 仅在文字溢出时启用渐变(宽度 + 5px 阈值) - 最后 10% 文字渐变透明以实现自然视觉过渡 Log: 修复通知中心在英文环境且字体为20号,展开通知面板后,鼠标hover到“清除全部”,此按钮未全部显示问题 Influence: 通知中心-顶部按钮 PMS: BUG-335143
d84deba to
13eccb1
Compare
deepin pr auto review这段代码实现了在通知中心标题栏中,当空间不足时对标题文本应用渐隐淡出效果的功能。以下是对该代码的审查意见,包括语法逻辑、代码质量、性能和安全方面的建议: 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
5. 其他细节
改进后的代码示例import QtQuick
import QtQuick.Controls
import QtQuick.Layouts
import Qt5Compat.GraphicalEffects // 或者考虑迁移到 QtQuick.Effects
import org.deepin.dtk 1.0
import org.deepin.ds.notification
import org.deepin.ds.notificationcenter
// ... 其他代码 ...
RowLayout {
anchors.fill: parent
spacing: 8
// 定义常量或从样式文件中读取
readonly property real fadeStartRatio: 0.9
Item {
id: titleContainer
Layout.alignment: Qt.AlignLeft
Layout.leftMargin: 18
Layout.fillWidth: true
implicitHeight: titleText.implicitHeight
// 优化:确保容器宽度有效且文本实际宽度超出容器时才开启
// 使用 titleText.width 而非 implicitWidth 进行比较,因为 layout 可能会压缩 width
layer.enabled: width > 0 && titleText.width > width + 5
layer.effect: OpacityMask {
maskSource: Rectangle {
width: titleContainer.width
height: titleContainer.height
gradient: Gradient {
orientation: Gradient.Horizontal
GradientStop { position: 0.0; color: "#FFFFFFFF" }
GradientStop { position: titleContainer.fadeStartRatio; color: "#FFFFFFFF" }
GradientStop { position: 1.0; color: "#00FFFFFF" }
}
}
}
NotifyHeaderTitleText {
id: titleText
text: qsTr("Notification Center")
elide: Text.ElideNone
tFont: DTK.fontManager.t4
// 确保文本宽度不超过容器太多,避免布局计算过于复杂
// 如果需要限制最大宽度,可以在这里设置
}
}
// ... 其他按钮代码 ...
}
// ... 其他代码 ...总结这段代码整体逻辑清晰,能够实现预期的 UI 效果。主要的改进点在于优化 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, electricface 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) |
effect on the right side
5px threshold)
visual transition
fix(notification): 英文环境且字体为20号,展开通知面板后,鼠标
hover到“清除全部”,此按钮未全部显示。
Log: 修复通知中心在英文环境且字体为20号,展开通知面板后,鼠标hover到“清除全部”,此按钮未全部显示问题
Influence: 通知中心-顶部按钮
PMS: BUG-335143
Summary by Sourcery
Enhancements: