fix: improve navigation object tracking in control center #2989
+31
−14
Conversation
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
Reviewer's GuideRefactors how the control center tracks navigation objects by distinguishing between the module path and the fully triggered object chain, updating activation/deactivation and animation decisions to be based on this new triggered path, and exposing both paths as properties/signals in the plugin API. Sequence diagram for doShowPage navigation and triggeredObjects updatesequenceDiagram
actor User
participant UI as QML_UI
participant Manager as DccManager
participant Obj as DccObject
participant Parent as DccObject_parent
participant DccObject_Private
User ->> UI: request navigation(target, cmd)
UI ->> Manager: doShowPage(obj, cmd)
activate Manager
Manager->>Obj: pageType()
alt obj is MenuEditor with children
Manager->>Obj: getChildren()
Manager->>Obj: setCurrentObject(null)
Manager->>Obj: active("")
Note over Manager,Obj: triggeredObj set to first child
else
Manager->>Obj: setCurrentObject(null)
Manager->>Obj: active("")
end
loop build_triggered_chain_until_Menu
Manager->>Obj: pageType()
alt Obj is not Menu
Manager->>Manager: triggeredObjs.prepend(Obj)
Manager->>DccObject_Private: FromObject(Obj)
DccObject_Private-->>Manager: Parent
alt Parent exists
Manager->>Parent: setCurrentObject(Obj)
Manager->>Parent: active("")
end
Manager->>Manager: Obj = Parent
else
Manager->>Manager: break
end
end
Manager->>Manager: compute modules path
Manager->>Manager: triggeredObjs = modules + triggeredObjs
Manager->>Manager: determine animationMode
loop for each oldObj in m_triggeredObjects
alt oldObj not in triggeredObjs
Manager->>oldObj: setCurrentObject(null)
Manager->>Manager: animationMode = AnimationPop
end
alt oldObj != m_root and oldObj != triggeredObjs.last
Manager->>oldObj: deactive()
end
end
Manager->>Manager: setAnimationMode(animationMode)
Manager->>Manager: m_currentObjects = modules
Manager->>Manager: m_triggeredObjects = triggeredObjs
Manager-->>UI: triggeredObjectsChanged(m_triggeredObjects)
alt lastObj in m_currentObjects != m_activeObject
Manager->>Manager: m_activeObject = lastObj
Manager-->>UI: activeObjectChanged(m_activeObject)
end
deactivate Manager
Class diagram for updated navigation tracking in DccApp and DccManagerclassDiagram
class DccObject {
+enum PageType
+PageType pageType()
+QList~DccObject*~ getChildren()
+void setCurrentObject(DccObject* object)
+void active(QString cmd)
+void deactive()
}
class DccApp {
<<abstract>>
+int width()
+int height()
+DccObject* root()
+DccObject* activeObject()
+const QVector~DccObject*~ & currentObjects()
+const QVector~DccObject*~ & triggeredObjects()
+AnimationMode animationMode()
+void setAnimationMode(AnimationMode mode)
+void sidebarWidthChanged(int width)
+void rootChanged(DccObject* root)
+void activeObjectChanged(DccObject* activeObject)
+void currentObjectsChanged(QVector~DccObject*~ triggeredObjects)
+void triggeredObjectsChanged(QVector~DccObject*~ triggeredObjects)
+void activeItemChanged(QQuickItem* item)
+void animationModeChanged(AnimationMode mode)
-AnimationMode m_animationMode
}
class DccManager {
+DccObject* activeObject()
+const QVector~DccObject*~ & currentObjects()
+const QVector~DccObject*~ & triggeredObjects()
+void doShowPage(DccObject* obj, QString cmd)
-QVector~DccObject*~ m_currentObjects
-QVector~DccObject*~ m_triggeredObjects
-DccObject* m_root
-DccObject* m_activeObject
-DccObject* m_noAddObjects
-DccObject* m_noParentObjects
-PluginManager* m_plugins
-QPointer~QWindow~ m_window
}
class DccObject_Private {
+static DccObject_Private* FromObject(DccObject* object)
+DccObject* getParent()
}
DccManager --|> DccApp
DccManager o--> DccObject : uses
DccManager ..> DccObject_Private : FromObject
DccApp o--> DccObject : root
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 2 issues, and left some high level feedback:
- When updating
m_currentObjectsinDccManager::doShowPage, consider emitting the newcurrentObjectsChanged(...)notifier so theQ_PROPERTY currentObjectsbindings receive updates. - The parameter names for
currentObjectsChanged(QVector<DccObject *> triggeredObjects)andtriggeredObjectsChanged(QVector<DccObject *> triggeredObjects)are identical and misleading; renaming the first parameter to something likecurrentObjectswould make the API clearer. - Previously
triggeredObj->active(cmd)was emitted whencmdwas non-empty, but the new logic only callsactive(QString()); if external callers depend on the command string, consider preservingcmdwhere appropriate or explicitly documenting the behavior change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When updating `m_currentObjects` in `DccManager::doShowPage`, consider emitting the new `currentObjectsChanged(...)` notifier so the `Q_PROPERTY currentObjects` bindings receive updates.
- The parameter names for `currentObjectsChanged(QVector<DccObject *> triggeredObjects)` and `triggeredObjectsChanged(QVector<DccObject *> triggeredObjects)` are identical and misleading; renaming the first parameter to something like `currentObjects` would make the API clearer.
- Previously `triggeredObj->active(cmd)` was emitted when `cmd` was non-empty, but the new logic only calls `active(QString())`; if external callers depend on the command string, consider preserving `cmd` where appropriate or explicitly documenting the behavior change.
## Individual Comments
### Comment 1
<location> `src/dde-control-center/dccmanager.cpp:735-736` </location>
<code_context>
}
setAnimationMode(animationMode);
- // 触发新对象
- if (!cmd.isEmpty()) {
- Q_EMIT triggeredObj->active(cmd);
- }
</code_context>
<issue_to_address>
**issue (bug_risk):** Re-check whether the `cmd` argument still needs to be delivered to the triggered object.
Previously we called `triggeredObj->active(cmd)` when `cmd` was non-empty, so callers could rely on that string for navigation/initialization. Now only `active(QString())` on `tmpObj`/parents is used and `active(cmd)` is never invoked. Please confirm whether any existing call sites still depend on the non-empty `cmd`; if so, we likely need to emit `active(cmd)` on the final `triggeredObj` (or another suitable object). If `cmd` is being deprecated, ensure all callers have been updated accordingly.
</issue_to_address>
### Comment 2
<location> `src/dde-control-center/plugin/dccapp.h:80-81` </location>
<code_context>
void sidebarWidthChanged(int width);
void rootChanged(DccObject *root);
void activeObjectChanged(DccObject *activeObject);
+ void currentObjectsChanged(QVector<DccObject *> triggeredObjects);
+ void triggeredObjectsChanged(QVector<DccObject *> triggeredObjects);
void activeItemChanged(QQuickItem *item);
void animationModeChanged(AnimationMode mode);
</code_context>
<issue_to_address>
**suggestion:** Consider using const references and consistent parameter naming in the change signals.
`currentObjectsChanged` and `triggeredObjectsChanged` currently take `QVector<DccObject *>` by value, causing a full copy on each emission. Taking a `const QVector<DccObject *> &` would avoid this overhead. Also, `currentObjectsChanged` uses the parameter name `triggeredObjects`, which can be misleading; renaming it to something like `currentObjects` would better match its meaning.
Suggested implementation:
```c
void activeObjectChanged(DccObject *activeObject);
void currentObjectsChanged(const QVector<DccObject *> ¤tObjects);
void triggeredObjectsChanged(const QVector<DccObject *> &triggeredObjects);
void activeItemChanged(QQuickItem *item);
```
1. Update the corresponding signal definitions/implementations (if present) in the matching `.cpp` file so that their signatures exactly match:
- `void DccApp::currentObjectsChanged(const QVector<DccObject *> ¤tObjects)`
- `void DccApp::triggeredObjectsChanged(const QVector<DccObject *> &triggeredObjects)`
2. Update all `emit currentObjectsChanged(...)` and `emit triggeredObjectsChanged(...)` call sites to pass an lvalue `QVector<DccObject *>` (or a temporary that can bind to a `const &`), and adjust the parameter names if used inside the function bodies.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
-735
to
-736
| // 触发新对象 | ||
| if (!cmd.isEmpty()) { |
There was a problem hiding this comment.
issue (bug_risk): Re-check whether the cmd argument still needs to be delivered to the triggered object.
Previously we called triggeredObj->active(cmd) when cmd was non-empty, so callers could rely on that string for navigation/initialization. Now only active(QString()) on tmpObj/parents is used and active(cmd) is never invoked. Please confirm whether any existing call sites still depend on the non-empty cmd; if so, we likely need to emit active(cmd) on the final triggeredObj (or another suitable object). If cmd is being deprecated, ensure all callers have been updated accordingly.
Comment on lines
80
to
81
| void currentObjectsChanged(QVector<DccObject *> triggeredObjects); | ||
| void triggeredObjectsChanged(QVector<DccObject *> triggeredObjects); |
There was a problem hiding this comment.
suggestion: Consider using const references and consistent parameter naming in the change signals.
currentObjectsChanged and triggeredObjectsChanged currently take QVector<DccObject *> by value, causing a full copy on each emission. Taking a const QVector<DccObject *> & would avoid this overhead. Also, currentObjectsChanged uses the parameter name triggeredObjects, which can be misleading; renaming it to something like currentObjects would better match its meaning.
Suggested implementation:
void activeObjectChanged(DccObject *activeObject);
void currentObjectsChanged(const QVector<DccObject *> ¤tObjects);
void triggeredObjectsChanged(const QVector<DccObject *> &triggeredObjects);
void activeItemChanged(QQuickItem *item);- Update the corresponding signal definitions/implementations (if present) in the matching
.cppfile so that their signatures exactly match:void DccApp::currentObjectsChanged(const QVector<DccObject *> ¤tObjects)void DccApp::triggeredObjectsChanged(const QVector<DccObject *> &triggeredObjects)
- Update all
emit currentObjectsChanged(...)andemit triggeredObjectsChanged(...)call sites to pass an lvalueQVector<DccObject *>(or a temporary that can bind to aconst &), and adjust the parameter names if used inside the function bodies.
caixr23
force-pushed
the
master
branch
3 times, most recently
from
c5a10be to
b5b9224
Compare
xionglinlin
approved these changes
Feb 2, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, xionglinlin 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 |
Enhanced the navigation object tracking mechanism to properly handle page transitions and object activation. The changes include: 1. Added new triggeredObjects list to track the complete navigation path from root to current object 2. Improved object activation sequence by setting current objects for parent items 3. Fixed animation mode detection by comparing against triggered objects instead of current objects 4. Added proper signal emission for triggered objects changes 5. Updated header files to expose new triggeredObjects property and signals Log: Improved navigation tracking and page transition animations in control center Influence: 1. Test navigation between different control center modules 2. Verify proper animation transitions when switching pages 3. Check that current object tracking works correctly in complex navigation scenarios 4. Test backward navigation to ensure proper object deactivation 5. Verify signal emissions for object changes fix: 优化控制中心导航对象跟踪机制 增强导航对象跟踪机制以正确处理页面切换和对象激活。更改包括： 1. 新增 triggeredObjects 列表用于跟踪从根对象到当前对象的完整导航路径 2. 通过为父项设置当前对象改进了对象激活顺序 3. 通过比较触发对象而非当前对象修复了动画模式检测 4. 添加了触发对象变化的正确信号发射 5. 更新头文件以暴露新的 triggeredObjects 属性和信号 Log: 优化控制中心导航跟踪和页面切换动画 Influence: 1. 测试不同控制中心模块间的导航功能 2. 验证页面切换时的动画过渡是否正确 3. 检查复杂导航场景下的当前对象跟踪是否正常工作 4. 测试后退导航以确保对象停用正确 5. 验证对象变化时的信号发射 PMS: BUG-326725
deepin pr auto review这份代码 diff 主要针对 以下是对语法逻辑、代码质量、代码性能和代码安全方面的详细审查与改进意见： 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与改进建议总体来说，这是一份质量较高的重构代码，逻辑清晰，注释完善。 具体的改进建议如下：
|
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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.
Enhanced the navigation object tracking mechanism to properly handle page transitions and object activation. The changes include:
Log: Improved navigation tracking and page transition animations in control center
Influence:
fix: 优化控制中心导航对象跟踪机制
增强导航对象跟踪机制以正确处理页面切换和对象激活。更改包括：
Log: 优化控制中心导航跟踪和页面切换动画
Influence:
PMS: BUG-326725
Summary by Sourcery
Improve control center navigation tracking and page transition behavior by distinguishing between current pages and fully triggered navigation paths.
Bug Fixes:
Enhancements: