sync: from linuxdeepin/dde-session-shell #487
Merged
+13
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Synchronize source files from linuxdeepin/dde-session-shell. Source-pull-request: linuxdeepin/dde-session-shell#56
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts DLineEditEx placeholder text rendering to prevent wrapping, add ellipsis when truncated, and show a tooltip with the full placeholder when truncation occurs. Flow diagram for DLineEditEx::paintEvent placeholder renderingflowchart TD
A[paintEvent called] --> B{lineEdit has no text and placeholder is visible}
B -->|No| Z[Call base QWidget::paintEvent and return]
B -->|Yes| C[Create QPainter and set pen color]
C --> D[Create QTextOption]
D --> E[Set alignment to Qt::AlignCenter]
E --> F[Set wrap mode to QTextOption::NoWrap]
F --> G[Create QFontMetrics from painter font]
G --> H[Get placeholderText from lineEdit]
H --> I[Compute elidedText using fm.elidedText with Qt::ElideRight and rect width]
I --> J[Draw text in rect with Qt::AlignCenter and Qt::TextSingleLine using elidedText]
J --> K{elidedText != placeholderText}
K -->|Yes| L[setToolTip with full placeholderText]
K -->|No| M[setToolTip with empty QString]
L --> N[Call base QWidget::paintEvent]
M --> N[Call base QWidget::paintEvent]
N --> O[Return from paintEvent]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 left some high level feedback:
- Avoid mutating widget state (setting tooltips) inside paintEvent; consider updating the tooltip only when the placeholder text or geometry changes (e.g., in resizeEvent or when the placeholder changes) and caching the elision result to keep paintEvent side-effect free and cheaper.
- The QTextOption instance is no longer used after switching to drawText with flags, so it can be removed to simplify the paintEvent implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid mutating widget state (setting tooltips) inside paintEvent; consider updating the tooltip only when the placeholder text or geometry changes (e.g., in resizeEvent or when the placeholder changes) and caching the elision result to keep paintEvent side-effect free and cheaper.
- The QTextOption instance is no longer used after switching to drawText with flags, so it can be removed to simplify the paintEvent implementation.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
deepin pr auto review这段代码主要针对 1. 语法逻辑审查
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
改进后的代码建议结合以上分析，建议对代码进行如下调整。为了性能和最佳实践，我们将 Tooltip 的逻辑移出 方案 A：最小改动（保留在 paintEvent 中，但增加判断） void DLineEditEx::paintEvent(QPaintEvent *event)
{
Q_UNUSED(event);
// ... 其他绘制逻辑 ...
if (!lineEdit()->text().isEmpty()) {
QWidget::paintEvent(event);
return;
}
QPainter pa(this);
// ... 设置颜色等 ...
QTextOption option;
option.setAlignment(Qt::AlignCenter);
QFontMetrics fm(pa.font());
const QString &placeholderText = lineEdit()->placeholderText();
// 计算省略文本
QString elidedText = fm.elidedText(placeholderText, Qt::ElideRight, rect().width());
// 绘制文本：使用 Qt::TextSingleLine 标志即可，不需要 option.setWrapMode
pa.drawText(rect(), Qt::AlignCenter | Qt::TextSingleLine, elidedText);
// 优化：仅当 Tooltip 内容确实需要改变时才设置
// 注意：toolTip() 返回的是当前显示的 tooltip，如果用户手动设置了其他 tooltip，这里可能会覆盖
// 更好的做法是检查是否是因为 placeholder 引起的 tooltip，但这需要额外的成员变量标记
const QString currentToolTip = toolTip();
if (elidedText != placeholderText) {
if (currentToolTip != placeholderText) {
setToolTip(placeholderText);
}
} else {
// 只有当当前的 tooltip 确实是我们之前设置的 placeholder 时才清空
// 这里的逻辑比较复杂，简单起见，如果不想覆盖用户设置，可以不做清空操作，或者只在特定条件下清空
if (currentToolTip == placeholderText) {
setToolTip(QString());
}
}
QWidget::paintEvent(event);
}方案 B：最佳实践（将逻辑分离，推荐） 建议在类中增加一个槽函数或私有方法来处理 Tooltip，在 // 在头文件中声明
protected:
void resizeEvent(QResizeEvent *event) override;
// 如果 placeholderText 是动态设置的，还需要重写相关 set 方法或监听信号
private:
void updatePlaceholderTooltip();
// 在源文件中实现
void DLineEditEx::updatePlaceholderTooltip()
{
if (lineEdit()->text().isEmpty()) {
QFontMetrics fm(font());
const QString &placeholderText = lineEdit()->placeholderText();
QString elidedText = fm.elidedText(placeholderText, Qt::ElideRight, width());
if (elidedText != placeholderText) {
setToolTip(placeholderText);
} else {
setToolTip(QString());
}
} else {
// 如果有文本，通常不需要显示 placeholder 的 tooltip
// 除非你想保留用户手动设置的 tooltip
if (toolTip() == lineEdit()->placeholderText()) {
setToolTip(QString());
}
}
}
void DLineEditEx::resizeEvent(QResizeEvent *event)
{
QLineEdit::resizeEvent(event);
updatePlaceholderTooltip();
}
void DLineEditEx::paintEvent(QPaintEvent *event)
{
// ... 原有的绘制逻辑 ...
if (!lineEdit()->text().isEmpty()) {
QWidget::paintEvent(event);
return;
}
QPainter pa(this);
// ... 设置颜色 ...
// 简化的绘制逻辑
QFontMetrics fm(pa.font());
const QString &placeholderText = lineEdit()->placeholderText();
QString elidedText = fm.elidedText(placeholderText, Qt::ElideRight, rect().width());
pa.drawText(rect(), Qt::AlignCenter | Qt::TextSingleLine, elidedText);
QWidget::paintEvent(event);
}总结原代码在功能上是可以工作的，但在性能（频繁计算和设置属性）和架构（在绘制函数中处理逻辑）上有改进空间。采用方案 B 将 Tooltip 更新逻辑移至 |
yixinshark
approved these changes
Feb 9, 2026
Contributor
Author
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot, yixinshark 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Synchronize source files from linuxdeepin/dde-session-shell.
Source-pull-request: linuxdeepin/dde-session-shell#56
Summary by Sourcery
Enhancements: