-
Notifications
You must be signed in to change notification settings - Fork 502
Fix Error problem and enhanced the error message logic #3163
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
Conversation
…ects installation paths and improve path sorting
…taller # Conflicts: # viewer/CMakeLists.txt
…ng command execution flow
…ng command execution flow
…ng command execution flow
…ent/libpag into feature/codywwang_installer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes several issues related to error message display and export window management. The changes address problems where error messages were showing as empty, export failure page navigation was incorrect, and the export panel couldn't be reopened after multiple uses.
Key Changes:
- Enhanced error message display logic to fallback to alert info data when primary error message is empty
- Added proper cleanup via
onWindowClosing()calls on error paths inExportWindow::init() - Implemented cancelExport signal handling to properly close windows when exports are cancelled by users
- Refactored event loop management by replacing
runEventLoopIfNeeded()with directapp->exec()calls
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| exporter/src/ui/exportPanel/CompositionsModel.h | Added cancelExport signal for handling user-initiated export cancellations |
| exporter/src/ui/exportPanel/CompositionsModel.cpp | Modified export loop to detect cancellation and emit cancelExport signal |
| exporter/src/ui/export/ExportWindow.cpp | Added onWindowClosing() calls to properly cleanup when initialization fails |
| exporter/src/ui/WindowManager.cpp | Simplified event loop management by removing runEventLoopIfNeeded() and calling exec() directly |
| exporter/src/export/PAGExport.h | Added canceled flag to track when exports are cancelled by user |
| exporter/src/export/PAGExport.cpp | Set canceled flag when user dismisses alert dialogs |
| exporter/assets/qml/PAGListView.qml | Connected ErrorTextArea targetListView property to enable proper scrolling |
| exporter/assets/qml/ExportCompositionsProgress.qml | Added Connections to handle cancelExport signal and close window |
| exporter/assets/qml/ErrorTextArea.qml | Made targetListView property configurable with null safety checks |
| exporter/assets/qml/AlertWarning.qml | Simplified button click handlers by removing unnecessary model checks |
| exporter/assets/qml/AlertError.qml | Enhanced error message display with fallback logic and reactive updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Connections { | ||
| target: alertInfoModel | ||
| function onErrorMessageChanged() { | ||
| mainWindow.errorMessage = alertInfoModel ? alertInfoModel.errorMessage : "" | ||
| } | ||
| function onAlertInfoChanged() { | ||
| mainWindow.errorMessage = alertInfoModel ? alertInfoModel.errorMessage : "" | ||
| } | ||
| } |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Connections target is set to alertInfoModel, but there's no null check. If alertInfoModel is null or undefined, these signal handlers will fail silently or cause errors. Consider adding an enabled property or checking if the target exists before attempting to connect.
| Connections { | ||
| target: exportCompositionsWindow | ||
| onCancelExport: { | ||
| window.close(); | ||
| } | ||
| } |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Connections target is set to exportCompositionsWindow without a null check. If exportCompositionsWindow is null (which it can be according to line 131's check), the signal connection will fail. Consider adding an enabled property that checks if the target exists, or use the function syntax with null checking inside the handler.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3163 +/- ##
==========================================
+ Coverage 77.26% 77.27% +0.01%
==========================================
Files 413 413
Lines 21997 21997
Branches 6282 6282
==========================================
+ Hits 16995 16998 +3
Misses 3798 3798
+ Partials 1204 1201 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1、修复错误提示页面消息显示为空问题。
2、修复导出失败时页面回退逻辑。
3、修复导出页面多次打开后无法打开问题。