Skip to content

Conversation

@dengzhongyuan365-dev
Copy link
Contributor

  • Refactor position update in FindWidget to account for parent window dimensions.
  • Ensure correct positioning relative to the parent window or fallback to screen geometry if no parent is present.

This change enhances the usability of the FindWidget by ensuring it appears correctly in relation to its parent window.

bug: https://pms.uniontech.com/bug-view-339997.html

- Refactor position update in FindWidget to account for parent window dimensions.
- Ensure correct positioning relative to the parent window or fallback to screen geometry if no parent is present.

This change enhances the usability of the FindWidget by ensuring it appears correctly in relation to its parent window.

bug: https://pms.uniontech.com/bug-view-339997.html
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个diff变更进行详细分析:

  1. 代码变更内容:

    • 删除了获取主屏幕几何信息的代码行
    • 将定位逻辑改为使用父窗口的宽度作为参考
  2. 代码质量分析:
    优点:

    • 简化了代码,减少了对QGuiApplication的依赖
    • 使用父窗口的尺寸作为参考更符合窗口组件的层级关系

    潜在问题:

    • 没有检查m_parentWidget是否为空指针,可能导致程序崩溃
    • 如果父窗口尺寸小于查找窗口,可能导致查找窗口显示在屏幕外
  3. 改进建议:

void FindWidget::updatePosition()
{
    qCDebug(appLog) << "Updating find widget position, yOffset:" << m_yOff;
    
    // 添加空指针检查
    if (!m_parentWidget) {
        qCWarning(appLog) << "Parent widget is null";
        return;
    }
    
    // 确保查找窗口不会超出父窗口范围
    int parentWidth = m_parentWidget->width();
    int widgetWidth = this->width();
    int x = qMax(10, parentWidth - widgetWidth - 10);
    
    this->move(x, m_yOff + 10);
    this->show();
    this->raise();
}
  1. 安全性考虑:

    • 添加了空指针检查,避免潜在的崩溃风险
    • 使用qMax确保窗口不会显示在负坐标位置
    • 保持了原有的日志输出,便于调试
  2. 性能分析:

    • 移除了对QGuiApplication::primaryScreen()的调用,减少了系统调用
    • 整体性能影响很小,主要是提高了代码的健壮性
  3. 其他建议:

    • 考虑添加边界检查,确保y坐标也不会超出父窗口范围
    • 可以考虑使用QRect来处理位置计算,使代码更清晰
    • 建议添加注释说明10这个边距值的用途

这个修改总体上是好的,但需要增加一些安全检查来提高代码的健壮性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

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

@dengzhongyuan365-dev
Copy link
Contributor Author

/forcenerge

@dengzhongyuan365-dev
Copy link
Contributor Author

/forcemerge

@deepin-bot deepin-bot bot merged commit 93c28c5 into linuxdeepin:master Nov 13, 2025
6 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