Feat: Add group setting to show screen size.#631
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:develop/eaglefrom Mar 31, 2026
Merged
Conversation
-- Add group setting to show screen size. Log: add feature Task: https://pms.uniontech.com/task-view-387965.html
Reviewer's GuideRefactors how the Device Manager decides whether and how to display monitor screen size by centralizing the DConfig-backed setting into a new Common::isShowScreenSize() helper and applying it consistently to both the overview text and the detailed device info list. Sequence diagram for loading monitor info with screen size settingsequenceDiagram
actor User
participant DeviceManagerUI
participant DeviceMonitor
participant Common
participant DConfig
User->>DeviceManagerUI: openDeviceManager
DeviceManagerUI->>DeviceMonitor: loadOtherDeviceInfo
DeviceMonitor->>Common: isShowScreenSize
Common->>DConfig: create(org.deepin.devicemanager)
DConfig-->>Common: showScreenSize_value
Common-->>DeviceMonitor: showScreenSize_flag
alt showScreenSize_flag_is_true
DeviceMonitor->>DeviceMonitor: addOtherDeviceInfo(Size, m_ScreenSize)
else showScreenSize_flag_is_false
DeviceMonitor->>DeviceMonitor: skip_adding_Size_info
end
DeviceMonitor->>DeviceManagerUI: other_device_info_list
DeviceManagerUI-->>User: display_monitor_details
Updated class diagram for Common and DeviceMonitor screen size logicclassDiagram
class Common {
+static bool isShowScreenSize()
+static QByteArray executeClientCmd(QString cmd, QStringList args, QString workPath, int msecsWaiting)
}
class DeviceMonitor {
+QString getOverviewInfo()
+void loadOtherDeviceInfo()
+void addOtherDeviceInfo(QString key, QString value)
}
DeviceMonitor ..> Common : calls
Common ..> DConfig : uses_screen_size_config
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
Common::isShowScreenSize(), theDConfig::create(...)result is never destroyed, which can cause a leak; consider wrapping it in a smart pointer (e.g.,QScopedPointer) or otherwise ensuring it is deleted after use. Common::isShowScreenSize()reads DConfig on every call, which may be unnecessarily expensive for a simple flag; consider caching the value (with optional invalidation) so it’s only read once or infrequently.- The
DeviceMonitor::getOverviewInfo()logic now has nestedif (Common::isShowScreenSize())and repeated checks for the samespecialComTypeset; consider simplifying the branching to avoid duplication and make the formatting rules easier to follow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Common::isShowScreenSize()`, the `DConfig::create(...)` result is never destroyed, which can cause a leak; consider wrapping it in a smart pointer (e.g., `QScopedPointer`) or otherwise ensuring it is deleted after use.
- `Common::isShowScreenSize()` reads DConfig on every call, which may be unnecessarily expensive for a simple flag; consider caching the value (with optional invalidation) so it’s only read once or infrequently.
- The `DeviceMonitor::getOverviewInfo()` logic now has nested `if (Common::isShowScreenSize())` and repeated checks for the same `specialComType` set; consider simplifying the branching to avoid duplication and make the formatting rules easier to follow.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
max-lvs
approved these changes
Mar 31, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017, max-lvs 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 |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
412225e
into
linuxdeepin:develop/eagle
20 of 22 checks passed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
-- Add group setting to show screen size.
Log: add feature
Task: https://pms.uniontech.com/task-view-387965.html
Summary by Sourcery
Introduce a configurable screen size display behavior for device monitor information, centralizing the setting in a common helper.
Enhancements:
showScreenSizeconfiguration lookup into a newCommon::isShowScreenSize()utility used across the device manager.