-
Notifications
You must be signed in to change notification settings - Fork 55
feat: add dock-specific tray item visibility control #1360
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
Reviewer's GuideAdds a dock-specific visibility layer for tray items, backed by new dconfig state, and integrates it into tray sorting, visual index calculation, and QML positioning so items can be hidden from the dock while remaining visible elsewhere (e.g. control center). Sequence diagram for toggling dock-specific tray item visibilitysequenceDiagram
actor User
participant DockUI as DockTraySettingsQML
participant TrayModel as TraySortOrderModel
participant DConfig as DtkCoreDConfig
User->>DockUI: Toggle "Show in dock" for surfaceId
DockUI->>TrayModel: setDockVisible(surfaceId, false)
activate TrayModel
TrayModel->>TrayModel: update m_dockHiddenIds (add surfaceId)
TrayModel->>TrayModel: updateVisualIndexes()
TrayModel->>DConfig: setValue(dockHiddenSurfaceIds, m_dockHiddenIds)
DConfig-->>TrayModel: value persisted
deactivate TrayModel
Note over DockUI,TrayModel: Dock tray now filters by
Note over DockUI,TrayModel: visibility && dockVisible
User->>DockUI: Opens control center tray list
DockUI->>DConfig: value(hiddenSurfaceIds)
DConfig-->>DockUI: list without surfaceId (still visible)
DockUI->>TrayModel: isDisplayedSurface(surfaceId)
TrayModel-->>DockUI: true
Note over User,DockUI: Item is hidden in dock but remains
Note over User,DockUI: visible and configurable in control center
rect rgb(230,230,250)
Note over DConfig,TrayModel: On next startup
DConfig-->>TrayModel: valueChanged(dockHiddenSurfaceIds)
TrayModel->>TrayModel: loadDataFromDConfig()
TrayModel->>TrayModel: updateVisualIndexes()
end
Updated class diagram for TraySortOrderModel dock-specific visibilityclassDiagram
class TraySortOrderModel {
+enum Roles
+enum VisualSections
+TraySortOrderModel(parent : QObject)
+bool dropToDockTray(draggedSurfaceId : QString, dropVisualIndex : int, isBefore : bool)
+void setSurfaceVisible(surfaceId : QString, visible : bool)
+bool isDisplayedSurface(surfaceId : QString) const
+void setDockVisible(surfaceId : QString, visible : bool)
+bool isDockVisible(surfaceId : QString) const
+QModelIndex getModelIndexByVisualIndex(visualIndex : int) const
-void updateVisualIndexes()
-QStandardItem* createTrayItem(name : QString, sectionType : int, iconKey : QString, delegateType : int, forbiddenSections : int)
-void loadDataFromDConfig()
-void saveDataToDConfig()
-- roles --
-int SurfaceIdRole
-int VisibilityRole
-int DockVisibleRole
-int SectionTypeRole
-int VisualIndexRole
-int DelegateTypeRole
-int PluginFlagsRole
-- state --
-DtkCoreDConfig* m_dconfig
-QStringList m_collapsableIds
-QStringList m_pinnedIds
-QStringList m_fixedIds
-QStringList m_hiddenIds
-QStringList m_dockHiddenIds
-bool m_collapsed
}
class DtkCoreDConfig {
+QVariant value(key : QString) const
+void setValue(key : QString, value : QVariant)
+signal valueChanged(key : QString)
}
TraySortOrderModel --> DtkCoreDConfig : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 there - I've reviewed your changes and found some issues that need to be addressed.
- In
updateVisualIndexes()for the stashed section, theitemVisibleexpression was accidentally changed to useAttribute_ForceDockin both the second and third terms, making(pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock)always true and effectively ignoringm_hiddenIds; this should likely remainAttribute_CanSettingin the negated check as in the other sections. - The new
dockVisiblehandling currently ignoresAttribute_ForceDock, so items withAttribute_ForceDockcan still be hidden viadockHiddenSurfaceIds; if the intent is that force-docked items are always visible in the dock per the PR description, consider computingdockVisibleas(flags & Dock::Attribute_ForceDock) || !m_dockHiddenIds.contains(id)(and potentially enforcing this insetDockVisible).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `updateVisualIndexes()` for the stashed section, the `itemVisible` expression was accidentally changed to use `Attribute_ForceDock` in both the second and third terms, making `(pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock)` always true and effectively ignoring `m_hiddenIds`; this should likely remain `Attribute_CanSetting` in the negated check as in the other sections.
- The new `dockVisible` handling currently ignores `Attribute_ForceDock`, so items with `Attribute_ForceDock` can still be hidden via `dockHiddenSurfaceIds`; if the intent is that force-docked items are always visible in the dock per the PR description, consider computing `dockVisible` as `(flags & Dock::Attribute_ForceDock) || !m_dockHiddenIds.contains(id)` (and potentially enforcing this in `setDockVisible`).
## Individual Comments
### Comment 1
<location> `panels/dock/tray/traysortordermodel.cpp:385` </location>
<code_context>
// forcedock and can not setting plugin need always set to visible
auto pluginFlags = results[0]->data(TraySortOrderModel::PluginFlagsRole).toInt();
- bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) || !m_hiddenIds.contains(id);
+ bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) || !m_hiddenIds.contains(id); bool dockVisible = !m_dockHiddenIds.contains(id);
+ bool dockVisible = !m_dockHiddenIds.contains(id);
results[0]->setData(SECTION_STASHED, TraySortOrderModel::SectionTypeRole);
</code_context>
<issue_to_address>
**issue (bug_risk):** The combined declaration line both fails to compile and uses a visibility condition that is effectively always true.
Two separate issues here:
1) `itemVisible` and `dockVisible` are on the same physical line, which will not compile and harms readability. Since `dockVisible` is correctly declared on the next line, drop it from the first line and format as:
```cpp
bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock)
|| !(pluginFlags & Dock::Attribute_CanSetting)
|| !m_hiddenIds.contains(id);
bool dockVisible = !m_dockHiddenIds.contains(id);
```
2) The current expression
```cpp
(pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock)
```
is a tautology (`A || !A`), so `itemVisible` is effectively always `true` (aside from the `m_hiddenIds` part). Other sections use `!(pluginFlags & Dock::Attribute_CanSetting)`, which matches the comment and should be mirrored here for correctness and consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // forcedock and can not setting plugin need always set to visible | ||
| auto pluginFlags = results[0]->data(TraySortOrderModel::PluginFlagsRole).toInt(); | ||
| bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) || !m_hiddenIds.contains(id); | ||
| bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) || !m_hiddenIds.contains(id); bool dockVisible = !m_dockHiddenIds.contains(id); |
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 (bug_risk): The combined declaration line both fails to compile and uses a visibility condition that is effectively always true.
Two separate issues here:
itemVisibleanddockVisibleare on the same physical line, which will not compile and harms readability. SincedockVisibleis correctly declared on the next line, drop it from the first line and format as:
bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock)
|| !(pluginFlags & Dock::Attribute_CanSetting)
|| !m_hiddenIds.contains(id);
bool dockVisible = !m_dockHiddenIds.contains(id);- The current expression
(pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock)is a tautology (A || !A), so itemVisible is effectively always true (aside from the m_hiddenIds part). Other sections use !(pluginFlags & Dock::Attribute_CanSetting), which matches the comment and should be mirrored here for correctness and consistency.
431e5cc to
72cbb67
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, 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 |
1. Added new dconfig property "dockHiddenSurfaceIds" to store tray items that should be hidden in dock but remain visible in other areas like control center 2. Introduced DockVisibleRole to track separate visibility state for dock display 3. Modified TrayItemPositioner.qml to consider both visibility and dockVisible properties when positioning items 4. Updated tray item filtering logic to check dockVisible status in addition to existing visibility checks 5. Added setDockVisible and isDockVisible methods for controlling dock- specific visibility 6. Enhanced updateVisualIndexes method to handle both visibility states when calculating visual positions Log: Added ability to hide tray items from dock while keeping them visible in control center Influence: 1. Test tray items can be hidden from dock while remaining visible in control center 2. Verify dockHiddenSurfaceIds configuration is properly saved and loaded 3. Test visual positioning considers both visibility and dockVisible states 4. Verify tray items with Attribute_ForceDock flag are always visible in dock 5. Test collapsable, pinned, and fixed sections handle dock visibility correctly 6. Verify stash placeholder visibility logic works with new dockVisible property feat: 添加 Dock 特定托盘项可见性控制 1. 新增 dconfig 属性 "dockHiddenSurfaceIds" 用于存储在 Dock 中隐藏但在控 制中心等其他区域保持可见的托盘项 2. 引入 DockVisibleRole 来跟踪 Dock 显示的独立可见性状态 3. 修改 TrayItemPositioner.qml 在定位项目时同时考虑可见性和 dockVisible 属性 4. 更新托盘项过滤逻辑,在现有可见性检查基础上增加 dockVisible 状态检查 5. 添加 setDockVisible 和 isDockVisible 方法来控制 Dock 特定可见性 6. 增强 updateVisualIndexes 方法,在计算视觉位置时处理两种可见性状态 注: 加上此参数为了处理,在控制中心可以显示相应的插件项,保证在控制中心的任务栏插件列表可以驻留在任务栏的,但是即使控制中心可以控制保留驻留在任务栏,也可以通过此参数设置为不显示在任务栏。 Log: 新增在控制中心保持可见的同时从 Dock 隐藏托盘项的功能 Influence: 1. 测试托盘项可以从 Dock 隐藏同时在控制中心保持可见 2. 验证 dockHiddenSurfaceIds 配置是否正确保存和加载 3. 测试视觉定位同时考虑可见性和 dockVisible 状态 4. 验证带有 Attribute_ForceDock 标志的托盘项在 Dock 中始终可见 5. 测试可折叠、固定和固定部分正确处理 Dock 可见性 6. 验证隐藏占位符可见性逻辑与新的 dockVisible 属性正常工作 PMS: BUG-341141 BUG-341651
72cbb67 to
a3f715f
Compare
deepin pr auto review我来对这段代码进行审查:
优点:
改进建议: (1) 性能优化: void TraySortOrderModel::setDockVisible(const QString &surfaceId, bool visible)
{
bool changed = false;
if (visible) {
if (m_dockHiddenIds.contains(surfaceId)) {
m_dockHiddenIds.removeOne(surfaceId);
changed = true;
}
} else {
if (!m_dockHiddenIds.contains(surfaceId)) {
m_dockHiddenIds.append(surfaceId);
changed = true;
}
}
if (changed) {
updateVisualIndexes();
saveDataToDConfig();
}
}(2) 安全性考虑:
(3) 代码健壮性:
bool TraySortOrderModel::shouldShowInDock(const QString& surfaceId, bool itemVisible) const
{
return itemVisible && !m_dockHiddenIds.contains(surfaceId);
}(4) 命名规范:
(5) 错误处理:
这些改进建议主要关注代码的健壮性、性能和可维护性,不会影响现有功能。 |
|
/forcemerge |
|
This pr force merged! (status: behind) |
Log: Added ability to hide tray items from dock while keeping them visible in control center
Influence:
feat: 添加 Dock 特定托盘项可见性控制
注:
加上此参数为了处理,在控制中心可以显示相应的插件项,保证在控制中心的任务栏插件列表可以驻留在任务栏的,但是即使控制中心可以控制保留驻留在任务栏,也可以通过此参数设置为不显示在任务栏。
Log: 新增在控制中心保持可见的同时从 Dock 隐藏托盘项的功能
Influence:
PMS: BUG-341141 BUG-341651
Summary by Sourcery
Add dock-specific tray item visibility, backed by configuration, and integrate it into tray item positioning and visual index calculation.
New Features:
Enhancements: