Fix: Add the functions of saving and restoring window status #320
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
-- Realize the saving and restoration of the window state, including maximizing the state and size. The relevant functions have been updated to ensure that the state is correctly saved when the window is maximized and the saved state is applied when restored. Log: Add the functions of saving and restoring window status Bug: https://pms.uniontech.com/bug-view-318459.html
Reviewer's GuideThis PR refactors window size persistence by introducing state-aware saveConfigWinState and restoreConfigWinState methods, replacing the old size-only storage, and ensuring correct handling of maximized windows via delayed state restoration. ER diagram for updated window state persistence keyserDiagram
SETTINGS {
string MAINWINDOW_WIDTH_NAME
string MAINWINDOW_HEIGHT_NAME
string MAINWINDOW_STATE_NAME
}
SETTINGS ||--o| "MainWindow" : persists
Class diagram for updated MainWindow window state managementclassDiagram
class MainWindow {
+void saveConfigWinState()
+void restoreConfigWinState()
+QSize getConfigWinSize()
-void saveConfigWinSize(int w, int h) // removed
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这个git diff进行代码审查：
void MainWindow::validateAndSaveWindowSize(int w, int h) {
int winWidth = w > MAINWINDOW_DEFAULTW ? w : MAINWINDOW_DEFAULTW;
int winHeight = h > MAINWINDOW_DEFAULTH ? h : MAINWINDOW_DEFAULTH;
m_pSettings->setValue(MAINWINDOW_HEIGHT_NAME, winHeight);
m_pSettings->setValue(MAINWINDOW_WIDTH_NAME, winWidth);
}
QTimer::singleShot(100, this, [this]() {
showMaximized();
});
Qt::WindowState savedState = static_cast<Qt::WindowState>(
m_pSettings->value(MAINWINDOW_STATE_NAME, Qt::WindowNoState).toInt());
// 确保状态值有效
if (savedState != Qt::WindowNoState && savedState != Qt::WindowMaximized) {
savedState = Qt::WindowNoState;
}
// Window state settings
#define MAINWINDOW_WIDTH_NAME "MainWindowWidthName"
#define MAINWINDOW_HEIGHT_NAME "MainWindowHeightName"
#define MAINWINDOW_STATE_NAME "MainWindowStateName"
总体来说，这个改动是正向的，提高了窗口状态管理的完整性和可靠性。代码结构清晰，逻辑合理。主要的改进空间在于代码复用和错误处理的完善。 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider leveraging QMainWindow::saveGeometry and restoreGeometry to simplify complete window state persistence instead of managing width, height, and state manually.
- Extend the save/restore logic to cover other window states (e.g., minimized or fullscreen) to avoid unexpected behavior when users use those modes.
- Using QTimer::singleShot(0) for maximizing the window may lead to race conditions; consider applying the window state in showEvent or after the window is fully initialized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider leveraging QMainWindow::saveGeometry and restoreGeometry to simplify complete window state persistence instead of managing width, height, and state manually.
- Extend the save/restore logic to cover other window states (e.g., minimized or fullscreen) to avoid unexpected behavior when users use those modes.
- Using QTimer::singleShot(0) for maximizing the window may lead to race conditions; consider applying the window state in showEvent or after the window is fully initialized.
## Individual Comments
### Comment 1
<location> `src/source/mainwindow.cpp:2464-2465` </location>
<code_context>
-void MainWindow::saveConfigWinSize(int w, int h)
+void MainWindow::saveConfigWinState()
{
- int winWidth = w > MAINWINDOW_DEFAULTW ? w : MAINWINDOW_DEFAULTW;
- int winHeight = h > MAINWINDOW_DEFAULTH ? h : MAINWINDOW_DEFAULTH;
- m_pSettings->setValue(MAINWINDOW_HEIGHT_NAME, winHeight);
- m_pSettings->setValue(MAINWINDOW_WIDTH_NAME, winWidth);
</code_context>
<issue_to_address>
**suggestion:** Minimum window size enforcement may not match setMinimumSize.
Ensure MAINWINDOW_DEFAULTW and MAINWINDOW_DEFAULTH match the values used in setMinimumSize to prevent inconsistent minimum window sizes.
Suggested implementation:
```cpp
#define MAINWINDOW_DEFAULTW 620
#define MAINWINDOW_DEFAULTH 300
```
If the definitions of `MAINWINDOW_DEFAULTW` and `MAINWINDOW_DEFAULTH` are not present in this file, but in a header (e.g., `mainwindow.h`), you should update them there instead. Also, check for any other places in the codebase where these constants are used to ensure consistency.
</issue_to_address>
### Comment 2
<location> `tests/UnitTest/src/source/ut_mainwindow.cpp:1558-1562` </location>
<code_context>
EXPECT_NE(m_tester->getConfigWinSize().width(), 0);
}
-TEST_F(UT_MainWindow, testsaveConfigWinSize)
-{
- m_tester->saveConfigWinSize(800, 600);
- EXPECT_EQ(m_tester->m_pSettings->value(MAINWINDOW_WIDTH_NAME).toInt(), 800);
- EXPECT_EQ(m_tester->m_pSettings->value(MAINWINDOW_HEIGHT_NAME).toInt(), 600);
-}
-
</code_context>
<issue_to_address>
**issue (testing):** Test for saveConfigWinSize was removed but not replaced with tests for saveConfigWinState.
Please add unit tests for saveConfigWinState and restoreConfigWinState to ensure window state and size are properly handled, including edge cases like missing size values and state transitions.
</issue_to_address>
### Comment 3
<location> `tests/UnitTest/src/source/ut_mainwindow.cpp:1555` </location>
<code_context>
TEST_F(UT_MainWindow, test_getConfigWinSize)
</code_context>
<issue_to_address>
**suggestion (testing):** No tests for restoring maximized state or timer-based maximization logic.
Please add a test that verifies the window is maximized after restoreConfigWinState is called with a maximized state, using QTest::qWait or similar to handle the timer.
</issue_to_address>
### Comment 4
<location> `src/source/mainwindow.cpp:2462` </location>
<code_context>
}
-void MainWindow::saveConfigWinSize(int w, int h)
+void MainWindow::saveConfigWinState()
{
- int winWidth = w > MAINWINDOW_DEFAULTW ? w : MAINWINDOW_DEFAULTW;
</code_context>
<issue_to_address>
**issue (complexity):** Consider replacing custom window size and state management with Qt's built-in save/restore routines for geometry and state.
Consider dropping the custom size/state logic entirely and switch to Qt’s built-in save/restore routines. You can keep all of your functionality—size, position, maximized/minimized state—while removing ~100 lines of custom code and the QTimer hack. For example:
1) In your initialization (after you create `m_pSettings`), replace
```cpp
restoreConfigWinState();
```
with:
```cpp
if (m_pSettings->contains("geometry"))
restoreGeometry(m_pSettings->value("geometry").toByteArray());
if (m_pSettings->contains("windowState"))
restoreState(m_pSettings->value("windowState").toByteArray());
```
2) In your close or destructor (override `closeEvent`):
```cpp
void MainWindow::closeEvent(QCloseEvent *event) {
m_pSettings->setValue("geometry", saveGeometry());
m_pSettings->setValue("windowState", saveState());
m_pSettings->sync();
DMainWindow::closeEvent(event);
}
```
3) Remove these methods entirely:
- `getConfigWinSize()`
- `saveConfigWinSize(...)`
- `saveConfigWinState()`
- `restoreConfigWinState()`
- All related constants and QTimer hacks
This preserves window placement, size constraints, and maximized state without manual branching.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| m_pSettings->setValue(MAINWINDOW_HEIGHT_NAME, winHeight); | ||
| m_pSettings->setValue(MAINWINDOW_WIDTH_NAME, winWidth); | ||
| // 如果窗口是最大化状态，保存最大化标志和正常尺寸 | ||
| if (isMaximized()) { |
There was a problem hiding this comment.
issue (complexity): Consider replacing custom window size and state management with Qt's built-in save/restore routines for geometry and state.
Consider dropping the custom size/state logic entirely and switch to Qt’s built-in save/restore routines. You can keep all of your functionality—size, position, maximized/minimized state—while removing ~100 lines of custom code and the QTimer hack. For example:
-
In your initialization (after you create
m_pSettings), replacerestoreConfigWinState();with:
if (m_pSettings->contains("geometry")) restoreGeometry(m_pSettings->value("geometry").toByteArray()); if (m_pSettings->contains("windowState")) restoreState(m_pSettings->value("windowState").toByteArray());
-
In your close or destructor (override
closeEvent):void MainWindow::closeEvent(QCloseEvent *event) { m_pSettings->setValue("geometry", saveGeometry()); m_pSettings->setValue("windowState", saveState()); m_pSettings->sync(); DMainWindow::closeEvent(event); }
-
Remove these methods entirely:
getConfigWinSize()saveConfigWinSize(...)saveConfigWinState()restoreConfigWinState()- All related constants and QTimer hacks
This preserves window placement, size constraints, and maximized state without manual branching.
lzwind
approved these changes
Nov 21, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Author
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
-- Realize the saving and restoration of the window state, including maximizing
the state and size. The relevant functions have been updated to ensure that
the state is correctly saved when the window is maximized and the saved
state is applied when restored.
Log: Add the functions of saving and restoring window status
Bug: https://pms.uniontech.com/bug-view-318459.html
Summary by Sourcery
Implement persistent storage and restoration of the window's size and maximization state by introducing state-based save and restore methods and integrating them into the MainWindow lifecycle.
New Features:
Bug Fixes:
Enhancements:
Tests: