fix: filter unsupported dock applets in DBus proxy#1563
Merged
Conversation
Reviewer's GuideAdds an explicit supported state to dock applets, wires MultiTaskView’s support to compositor/KWin effect availability, and updates the DBus proxy to ignore unsupported applets for visibility and plugin listing, ensuring visibility and signals reflect both user choice and platform support. Sequence diagram for compositor changes affecting MultiTaskView visibilitysequenceDiagram
participant DWindowManagerHelper
participant MultiTaskView
participant DAppletDock as BaseDockApplet
DWindowManagerHelper->>MultiTaskView: hasCompositeChanged(newHasComposite)
activate MultiTaskView
MultiTaskView-->>MultiTaskView: Q_EMIT visibleChanged()
MultiTaskView->>MultiTaskView: setSupported(m_kWinEffect && hasComposite())
alt supported state unchanged
MultiTaskView-->>MultiTaskView: return (no signal change)
else supported state changed
MultiTaskView->>BaseDockApplet: setSupported(bool supported)
activate BaseDockApplet
BaseDockApplet-->>BaseDockApplet: oldEffectiveVisible = visible()
BaseDockApplet-->>BaseDockApplet: m_isSupported = supported
BaseDockApplet-->>BaseDockApplet: Q_EMIT supportedChanged()
alt effective visibility changed
BaseDockApplet-->>BaseDockApplet: Q_EMIT visibleChanged()
end
deactivate BaseDockApplet
end
deactivate MultiTaskView
Sequence diagram for DockDBusProxy filtering unsupported dock appletssequenceDiagram
participant Client
participant DockDBusProxy
participant DockApplet1
participant DockApplet2
Client->>DockDBusProxy: updateDockPluginsVisible(pluginsVisible)
activate DockDBusProxy
loop for each dockApplet in dockApplets(parent)
alt dockApplet is unsupported
DockDBusProxy->>DockApplet1: isSupported()
DockApplet1-->>DockDBusProxy: false
DockDBusProxy-->>DockDBusProxy: continue (skip applet)
else dockApplet is supported
DockDBusProxy->>DockApplet2: isSupported()
DockApplet2-->>DockDBusProxy: true
DockDBusProxy->>DockApplet2: dockItemInfo()
DockApplet2-->>DockDBusProxy: DockItemInfo
DockDBusProxy-->>DockDBusProxy: apply pluginsVisible[itemKey]
end
end
deactivate DockDBusProxy
Client->>DockDBusProxy: plugins()
activate DockDBusProxy
loop for each dockApplet in dockApplets(parent)
alt dockApplet is unsupported
DockDBusProxy->>DockApplet1: isSupported()
DockApplet1-->>DockDBusProxy: false
DockDBusProxy-->>DockDBusProxy: skip appending
else dockApplet is supported
DockDBusProxy->>DockApplet2: isSupported()
DockApplet2-->>DockDBusProxy: true
DockDBusProxy->>DockApplet2: dockItemInfo()
DockApplet2-->>DockDBusProxy: DockItemInfo
DockDBusProxy-->>DockDBusProxy: iteminfos.append(info)
end
end
DockDBusProxy-->>Client: DockItemInfos (supported applets only)
deactivate DockDBusProxy
Class diagram for dock applet support and visibility modelclassDiagram
class DAppletDockPrivate {
bool m_visible
bool m_isSupported
}
class DAppletDock {
+bool visible()
+void setVisible(bool visible)
+bool isSupported()
+void setSupported(bool supported)
+DockItemInfo dockItemInfo()
+void init()
<<signals>>
+void visibleChanged()
+void supportedChanged()
}
class MultiTaskView {
+MultiTaskView(QObject parent)
+bool init()
+DockItemInfo dockItemInfo()
-bool m_kWinEffect
-QScopedPointer multitaskview
}
class DockDBusProxy {
+QRect geometry()
+void updateDockPluginsVisible(QVariantMap pluginsVisible)
+DockItemInfos plugins()
}
class DWindowManagerHelper {
+static DWindowManagerHelper instance()
+bool hasComposite()
<<signals>>
+void hasCompositeChanged(bool hasComposite)
}
class DockItemInfo {
QString displayName
QString itemKey
QString settingKey
bool visible
QString dccIcon
}
DAppletDockPrivate <.. DAppletDock : uses
DAppletDock *-- DAppletDockPrivate : owns
MultiTaskView --|> DAppletDock
DockDBusProxy ..> DAppletDock : iterates_dockApplets
DWindowManagerHelper ..> MultiTaskView : notifies_compositor_change
DAppletDock ..> DockItemInfo : returns
MultiTaskView ..> DockItemInfo : returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In MultiTaskView, the hasCompositeChanged and kWinEffect changed handlers both emit visibleChanged and then call setSupported, which can emit visibleChanged again; consider relying on setSupported’s effective-visibility detection to avoid redundant signal emissions.
- The condition
m_kWinEffect && DWindowManagerHelper::instance()->hasComposite()is duplicated in several places in MultiTaskView (constructor lambda, kWinEffect change, init); consider extracting a small helper (e.g.updateSupportFromCompositor()) or usingisSupported()consistently to keep the logic centralized and less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In MultiTaskView, the hasCompositeChanged and kWinEffect changed handlers both emit visibleChanged and then call setSupported, which can emit visibleChanged again; consider relying on setSupported’s effective-visibility detection to avoid redundant signal emissions.
- The condition `m_kWinEffect && DWindowManagerHelper::instance()->hasComposite()` is duplicated in several places in MultiTaskView (constructor lambda, kWinEffect change, init); consider extracting a small helper (e.g. `updateSupportFromCompositor()`) or using `isSupported()` consistently to keep the logic centralized and less error-prone.
## Individual Comments
### Comment 1
<location path="panels/dock/multitaskview/multitaskview.cpp" line_range="28-30" />
<code_context>
, m_iconName("deepin-multitasking-view")
{
- connect(DWindowManagerHelper::instance(), &DWindowManagerHelper::hasCompositeChanged, this, &MultiTaskView::visibleChanged);
+ connect(DWindowManagerHelper::instance(), &DWindowManagerHelper::hasCompositeChanged, this, [this]() {
+ Q_EMIT visibleChanged();
+ setSupported(m_kWinEffect && DWindowManagerHelper::instance()->hasComposite());
+ });
auto platformName = QGuiApplication::platformName();
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid potentially emitting `visibleChanged` twice on compositor changes.
Because the lambda always emits `visibleChanged()`, and `setSupported()` can emit it again when the effective visibility changes, a single compositor change may trigger duplicate notifications. Either rely on `setSupported()` alone to emit `visibleChanged()` when needed, or emit `visibleChanged()` here only when `supported` actually changes (e.g. compute the new supported state, compare to `isSupported()`, then call `setSupported()` without the unconditional `Q_EMIT visibleChanged()`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
wjyrich
force-pushed
the
feat-dockapplete-1
branch
from
92fe171 to
d46d828
Compare
1. Added isSupported property to DAppletDock class to mark applet compatibility 2. Modified dock DBus proxy to skip unsupported applets when updating plugin visibility and listing plugins 3. Enhanced MultiTaskView to dynamically update supported status based on window compositor availability 4. Fixed visible property calculation to consider both m_visible and m_isSupported flags 5. Added proper signal emission when supported status changes affect effective visibility Log: Fixed dock applet visibility issues when compositor is unavailable Influence: 1. Test dock applet visibility when switching between composited and non-composited modes 2. Verify DBus plugin list excludes unsupported applets 3. Check that MultiTaskView correctly appears/disappears based on compositor support 4. Test that visibleChanged signal fires appropriately when supported status changes 5. Verify dock settings UI correctly reflects available applets fix: 在DBus代理中过滤不支持的停靠小程序 1. 在DAppletDock类中添加isSupported属性以标记小程序兼容性 2. 修改停靠DBus代理，在更新插件可见性和列出插件时跳过不支持的小程序 3. 增强MultiTaskView以根据窗口合成器可用性动态更新支持状态 4. 修复visible属性计算，同时考虑m_visible和m_isSupported标志 5. 当支持状态变化影响有效可见性时添加适当的信号发射 Log: 修复合成器不可用时停靠小程序可见性问题 Influence: 1. 测试在合成和非合成模式间切换时停靠小程序的可见性 2. 验证DBus插件列表排除不支持的小程序 3. 检查MultiTaskView是否根据合成器支持正确显示/隐藏 4. 测试当支持状态变化时visibleChanged信号是否适当触发 5. 验证停靠设置UI正确反映可用的小程序
wjyrich
force-pushed
the
feat-dockapplete-1
branch
from
d46d828 to
402b3b2
Compare
18202781743
approved these changes
Apr 20, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
deepin pr auto review这份代码变更主要实现了对Dock插件（Applet）的"支持状态"（supported）进行管理，使得插件不仅可以通过 以下是对代码的详细审查和改进建议： 1. 语法逻辑问题： for (auto *dockApplet : dockApplets(parent)) {
connect(dockApplet, &DS_NAMESPACE::DAppletDock::supportedChanged, this, [this]() {
auto pluginsVisible = DockSettings::instance()->pluginsVisible();
updateDockPluginsVisible(pluginsVisible);
Q_EMIT pluginsChanged();
});
}分析：这个循环中存在一个潜在的逻辑问题。Lambda 表达式 改进建议：
问题： // 旧代码
info.visible = m_kWinEffect && DWindowManagerHelper::instance()->hasComposite();
// 新代码
info.visible = visible();分析：这里将可见性的判断逻辑直接委托给了基类的 2. 代码质量问题： if (!dockApplet->isSupported()) {
continue;
}
// ... 后续处理 pluginsVisible这里如果 改进建议： 3. 代码性能问题：频繁的信号发射 改进建议： 4. 代码安全问题：Lambda 捕获与生命周期 connect(dockApplet, &DS_NAMESPACE::DAppletDock::supportedChanged, this, [this]() { ... });如果 问题：空指针检查 总结与修改建议建议对 修改后的 如果 // 在 DockDBusProxy 类中添加一个槽函数
private slots:
void onAppletSupportedChanged() {
// 可以在这里加一些判断，比如检查发送者是否在当前列表中
auto pluginsVisible = DockSettings::instance()->pluginsVisible();
updateDockPluginsVisible(pluginsVisible);
Q_EMIT pluginsChanged();
}
// 在构造函数中
DockDBusProxy::DockDBusProxy(DockPanel* parent)
: QObject(parent)
{
// ... 其他初始化代码 ...
// 监听父对象（假设是 DockPanel）的信号，当有新 Applet 添加时连接
if (parent) {
// 假设 parent 有 appletAdded 信号
connect(parent, &DockPanel::appletAdded, this, [this](DAppletDock *applet) {
if (applet) {
connect(applet, &DAppletDock::supportedChanged, this, &DockDBusProxy::onAppletSupportedChanged);
}
});
// 初始化连接现有的 applets
for (auto *dockApplet : dockApplets(parent)) {
connect(dockApplet, &DAppletDock::supportedChanged, this, &DockDBusProxy::onAppletSupportedChanged);
}
}
}注意：上述代码假设 其他文件：
总体而言，代码逻辑是正确的，主要风险在于 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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
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.
Log: Fixed dock applet visibility issues when compositor is unavailable
Influence:
fix: 在DBus代理中过滤不支持的停靠小程序
Log: 修复合成器不可用时停靠小程序可见性问题
Influence:
Summary by Sourcery
Ensure dock applets respect both visibility and platform support, and exclude unsupported applets from DBus-exposed plugin state.
New Features:
Bug Fixes:
Enhancements: