Skip to content

fix(usbselect): set fixed height for warning hint label to prevent la…#117

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
pppanghu77:bug3
May 12, 2026
Merged

fix(usbselect): set fixed height for warning hint label to prevent la…#117
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
pppanghu77:bug3

Conversation

@pppanghu77
Copy link
Copy Markdown

  • Set fixed height 50px for m_warningHint label in UsbSelectView to avoid layout jumping when warning text changes

修复(usbselect): 为警告提示标签设置固定高度以防止布局跳动

  • 在 UsbSelectView 中为 m_warningHint 警告提示标签设置固定高度 50px,避免警告文本变化时布局跳动

Log: 为 USB 选择界面的警告提示标签设置固定高度,防止文本内容变化引起的布局跳动
Bug: https://pms.uniontech.com/bug-view-241751.html

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @pppanghu77, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

…yout jumping

- Set fixed height 50px for m_warningHint label in UsbSelectView to avoid layout jumping when warning text changes

修复(usbselect): 为警告提示标签设置固定高度以防止布局跳动

- 在 UsbSelectView 中为 m_warningHint 警告提示标签设置固定高度 50px,避免警告文本变化时布局跳动

Log: 为 USB 选择界面的警告提示标签设置固定高度,防止文本内容变化引起的布局跳动
Bug: https://pms.uniontech.com/bug-view-241751.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff 输入。本次修改主要包含两处变更:更新了版权年份,以及为警告提示标签设置了固定高度。

以下是我对本次代码变更的详细审查意见:

1. 语法逻辑

  • 逻辑正确性:代码语法完全正确,setFixedHeight(50) 的调用位置也合理,放在属性设置之后、加入布局之前。
  • 潜在布局问题setFixedHeight 会同时设置控件的最小高度和最大高度为 50。在 Qt 的布局系统中,这意味着该控件将无法在垂直方向上随窗口拉伸或压缩。如果 m_warningHint 的文本内容较长并发生换行,固定高度可能会导致文本被截断,无法完整显示。

2. 代码质量

  • 魔法数字m_warningHint->setFixedHeight(50); 中的 50 是一个硬编码的“魔法数字”。在 UI 代码中,硬编码像素值通常是不推荐的,因为不同的屏幕 DPI 和系统字体缩放比例下,50px 的实际表现可能差异巨大。
  • 建议:建议将其提取为具名常量,或者使用与字体度量相关的动态计算值。例如:
    // 方案1:提取常量
    static const int kWarningHintHeight = 50;
    m_warningHint->setFixedHeight(kWarningHintHeight);
    
    // 方案2:基于字体高度动态计算(更推荐,能自适应DPI和字体缩放)
    int fontHeight = m_warningHint->fontMetrics().height();
    // 上下留出一定的 padding
    m_warningHint->setFixedHeight(fontHeight + 20); 

3. 代码性能

  • 性能影响微乎其微:设置固定高度只是修改了 Qt 布局系统中的尺寸约束,触发一次重新布局,对运行时性能没有负面影响。

4. 代码安全

  • 安全性良好:版权年份的修改和 UI 控件属性的设置不涉及任何外部输入、内存越界或权限问题,没有安全隐患。

💡 综合改进建议

考虑到 m_warningHint 是一个可能需要显示多语言文本(不同语言长度不同)的警告提示,强制固定高度可能会导致部分语言下文本显示不全。如果是为了保证布局美观,建议使用 setMinimumHeight 替代 setFixedHeight,这样既能保证控件不会过于矮小,又能在文本过长换行时自适应扩展高度。

改进后的代码示例:

// SPDX-FileCopyrightText: 2017 - 2026 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-only

// ... 其他代码 ...

font.setWeight(QFont::Normal);
m_warningHint->setFont(font);
m_warningHint->setAlignment(Qt::AlignCenter);

// 改进:使用动态计算高度或最小高度,避免硬编码和文本截断
constexpr int kWarningHintPadding = 20; // 上下内边距
int textHeight = m_warningHint->fontMetrics().height();
m_warningHint->setMinimumHeight(textHeight + kWarningHintPadding); 
// 如果你确实需要它严格保持50的高度且不换行,请确保在m_warningHint上设置换行禁用或省略号:
// m_warningHint->setWordWrap(false);
// m_warningHint->setFixedHeight(50);

QHBoxLayout* pWarningLayout = new QHBoxLayout;
pWarningLayout->setContentsMargins(30, 0, 30, 0);
pWarningLayout->addWidget(m_warningHint);

总结:代码本身没有语法错误或严重缺陷,主要问题在于硬编码像素值可能带来的跨平台/跨分辨率 UI 适配问题。建议优先考虑使用动态尺寸或 setMinimumHeight 以提升代码的健壮性和 UI 的兼容性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: max-lvs, pppanghu77

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pppanghu77
Copy link
Copy Markdown
Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented May 12, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 64b8284 into linuxdeepin:release/eagle May 12, 2026
19 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants