Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: move burn confirm dialog center. #2533

Merged
merged 1 commit into from
Dec 31, 2024
Merged

Conversation

itsXuSt
Copy link
Contributor

@itsXuSt itsXuSt commented Dec 31, 2024

show dialogs in cursor screen's center.

Log: as above.

show dialogs in cursor screen's center.

Log: as above.
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码重复:在showOpticalJobCompletionDialogshowOpticalJobFailureDialogshowOpticalDumpISOSuccessDialogshowOpticalDumpISOFailedDialog四个函数中,d.move的调用是重复的。可以考虑将这个逻辑提取到一个单独的函数中,以减少代码重复。

  2. 硬编码的尺寸:在d.move调用中,使用了硬编码的尺寸(d.width() / 2d.height() / 2)。如果窗口的尺寸发生变化,这个逻辑可能不再准确。建议使用更灵活的方法来计算窗口的位置,例如使用窗口的sizeHint()方法。

  3. 缺少错误处理:在WindowUtils::cursorScreen()调用中,如果当前没有屏幕可用,这个函数可能会返回nullptr。在调用geometry().center()之前,应该检查返回的屏幕对象是否为nullptr,以避免潜在的空指针异常。

  4. 国际化:在d.addButtond.setDefaultButton调用中,使用了硬编码的字符串(如"OK", "Confirm")。建议使用tr函数进行国际化处理,以确保应用程序在不同语言环境下的正确显示。

  5. 代码风格:在d.move调用中,使用了两个连续的加号(+)来连接字符串。虽然这在C++中是合法的,但建议使用QString+运算符来连接字符串,以提高代码的可读性。

  6. 未使用的头文件:在代码的顶部,包含了<QScreen>头文件,但在代码中并没有使用到。建议移除未使用的头文件,以保持代码的整洁。

综上所述,建议对代码进行重构,以提高代码的可维护性和可读性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: itsXuSt, Johnson-zs

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

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

@itsXuSt
Copy link
Contributor Author

itsXuSt commented Dec 31, 2024

/merge

@deepin-bot deepin-bot bot merged commit 6622166 into linuxdeepin:master Dec 31, 2024
20 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