sync: from linuxdeepin/dde-session-shell - #538
deepin-ci-robot wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot 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 guide (collapsed on small PRs)Reviewer's GuideSynchronizes placeholder rendering with the upstream implementation by calculating an icon-safe text region from QLineEdit margins before eliding and painting the placeholder, preventing overlap with trailing icons while preserving truncation tooltips. Flow diagram for icon-safe placeholder renderingflowchart TD
A[Placeholder paint event] --> B[Read QLineEdit geometry and textMargins]
B --> C[Calculate icon-safe textRect]
C --> D[Elide placeholder to textRect width]
D --> E[Paint placeholder in textRect]
E --> F{Text was elided}
F -->|Yes| G[Show full placeholder in tooltip]
F -->|No| H[Finish rendering]
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/widgets/dlineeditex.cpp" line_range="167-169" />
<code_context>
+ // 避免占位文本与右侧图标(大写状态、密码显示、密码提示等)重叠
+ QRect leRect = lineEdit()->geometry();
+ QMargins textMargins = lineEdit()->textMargins();
+ QRect textRect(leRect.x() + textMargins.left(), rect().y(),
+ leRect.width() - textMargins.left() - textMargins.right(),
+ rect().height());
+ QString elidedText = fm.elidedText(placeholderText, Qt::ElideRight, textRect.width());
+ pa.drawText(textRect, Qt::AlignCenter | Qt::TextSingleLine, elidedText);
</code_context>
<issue_to_address>
**issue (bug_risk):** The effective placeholder rectangle does not reserve the full space occupied by the embedded controls. In `AuthPassword`, the left text margin is set only to the caps-lock label width while the child layout also has a 10-pixel left content margin, so a sufficiently long centered placeholder extends into the caps-lock icon area.
**Triggers:** When the caps-lock indicator is visible and the placeholder is wide enough to reach the left side of the calculated text rectangle.
**Suggested fix:** Calculate the text rectangle from the actual embedded-control bounds, including the child layout's contents margins and spacing, or include those values in the line edit's text margins.
</issue_to_address>| QRect textRect(leRect.x() + textMargins.left(), rect().y(), | ||
| leRect.width() - textMargins.left() - textMargins.right(), | ||
| rect().height()); |
There was a problem hiding this comment.
issue (bug_risk): The effective placeholder rectangle does not reserve the full space occupied by the embedded controls. In AuthPassword, the left text margin is set only to the caps-lock label width while the child layout also has a 10-pixel left content margin, so a sufficiently long centered placeholder extends into the caps-lock icon area.
Triggers: When the caps-lock indicator is visible and the placeholder is wide enough to reach the left side of the calculated text rectangle.
Suggested fix: Calculate the text rectangle from the actual embedded-control bounds, including the child layout's contents margins and spacing, or include those values in the line edit's text margins.
|
TAG Bot New tag: 6.0.68 |
Synchronize source files from linuxdeepin/dde-session-shell. Source-pull-request: linuxdeepin/dde-session-shell#89
c34fad9 to
50aab15
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 良好 ✅ 通过 潜在问题:
建议: 1. 在 setPlaceholderTextFont 中添加防护:if (availWidth <= 0) { setFont(fontTmp); return; } 2. 代码质量 ✅评价: 良好 ✅ 通过 潜在问题:
建议: 建议将边距计算逻辑提取为私有辅助函数 effectiveTextMargins() 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 可将 QFontMetrics 构造移至循环外。此为既有问题,非本次变更引入。 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 本次变更不涉及安全敏感操作,无需安全加固。 💡 改进建议代码示例// 1. setPlaceholderTextFont 中添加 availWidth 防护
int availWidth = width() - textMargins.left() - textMargins.right()
- layoutMargins.left() - layoutMargins.right();
if (availWidth <= 0) {
setFont(fontTmp);
return;
}
while (QFontMetrics(fontTmp).boundingRect(text).width() > availWidth) {
// ...
}
// 2. paintEvent 中添加 textRect.width() 防护
QRect textRect(leRect.x() + leftOffset, rect().y(),
leRect.width() - leftOffset - rightOffset,
rect().height());
if (textRect.width() <= 0)
return;
QString elidedText = fm.elidedText(placeholderText, Qt::ElideRight, textRect.width());
// 3. 提取公共边距计算辅助函数
QMargins DLineEditEx::effectiveTextMargins() const
{
QMargins textMargins = lineEdit()->textMargins();
QMargins layoutMargins(0, 0, 0, 0);
if (auto *layout = lineEdit()->layout()) {
layoutMargins = layout->contentsMargins();
}
return QMargins(
textMargins.left() + layoutMargins.left(),
textMargins.top() + layoutMargins.top(),
textMargins.right() + layoutMargins.right(),
textMargins.bottom() + layoutMargins.bottom()
);
}本报告由 AI 代码审查工具自动生成 |
Synchronize source files from linuxdeepin/dde-session-shell.
Source-pull-request: linuxdeepin/dde-session-shell#89
Summary by Sourcery
Improve password placeholder rendering so text is sized and displayed within the available space alongside embedded controls.
Bug Fixes:
Enhancements: