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 dialog center #2428

Merged
merged 1 commit into from
Nov 23, 2024
Merged

fix: move burn dialog center #2428

merged 1 commit into from
Nov 23, 2024

Conversation

itsXuSt
Copy link
Contributor

@itsXuSt itsXuSt commented Nov 23, 2024

as above.

Log: as above.

Bug: https://pms.uniontech.com/bug-view-286669.html
Change-Id: Iabb35073da3fb42913dc7ee9af67362e1417ce24

as above.

Log: as above.

Bug: https://pms.uniontech.com/bug-view-286669.html
Change-Id: Iabb35073da3fb42913dc7ee9af67362e1417ce24
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. 代码重复:在showOpticalJobCompletionDialogshowOpticalJobFailureDialog两个函数中,d.moveToCenter();被重复添加。建议将这个调用移到这两个函数之外,统一管理,以减少代码重复。

  2. 翻译字符串tr("OK", "button")tr("Confirm", "button")中的翻译字符串"button"可能不是最佳选择,应该根据上下文选择更合适的翻译字符串。

  3. 按钮焦点设置d.getButton(0)->setFocus();d.getButton(1)->setFocus();在设置焦点时,应该考虑用户的习惯和期望,确保焦点设置不会干扰用户的操作。

  4. 对话框执行d.exec();在对话框显示后,应该确保对话框的执行不会阻塞其他线程或操作,如果需要,可以考虑使用异步方式执行对话框。

  5. 错误处理:在showOpticalJobFailureDialog函数中,只显示了错误对话框,但没有提供错误处理逻辑,例如记录错误日志或提供重试选项。

  6. 代码注释:代码中没有足够的注释来解释moveToCenter函数的作用和实现细节,这可能会对其他开发者理解代码造成困难。

综上所述,建议对代码进行重构,减少重复代码,优化翻译字符串,考虑用户操作习惯,确保对话框执行不会阻塞,并提供适当的错误处理和注释。

@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

@Johnson-zs
Copy link
Contributor

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Nov 23, 2024

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 5cdc9ba into linuxdeepin:master Nov 23, 2024
18 of 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