-
Notifications
You must be signed in to change notification settings - Fork 55
feat: add dock-specific tray item visibility control #1398
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
feat: add dock-specific tray item visibility control #1398
Conversation
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
update changelog to 2.0.19.2
|
TAG Bot TAG: 2.0.19.2 |
Reviewer's GuideAdds dock-specific tray item visibility control by introducing a separate DockVisibleRole, persisting dock-hidden surface IDs, and updating tray item layout logic and QML bindings to respect this new visibility dimension. Sequence diagram for dock-specific tray item visibility togglesequenceDiagram
actor User
participant DockUI
participant TrayItemPositionerQML as TrayItemPositioner_QML
participant TraySortOrderModel
participant DConfig
User ->> DockUI: Toggle dock visibility for tray item
DockUI ->> TrayItemPositionerQML: onToggleDockVisible(surfaceId, visible)
TrayItemPositionerQML ->> TraySortOrderModel: setDockVisible(surfaceId, visible)
alt visible is true
TraySortOrderModel ->> TraySortOrderModel: remove surfaceId from m_dockHiddenIds
else visible is false
TraySortOrderModel ->> TraySortOrderModel: add surfaceId to m_dockHiddenIds
end
TraySortOrderModel ->> TraySortOrderModel: updateVisualIndexes()
TraySortOrderModel ->> DConfig: setValue(dockHiddenSurfaceIds, m_dockHiddenIds)
TraySortOrderModel ->> DConfig: setValue(isCollapsed, m_collapsed)
DConfig -->> TraySortOrderModel: valueChanged(dockHiddenSurfaceIds)
TraySortOrderModel ->> TraySortOrderModel: loadDataFromDConfig()
TraySortOrderModel ->> TraySortOrderModel: updateVisualIndexes()
TrayItemPositionerQML ->> TraySortOrderModel: access model.visibility, model.dockVisible
TrayItemPositionerQML ->> DockUI: isVisible = visibility && dockVisible && sectionType rules
DockUI ->> User: Updated tray layout with dock-specific visibility
Class diagram for updated TraySortOrderModel dock visibility rolesclassDiagram
class TraySortOrderModel {
// Roles
<<enum>> Roles
SurfaceIdRole : int
VisibilityRole : int
DockVisibleRole : int
SectionTypeRole : int
VisualIndexRole : int
DelegateTypeRole : int
PluginFlagsRole : int
// QML invokable API
Q_INVOKABLE bool dropToDockTray(QString draggedSurfaceId, int dropVisualIndex, bool isBefore)
Q_INVOKABLE void setSurfaceVisible(QString surfaceId, bool visible)
Q_INVOKABLE bool isDisplayedSurface(QString surfaceId) const
Q_INVOKABLE void setDockVisible(QString surfaceId, bool visible)
Q_INVOKABLE bool isDockVisible(QString surfaceId) const
Q_INVOKABLE QModelIndex getModelIndexByVisualIndex(int visualIndex) const
// Persistence helpers
void loadDataFromDConfig()
void saveDataToDConfig()
void updateVisualIndexes()
QStandardItem * createTrayItem(QString name, int sectionType, QString delegateType)
// Visibility helpers
bool isDisplayedSurface(QString surfaceId) const
bool isDockVisible(QString surfaceId) const
void setSurfaceVisible(QString surfaceId, bool visible)
void setDockVisible(QString surfaceId, bool visible)
// State
QStringList m_collapsableIds
QStringList m_pinnedIds
QStringList m_fixedIds
QStringList m_hiddenIds
QStringList m_dockHiddenIds
bool m_collapsed
}
class DConfig {
void setValue(QString key, QVariant value)
QVariant value(QString key) const
signal valueChanged(QString key, QVariant value)
}
TraySortOrderModel --> DConfig : 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 - I've found 1 issue, and left some high level feedback:
- In
TraySortOrderModel::setDockVisible, consider early-returning whenvisiblestate doesn't actually change membership inm_dockHiddenIdsto avoid unnecessaryupdateVisualIndexes()and config writes. - The visibility computation in
updateVisualIndexes()for the stashed section ((pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) || !m_hiddenIds.contains(id)) is logically redundant (A || !Ais always true); it would be good to simplify this while you're modifying the visibility logic. - You repeat
bool dockVisible = !m_dockHiddenIds.contains(id);and the subsequent role updates in each section block ofupdateVisualIndexes(); consider extracting a small helper or consolidating the pattern to reduce duplication and keep the dock-visibility logic consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `TraySortOrderModel::setDockVisible`, consider early-returning when `visible` state doesn't actually change membership in `m_dockHiddenIds` to avoid unnecessary `updateVisualIndexes()` and config writes.
- The visibility computation in `updateVisualIndexes()` for the stashed section (`(pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) || !m_hiddenIds.contains(id)`) is logically redundant (`A || !A` is always true); it would be good to simplify this while you're modifying the visibility logic.
- You repeat `bool dockVisible = !m_dockHiddenIds.contains(id);` and the subsequent role updates in each section block of `updateVisualIndexes()`; consider extracting a small helper or consolidating the pattern to reduce duplication and keep the dock-visibility logic consistent.
## 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 dockVisible = !m_dockHiddenIds.contains(id);
results[0]->setData(SECTION_STASHED, TraySortOrderModel::SectionTypeRole);
</code_context>
<issue_to_address>
**issue (bug_risk):** The visibility condition for stashed items is logically always true and likely not what you intend.
`(pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock)` is a tautology, so `itemVisible` is always `true` and `m_hiddenIds` is effectively ignored in this branch. This also differs from other sections where `Dock::Attribute_ForceDock` is paired with `Dock::Attribute_CanSetting`. If the intent is something like "forcedock or non-configurable or not hidden", this likely should be `!(pluginFlags & Dock::Attribute_CanSetting)` instead of `!(pluginFlags & Dock::Attribute_ForceDock)`, so that hidden IDs actually take effect.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (stashPlaceholder == results[0]) continue; | ||
| // 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); |
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 visibility condition for stashed items is logically always true and likely not what you intend.
(pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) is a tautology, so itemVisible is always true and m_hiddenIds is effectively ignored in this branch. This also differs from other sections where Dock::Attribute_ForceDock is paired with Dock::Attribute_CanSetting. If the intent is something like "forcedock or non-configurable or not hidden", this likely should be !(pluginFlags & Dock::Attribute_CanSetting) instead of !(pluginFlags & Dock::Attribute_ForceDock), so that hidden IDs actually take effect.
deepin pr auto review代码审查报告:Dock托盘特定项目可见性控制功能总体评价该代码实现了一个新功能,允许用户控制特定托盘项目在Dock中的可见性,同时保持这些项目在其他区域(如控制中心)可见。整体实现逻辑清晰,遵循了现有代码风格,但在性能、代码冗余和错误处理方面有改进空间。 代码质量与逻辑问题1. 代码冗余问题在 // 在多个section处理中重复出现以下模式
bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_CanSetting) || !m_hiddenIds.contains(id);
bool dockVisible = !m_dockHiddenIds.contains(id);
results[0]->setData(SECTION_STASHED, TraySortOrderModel::SectionTypeRole);
results[0]->setData(itemVisible, TraySortOrderModel::VisibilityRole);
results[0]->setData(dockVisible, TraySortOrderModel::DockVisibleRole);
if (itemVisible && dockVisible) {
// 设置visualIndex
}建议:提取公共逻辑到一个私有方法中,减少代码重复,提高可维护性。 2. 变量命名不一致在
建议:统一命名风格,例如使用 3. 逻辑表达式问题在 bool itemVisible = (pluginFlags & Dock::Attribute_ForceDock) || !(pluginFlags & Dock::Attribute_ForceDock) || !m_hiddenIds.contains(id);其中 建议:简化为 性能问题1. 频繁的列表操作在 void TraySortOrderModel::setDockVisible(const QString &surfaceId, bool visible)
{
if (visible) {
if (m_dockHiddenIds.contains(surfaceId)) {
m_dockHiddenIds.removeOne(surfaceId);
}
} else {
if (!m_dockHiddenIds.contains(surfaceId)) {
m_dockHiddenIds.append(surfaceId);
}
}
updateVisualIndexes();
saveDataToDConfig();
}
建议:考虑使用 2. 不必要的更新每次调用 建议:在方法开始时检查值是否真的需要改变,避免不必要的更新。 安全性问题1. 缺少输入验证
建议:添加输入验证,确保 void TraySortOrderModel::setDockVisible(const QString &surfaceId, bool visible)
{
if (surfaceId.isEmpty()) {
qWarning() << "Attempt to set dock visibility for empty surface ID";
return;
}
// 其余代码...
}2. 缺少错误处理
建议:添加错误处理和日志记录,例如: void TraySortOrderModel::saveDataToDConfig()
{
if (!m_dconfig) {
qWarning() << "DConfig is not initialized";
return;
}
try {
m_dconfig->setValue("collapsableSurfaceIds", m_collapsableIds);
m_dconfig->setValue("pinnedSurfaceIds", m_pinnedIds);
m_dconfig->setValue("hiddenSurfaceIds", m_hiddenIds);
m_dconfig->setValue("dockHiddenSurfaceIds", m_dockHiddenIds);
m_dconfig->setValue("isCollapsed", m_collapsed);
} catch (const std::exception &e) {
qWarning() << "Failed to save data to DConfig:" << e.what();
}
}其他建议
function isItemVisible(model, collapsed) {
if (DDT.TraySortOrderModel.isUpdating) {
return false
}
const baseVisible = model.visibility && model.dockVisible;
if (model.sectionType === "collapsable") return !collapsed && baseVisible;
return model.sectionType !== "stashed" && baseVisible;
}总结该代码实现了基本功能需求,但在代码质量、性能和安全性方面有改进空间。主要建议包括:
这些改进将使代码更健壮、更易维护,并提高整体性能。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
06c5d8c
into
linuxdeepin:release/deepin25.0.10
|
/integrate |
|
AutoIntegrationPr Bot |
feat: add dock-specific tray item visibility control
Summary by Sourcery
Add support for controlling tray item visibility specifically for the dock, independent of general visibility settings.
New Features:
Enhancements: