Skip to content

fix: remove minHeight reset and add minHeight message handling#548

Merged
wjyrich merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-356951
Apr 22, 2026
Merged

fix: remove minHeight reset and add minHeight message handling#548
wjyrich merged 1 commit intolinuxdeepin:masterfrom
wjyrich:fix-bug-356951

Conversation

@wjyrich
Copy link
Copy Markdown
Contributor

@wjyrich wjyrich commented Apr 22, 2026

  1. Removed m_minHeight reset in hideEvent to preserve minimum height setting across hide/show cycles
  2. Added message handling for MSG_SET_APPLET_MIN_HEIGHT to support dynamic minimum height configuration
  3. The plugin now responds to external requests to set minimum height for the applet container
  4. This ensures proper layout behavior when the dock content widget is hidden and shown again

Log: Fixed dock network plugin layout preservation issues

Influence:

  1. Test hiding and showing the network plugin in dock to verify height preservation
  2. Verify that external messages to set minimum height are properly processed
  3. Test with different minimum height values to ensure correct layout behavior
  4. Check that the plugin maintains consistent appearance after multiple hide/show cycles
  5. Verify compatibility with dock container margin settings

fix: 移除最小高度重置并添加最小高度消息处理

  1. 移除 hideEvent 中的 m_minHeight 重置,以在隐藏/显示周期中保持最小高度 设置
  2. 添加 MSG_SET_APPLET_MIN_HEIGHT 消息处理,支持动态最小高度配置
  3. 插件现在能够响应外部设置应用容器最小高度的请求
  4. 确保停靠栏内容部件在隐藏和再次显示时具有正确的布局行为

Log: 修复停靠栏网络插件布局保持问题

Influence:

  1. 测试隐藏和显示停靠栏中的网络插件,验证高度保持功能
  2. 验证外部设置最小高度的消息是否被正确处理
  3. 使用不同的最小高度值测试,确保正确的布局行为
  4. 检查插件在多次隐藏/显示循环后是否保持一致的显示效果
  5. 验证与停靠栏容器边距设置的兼容性

PMS: BUG-356951

Summary by Sourcery

Preserve the dock network plugin applet's minimum height across hide/show cycles and handle external messages to update it dynamically.

Bug Fixes:

  • Fix loss of the applet's configured minimum height when the dock network plugin is hidden and shown again.

Enhancements:

  • Add support for processing external messages to set the applet container's minimum height, ensuring correct layout behavior with varying dock margins.

1. Removed m_minHeight reset in hideEvent to preserve minimum height
setting across hide/show cycles
2. Added message handling for MSG_SET_APPLET_MIN_HEIGHT to support
dynamic minimum height configuration
3. The plugin now responds to external requests to set minimum height
for the applet container
4. This ensures proper layout behavior when the dock content widget is
hidden and shown again

Log: Fixed dock network plugin layout preservation issues

Influence:
1. Test hiding and showing the network plugin in dock to verify height
preservation
2. Verify that external messages to set minimum height are properly
processed
3. Test with different minimum height values to ensure correct layout
behavior
4. Check that the plugin maintains consistent appearance after multiple
hide/show cycles
5. Verify compatibility with dock container margin settings

fix: 移除最小高度重置并添加最小高度消息处理

1. 移除 hideEvent 中的 m_minHeight 重置,以在隐藏/显示周期中保持最小高度
设置
2. 添加 MSG_SET_APPLET_MIN_HEIGHT 消息处理,支持动态最小高度配置
3. 插件现在能够响应外部设置应用容器最小高度的请求
4. 确保停靠栏内容部件在隐藏和再次显示时具有正确的布局行为

Log: 修复停靠栏网络插件布局保持问题

Influence:
1. 测试隐藏和显示停靠栏中的网络插件,验证高度保持功能
2. 验证外部设置最小高度的消息是否被正确处理
3. 使用不同的最小高度值测试,确保正确的布局行为
4. 检查插件在多次隐藏/显示循环后是否保持一致的显示效果
5. 验证与停靠栏容器边距设置的兼容性

PMS: BUG-356951
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 22, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Preserves the dock network plugin’s minimum height across hide/show cycles and adds message-based control for the applet container’s minimum height, while keeping existing container margin handling intact.

Sequence diagram for handling MSG_SET_APPLET_MIN_HEIGHT in NetworkPlugin::message

sequenceDiagram
    participant ExternalSender
    participant Dock
    participant NetworkPlugin
    participant DockContentWidget

    ExternalSender->>Dock: send JSON with MSG_SET_APPLET_MIN_HEIGHT
    Dock->>NetworkPlugin: message(msg)
    NetworkPlugin->>NetworkPlugin: parse msg into msgObj
    alt msgObj[MSG_TYPE] == MSG_SET_APPLET_MIN_HEIGHT
        NetworkPlugin->>NetworkPlugin: minHeight = msgObj[MSG_DATA].toInt(-1)
        alt m_dockContentWidget exists and minHeight > 0
            NetworkPlugin->>DockContentWidget: setMinHeight(minHeight)
        end
    end
    NetworkPlugin->>Dock: return result string
Loading

Updated class diagram for NetworkPlugin and DockContentWidget minHeight handling

classDiagram
    class NetworkPlugin {
        +QString message(QString msg)
        -DockContentWidget* m_dockContentWidget
    }

    class DockContentWidget {
        -int m_minHeight
        +void hideEvent(QHideEvent* event)
        +void setMinHeight(int minHeight)
        +void updateSize()
    }

    NetworkPlugin --> DockContentWidget : manages_minHeight
Loading

File-Level Changes

Change Details Files
Preserve dock content widget minimum height across hide/show cycles.
  • Removed reset of the internal minimum height state in the widget’s hideEvent override so the previously configured minimum height persists when the widget is hidden and shown again.
  • Kept the post-hide queued call to updateSize so the widget still recalculates its collapsed size on hide.
dock-network-plugin/dockcontentwidget.h
Handle external messages to dynamically set the applet container’s minimum height.
  • Extended the message handler to recognize MSG_SET_APPLET_MIN_HEIGHT requests extracted from the parsed JSON message object.
  • When such a message is received with a positive minHeight value and the dock content widget exists, call setMinHeight(minHeight) on the dock content widget.
  • Left existing MSG_APPLET_CONTAINER handling unchanged to continue applying top margins based on APPLET_CONTAINER_QUICK_PANEL state.
dock-network-plugin/networkplugin.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Consider caching msgObj.value(Dock::MSG_TYPE).toString() in a local variable so you don't perform the same lookup and conversion twice when handling different message types.
  • When processing MSG_SET_APPLET_MIN_HEIGHT, it may be useful to validate and clamp unreasonable height values (or add a clear fallback behavior) rather than silently ignoring non-positive values.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider caching `msgObj.value(Dock::MSG_TYPE).toString()` in a local variable so you don't perform the same lookup and conversion twice when handling different message types.
- When processing `MSG_SET_APPLET_MIN_HEIGHT`, it may be useful to validate and clamp unreasonable height values (or add a clear fallback behavior) rather than silently ignoring non-positive values.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码的 diff 主要涉及网络插件在隐藏事件处理和消息响应方面的修改。以下是对代码的详细审查意见:

1. 逻辑与功能审查

修改点 1:移除 m_minHeight = -1

  • 现状:在 DockContentWidget::hideEvent 中删除了 m_minHeight = -1 这行代码。
  • 分析:原来的逻辑是在窗口隐藏时重置最小高度为 -1(无效值)。现在的修改移除了这个重置操作。
  • 潜在风险:这表明 m_minHeight 的状态现在需要在窗口显示或重新初始化时由外部(通过 message 函数)明确设置,而不是在隐藏时自动清除。
  • 关联性:这与下文 NetworkPlugin::message 中新增的 MSG_SET_APPLET_MIN_HEIGHT 处理逻辑相呼应。现在系统通过消息机制显式控制最小高度,而不是依赖内部状态的重置。这是一个改进,使得状态管理更加显式和可控,减少了隐藏/显示过程中的副作用。

修改点 2:新增 MSG_SET_APPLET_MIN_HEIGHT 消息处理

  • 现状:在 NetworkPlugin::message 中增加了对 Dock::MSG_SET_APPLET_MIN_HEIGHT 消息的监听,并调用 setMinHeight
  • 分析:这是为了配合修改点 1,确保插件能够响应外部(如 Dock 栏)发送的最小高度设置请求。
  • 逻辑
    if (msgObj.value(Dock::MSG_TYPE).toString() == Dock::MSG_SET_APPLET_MIN_HEIGHT) {
        const int minHeight = msgObj.value(Dock::MSG_DATA).toInt(-1);
        if (m_dockContentWidget && minHeight > 0)
            m_dockContentWidget->setMinHeight(minHeight);
    }
    逻辑清晰,增加了 minHeight > 0 的判断,防止无效值(如 -1 或 0)被设置,这是一个很好的防御性编程实践。

2. 代码质量与规范

  • 代码风格:代码符合 Qt/C++ 的常规命名和缩进规范。
  • 常量使用:使用了 Dock:: 命名空间下的常量(如 MSG_TYPE, MSG_DATA),避免了魔法字符串,提高了可读性。

3. 代码性能

  • 字符串比较:代码中多次使用 msgObj.value(Dock::MSG_TYPE).toString() 进行比较。
    • 建议QJsonObject::value()toString() 会产生临时对象。虽然在这个场景下(消息处理频率不高)性能影响可以忽略不计,但为了代码的极致优化和可读性,建议提取 msgType 变量:
    const QString msgType = msgObj.value(Dock::MSG_TYPE).toString();
    if (msgType == Dock::MSG_SET_APPLET_MIN_HEIGHT) {
        // ...
    }
    if (msgType == Dock::MSG_APPLET_CONTAINER && m_dockContentWidget) {
        // ...
    }

4. 代码安全

  • 空指针检查:代码中包含了 if (m_dockContentWidget && ...) 的检查,这是非常必要的,防止了在插件未完全初始化或已销毁时调用成员方法导致的崩溃。
  • 数据有效性检查minHeight > 0 的检查防止了非法尺寸的设置,保证了 UI 布局的稳定性。

总结与改进建议

这段代码的修改逻辑是正确的,将内部状态的重置逻辑转变为外部消息驱动的设置逻辑,解耦了隐藏事件与属性状态。

改进建议代码片段:

QString NetworkPlugin::message(const QString &msg)
{
    // ... (前面的 JSON 解析代码保持不变)
    const auto &msgObj = resultDoc.object();
    
    // 提取消息类型以避免重复调用 toString()
    const QString msgType = msgObj.value(Dock::MSG_TYPE).toString();

    // 处理最小高度设置
    if (msgType == Dock::MSG_SET_APPLET_MIN_HEIGHT) {
        const int minHeight = msgObj.value(Dock::MSG_DATA).toInt(-1);
        if (m_dockContentWidget && minHeight > 0) {
            m_dockContentWidget->setMinHeight(minHeight);
        }
    }

    // 处理容器边距设置
    if (msgType == Dock::MSG_APPLET_CONTAINER && m_dockContentWidget) {
        // 保持原有逻辑
        const int data = msgObj.value(Dock::MSG_DATA).toInt(-1);
        const int topMargin = (data == Dock::APPLET_CONTAINER_QUICK_PANEL) ? 6 : 10;
        m_dockContentWidget->setMainLayoutMargins(QMargins(0, topMargin, 0, 0));
    }
    
    // ...
}

主要优化点:

  1. 提取 msgType 变量,减少重复计算,提升微小的性能并提高可读性。
  2. 将三元运算符逻辑稍微拆分或保持原样(视团队风格而定),上面的示例中为了清晰度稍微拆分了变量赋值,但原代码写法也是可以接受的。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, wjyrich

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

Details 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

@wjyrich wjyrich merged commit 4911489 into linuxdeepin:master Apr 22, 2026
17 of 19 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