-
Notifications
You must be signed in to change notification settings - Fork 48
fix: Fix the abnormal display of network panel height #469
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
Fix the abnormal display of network panel height Log: Fix the abnormal display of network panel height pms: BUG-311381
Reviewer's GuideAdjusts network dock panel sizing behavior by recalculating sizes on show/hide events and ensuring the NetView layout geometry is updated when shown, to fix abnormal panel height display. Sequence diagram for network panel show and hide sizing logicsequenceDiagram
actor User
participant DockContentWidget
participant NetView
User->>DockContentWidget: open_panel
activate DockContentWidget
DockContentWidget->>DockContentWidget: showEvent(event)
DockContentWidget->>DockContentWidget: updateSize()
DockContentWidget->>DockContentWidget: base_QWidget_showEvent(event)
DockContentWidget->>DockContentWidget: queue_updateSize_and_resize()
deactivate DockContentWidget
User->>NetView: show_panel_window
activate NetView
NetView->>NetView: showEvent(event)
NetView->>NetView: maybe_exec_ToggleExpand()
NetView->>NetView: updateGeometries()
deactivate NetView
User->>DockContentWidget: close_panel
activate DockContentWidget
DockContentWidget->>DockContentWidget: hideEvent(event)
DockContentWidget->>DockContentWidget: base_QWidget_hideEvent(event)
DockContentWidget->>DockContentWidget: m_minHeight = -1
DockContentWidget->>DockContentWidget: queue_updateSize_collapsed()
deactivate DockContentWidget
Updated class diagram for DockContentWidget and NetView sizing behaviorclassDiagram
class DockContentWidget {
+QVBoxLayout* m_mainLayout
+NetView* m_netView
+QPushButton* m_netSetBtn
+QPushButton* m_netCheckBtn
+int m_minHeight
+void setMargins(QMargins margin)
+void setMinHeight(int minHeight)
+void updateSize()
+void showEvent(QShowEvent* event)
+void hideEvent(QHideEvent* event)
}
class NetView {
+void showEvent(QShowEvent* event)
+void hideEvent(QHideEvent* event)
+void updateGeometries()
}
DockContentWidget --> NetView : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review代码审查意见整体评价这段代码主要对DockContentWidget类添加了showEvent和hideEvent方法,并在NetView类中添加了updateGeometries调用,目的是优化窗口显示时的尺寸计算和更新。代码整体逻辑清晰,但有一些可以改进的地方。 具体审查意见1. 语法与逻辑问题
2. 代码质量
3. 代码性能
4. 代码安全
修改建议综合以上问题,建议修改如下: // dockcontentwidget.h
class DockContentWidget : public QWidget
{
// ... 其他代码 ...
static constexpr int POPUP_WIDGET_HEIGHT_MARGIN = 20;
static constexpr int BUTTON_SPACING = 10;
public Q_SLOTS:
void updateSize() {
if (!m_netView || !m_mainLayout || !m_netSetBtn) {
return;
}
const int baseHeight = Dock::DOCK_POPUP_WIDGET_MAX_HEIGHT - POPUP_WIDGET_HEIGHT_MARGIN;
const int marginHeight = m_mainLayout->contentsMargins().top();
const int buttonHeight = m_netSetBtn->height() + (m_netCheckBtn->isVisible() ? (m_netCheckBtn->height() + BUTTON_SPACING) : 0);
const int availableHeight = baseHeight - marginHeight - buttonHeight;
m_netView->setMaxHeight(availableHeight);
}
protected:
void showEvent(QShowEvent *event) override
{
QWidget::showEvent(event);
updateSize();
QMetaObject::invokeMethod(this, [this]() {
updateSize();
adjustSize();
}, Qt::QueuedConnection);
}
void hideEvent(QHideEvent *event) override
{
QWidget::hideEvent(event);
m_minHeight = -1;
QMetaObject::invokeMethod(this, [this]() {
updateSize();
}, Qt::QueuedConnection);
}
// ... 其他代码 ...
};总结这段代码主要改进了窗口显示时的尺寸计算和更新逻辑,但存在一些可以优化的地方,包括:
以上修改将使代码更健壮、更易维护,同时保持原有功能。 |
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.
Hey - I've found 2 issues, and left some high level feedback:
- In DockContentWidget::showEvent you call updateSize() immediately and again in a queued lambda; consider whether both are required or add a brief inline comment to clarify the need for the second call to avoid future confusion about potential redundant work.
- For the queued calls to updateSize in DockContentWidget (both in showEvent and hideEvent), prefer using the function-pointer overload of QMetaObject::invokeMethod (e.g. &DockContentWidget::updateSize) instead of the string-based version for better type safety and compile-time checking.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In DockContentWidget::showEvent you call updateSize() immediately and again in a queued lambda; consider whether both are required or add a brief inline comment to clarify the need for the second call to avoid future confusion about potential redundant work.
- For the queued calls to updateSize in DockContentWidget (both in showEvent and hideEvent), prefer using the function-pointer overload of QMetaObject::invokeMethod (e.g. &DockContentWidget::updateSize) instead of the string-based version for better type safety and compile-time checking.
## Individual Comments
### Comment 1
<location> `dock-network-plugin/dockcontentwidget.h:107-111` </location>
<code_context>
+ }, Qt::QueuedConnection);
+ }
+
+ void hideEvent(QHideEvent *event) override
+ {
+ QWidget::hideEvent(event);
+ m_minHeight = -1;
+ // 隐藏时更新尺寸为折叠状态,确保下次显示时初始尺寸正确
+ QMetaObject::invokeMethod(this, "updateSize", Qt::QueuedConnection);
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Mixed styles of QMetaObject::invokeMethod reduce type safety and consistency.
In `showEvent` you use the functor-based overload, but `hideEvent` uses the string-based one, which is less type-safe and more fragile during refactors. Please update `hideEvent` to the functor form as well, e.g. `QMetaObject::invokeMethod(this, [this]{ updateSize(); }, Qt::QueuedConnection);`.
```suggestion
QWidget::hideEvent(event);
m_minHeight = -1;
// 隐藏时更新尺寸为折叠状态,确保下次显示时初始尺寸正确
QMetaObject::invokeMethod(this, [this]() {
updateSize();
}, Qt::QueuedConnection);
}
```
</issue_to_address>
### Comment 2
<location> `dock-network-plugin/dockcontentwidget.h:95` </location>
<code_context>
return QWidget::eventFilter(watch, event);
}
+ void showEvent(QShowEvent *event) override
+ {
+ updateSize();
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the deferred sizing behavior into a shared helper to centralize logic and avoid string-based invokeMethod usage while preserving current show/hide behavior.
You can reduce the new complexity by centralizing the deferred sizing logic and avoiding the string-based `invokeMethod` while keeping behavior (immediate + deferred updates, min-height reset, resize) intact.
### 1. Centralize deferred sizing
Extract the queued sizing logic to a small helper, used from both `showEvent` and `hideEvent`. This removes duplication and makes the async behavior explicit:
```c++
private:
bool m_deferredUpdateScheduled = false;
void scheduleDeferredUpdateSize(bool doResize)
{
if (m_deferredUpdateScheduled)
return;
m_deferredUpdateScheduled = true;
QMetaObject::invokeMethod(this, [this, doResize]() {
m_deferredUpdateScheduled = false;
updateSize();
if (doResize)
resize(size());
}, Qt::QueuedConnection);
}
```
### 2. Simplify `showEvent` and `hideEvent` with the helper
Reuse the helper so the control flow is easy to follow and you no longer need the string-based overload:
```c++
void showEvent(QShowEvent *event) override
{
updateSize(); // keep immediate update
QWidget::showEvent(event);
scheduleDeferredUpdateSize(true); // deferred update + resize
}
void hideEvent(QHideEvent *event) override
{
QWidget::hideEvent(event);
m_minHeight = -1;
// 隐藏时更新尺寸为折叠状态,确保下次显示时初始尺寸正确
scheduleDeferredUpdateSize(false); // deferred update only
}
```
This keeps all current behavior (immediate sizing on show, deferred re-sizes, min-height reset on hide) but removes duplicated lambdas, avoids string-based `invokeMethod`, and makes the deferred sizing semantics clear and centralized.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| QWidget::hideEvent(event); | ||
| m_minHeight = -1; | ||
| // 隐藏时更新尺寸为折叠状态,确保下次显示时初始尺寸正确 | ||
| QMetaObject::invokeMethod(this, "updateSize", Qt::QueuedConnection); | ||
| } |
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.
suggestion: Mixed styles of QMetaObject::invokeMethod reduce type safety and consistency.
In showEvent you use the functor-based overload, but hideEvent uses the string-based one, which is less type-safe and more fragile during refactors. Please update hideEvent to the functor form as well, e.g. QMetaObject::invokeMethod(this, [this]{ updateSize(); }, Qt::QueuedConnection);.
| QWidget::hideEvent(event); | |
| m_minHeight = -1; | |
| // 隐藏时更新尺寸为折叠状态,确保下次显示时初始尺寸正确 | |
| QMetaObject::invokeMethod(this, "updateSize", Qt::QueuedConnection); | |
| } | |
| QWidget::hideEvent(event); | |
| m_minHeight = -1; | |
| // 隐藏时更新尺寸为折叠状态,确保下次显示时初始尺寸正确 | |
| QMetaObject::invokeMethod(this, [this]() { | |
| updateSize(); | |
| }, Qt::QueuedConnection); | |
| } |
| return QWidget::eventFilter(watch, event); | ||
| } | ||
|
|
||
| void showEvent(QShowEvent *event) override |
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.
issue (complexity): Consider extracting the deferred sizing behavior into a shared helper to centralize logic and avoid string-based invokeMethod usage while preserving current show/hide behavior.
You can reduce the new complexity by centralizing the deferred sizing logic and avoiding the string-based invokeMethod while keeping behavior (immediate + deferred updates, min-height reset, resize) intact.
1. Centralize deferred sizing
Extract the queued sizing logic to a small helper, used from both showEvent and hideEvent. This removes duplication and makes the async behavior explicit:
private:
bool m_deferredUpdateScheduled = false;
void scheduleDeferredUpdateSize(bool doResize)
{
if (m_deferredUpdateScheduled)
return;
m_deferredUpdateScheduled = true;
QMetaObject::invokeMethod(this, [this, doResize]() {
m_deferredUpdateScheduled = false;
updateSize();
if (doResize)
resize(size());
}, Qt::QueuedConnection);
}2. Simplify showEvent and hideEvent with the helper
Reuse the helper so the control flow is easy to follow and you no longer need the string-based overload:
void showEvent(QShowEvent *event) override
{
updateSize(); // keep immediate update
QWidget::showEvent(event);
scheduleDeferredUpdateSize(true); // deferred update + resize
}
void hideEvent(QHideEvent *event) override
{
QWidget::hideEvent(event);
m_minHeight = -1;
// 隐藏时更新尺寸为折叠状态,确保下次显示时初始尺寸正确
scheduleDeferredUpdateSize(false); // deferred update only
}This keeps all current behavior (immediate sizing on show, deferred re-sizes, min-height reset on hide) but removes duplicated lambdas, avoids string-based invokeMethod, and makes the deferred sizing semantics clear and centralized.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, pengfeixx 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 |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
Fix the abnormal display of network panel height
Log: Fix the abnormal display of network panel height
pms: BUG-311381
Summary by Sourcery
Fix layout handling for the dock network panel to ensure correct height and geometry when the panel is shown or hidden.
Bug Fixes: