Open
Conversation
Reviewer's GuideImplements a startup phase for tray items with a timer-driven end, exposing it via TraySortOrderModel and using new QML states and transitions to hide all tray items during startup and then animate them into view without layout flicker. Sequence diagram for tray startup phase and load animationsequenceDiagram
actor User
participant Dock
participant Model as TraySortOrderModel
participant StartupTimer as QTimer
participant TrayItemQML as TrayItemPositioner
participant StashedItemQML as StashedItemPositioner
User->>Dock: Start desktop session
Dock->>Model: Construct TraySortOrderModel()
activate Model
Model->>StartupTimer: new QTimer(this)
StartupTimer-->>Model: configured singleShot(500ms)
Model->>TrayItemQML: Q_PROPERTY startupPhase (true)
Model->>StashedItemQML: Q_PROPERTY startupPhase (true)
TrayItemQML->>TrayItemQML: itemVisible evaluates to false
StashedItemQML->>StashedItemQML: itemVisible evaluates to false
TrayItemQML->>TrayItemQML: enter state startup-hidden
StashedItemQML->>StashedItemQML: enter state startup-hidden
loop surfaces appear during startup
Dock->>Model: onAvailableSurfacesChanged()
Model->>Model: updateVisualIndexes()
Model->>Model: saveDataToDConfig()
alt startupPhase is true
Model->>StartupTimer: start() (reset 500ms)
end
end
StartupTimer-->>Model: timeout()
Model->>Model: setStartupPhase(false)
Model-->>TrayItemQML: startupPhaseChanged(false)
Model-->>StashedItemQML: startupPhaseChanged(false)
TrayItemQML->>TrayItemQML: leave startup-hidden\nresolve itemVisible
StashedItemQML->>StashedItemQML: leave startup-hidden\nresolve itemVisible
TrayItemQML->>TrayItemQML: Transition startup-hidden -> item-visible\nrun opacity/scale animations
StashedItemQML->>StashedItemQML: Transition startup-hidden -> item-visible or item-invisible\nrun opacity animation
Class diagram for updated TraySortOrderModel startup phaseclassDiagram
class TraySortOrderModel {
%% Q_PROPERTIES
bool isCollapsing
bool actionsAlwaysVisible
bool isUpdating
bool startupPhase
QList~QVariantMap~ availableSurfaces
QString stagedSurfaceId
int stagedVisualIndex
%% Methods
+TraySortOrderModel(QObject *parent)
+void stageDropPosition(QString surfaceId, int visualIndex)
+void commitStagedDrop()
+void clearStagedDrop()
+bool startupPhase() const
+void setStartupPhase(bool phase)
%% Signals
void collapsedChanged(bool collapsed)
void isCollapsingChanged(bool collapsing)
void actionsAlwaysVisibleChanged(bool visible)
void isUpdatingChanged(bool updating)
void startupPhaseChanged(bool phase)
void visualItemCountChanged(int count)
void availableSurfacesChanged(QList~QVariantMap~ surfaces)
void stagedDropChanged()
%% Internal state
bool m_collapsed
bool m_isCollapsing
bool m_actionsAlwaysVisible
bool m_isUpdating
bool m_startupPhase
std::unique_ptr~Dtk::Core::DConfig~ m_dconfig
QTimer * m_startupTimer
QList~QVariantMap~ m_availableSurfaces
}
class QTimer {
+void setSingleShot(bool singleShot)
+void setInterval(int msec)
+void start()
+void timeout()
}
TraySortOrderModel --> QTimer : uses m_startupTimer
State diagram for tray item visibility with startup phasestateDiagram-v2
[*] --> startup_hidden
state startup_hidden {
}
state item_visible {
}
state item_invisible {
}
startup_hidden --> item_visible: TraySortOrderModel.startupPhase becomes false
startup_hidden --> item_invisible: TraySortOrderModel.startupPhase becomes false and itemVisible false
item_visible --> item_invisible: itemVisible becomes false (e.g. isUpdating true or other rules)
item_invisible --> item_visible: itemVisible becomes true
note "Logic (TrayItemPositioner / StashedItemPositioner):\n- If TraySortOrderModel.startupPhase is true: itemVisible = false, force startup_hidden state.\n- After startupPhase becomes false, normal visibility rules apply (isUpdating, plugin visibility, etc.).\n- Transition from startup_hidden triggers entry animations into item_visible or item_invisible." as N1
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码主要实现了在系统托盘启动阶段隐藏所有托盘项，并在启动完成后通过动画显示它们的功能，以避免启动过程中的闪烁和布局抖动。以下是对代码的详细审查和改进意见： 1. 语法逻辑StashedItemPositioner.qml & TrayItemPositioner.qml
TraySortOrderModel.cpp
2. 代码质量
3. 代码性能
4. 代码安全
5. 改进建议
总结这段代码整体质量很高，逻辑清晰，有效地解决了启动时的闪烁问题。
修改后的代码片段建议（针对常量定义）： // traysortordermodel.h
class TraySortOrderModel : public QStandardItemModel
{
Q_OBJECT
// ... 其他属性 ...
public:
static constexpr int STARTUP_TIMEOUT_MS = 500; // 定义常量
// ...
};
// traysortordermodel.cpp
TraySortOrderModel::TraySortOrderModel(QObject *parent)
: QStandardItemModel(parent)
{
// ...
m_startupTimer->setInterval(STARTUP_TIMEOUT_MS); // 使用常量
// ...
}总体而言，这是一个可以合并的改动，只需在实际测试中关注启动速度较快时的动画流畅度。 |
![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 3 issues, and left some high level feedback:
- In both TrayItemPositioner.qml and StashedItemPositioner.qml,
itemVisiblereturns false duringstartupPhasewhile you also have astartup-hiddenstate conditioned onstartupPhase; since theitem-invisiblestate (when: !root.itemVisible) is declared afterstartup-hidden, it will win when both conditions are true, so you likely need to reorder the states or decoupleitemVisiblefromstartupPhaseto ensure the startup animation state is actually used. - Consider explicitly resetting
startupPhaseor starting them_startupTimerin the model constructor or an initialization path; as written, if no new surfaces arrive, the model will remain instartupPhaseindefinitely and tray items will stay hidden.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In both TrayItemPositioner.qml and StashedItemPositioner.qml, `itemVisible` returns false during `startupPhase` while you also have a `startup-hidden` state conditioned on `startupPhase`; since the `item-invisible` state (`when: !root.itemVisible`) is declared after `startup-hidden`, it will win when both conditions are true, so you likely need to reorder the states or decouple `itemVisible` from `startupPhase` to ensure the startup animation state is actually used.
- Consider explicitly resetting `startupPhase` or starting the `m_startupTimer` in the model constructor or an initialization path; as written, if no new surfaces arrive, the model will remain in `startupPhase` indefinitely and tray items will stay hidden.
## Individual Comments
### Comment 1
<location path="panels/dock/tray/traysortordermodel.cpp" line_range="68-71" />
<code_context>
updateVisualIndexes();
});
+
+ // Startup phase timer: end startup phase after 500ms of no new surfaces
+ m_startupTimer = new QTimer(this);
+ m_startupTimer->setSingleShot(true);
+ m_startupTimer->setInterval(500);
+ connect(m_startupTimer, &QTimer::timeout, this, [this](){
+ if (m_startupPhase) {
</code_context>
<issue_to_address>
**issue:** Consider a fallback to end startupPhase even when no surfaces ever appear.
Since the timer only starts in onAvailableSurfacesChanged, m_startupPhase never ends if no tray surfaces ever appear and QML will keep items hidden indefinitely. Consider starting the timer once in the constructor (or adding an initial one-shot) so startupPhase finishes even when there are no surfaces.
</issue_to_address>
### Comment 2
<location path="panels/dock/tray/package/TrayItemPositioner.qml" line_range="83" />
<code_context>
}
]
transitions: [
+ Transition {
+ from: "startup-hidden"
+ to: "*"
</code_context>
<issue_to_address>
**issue (bug_risk):** Transition from "startup-hidden" to "*" may override the dedicated "item-invisible" transition.
Since `from: "startup-hidden"; to: "*"` matches all targets, it will also apply to `startup-hidden -> item-invisible`. Declared before the specific `to: "item-invisible"` transition, it will likely override it, causing elements that should stay hidden to briefly animate in. Consider narrowing this to a visible state only (e.g. `to: "item-visible"`) or reordering/adjusting conditions so `startup-hidden -> item-invisible` uses the non-showing transition.
</issue_to_address>
### Comment 3
<location path="panels/dock/tray/package/StashedItemPositioner.qml" line_range="48" />
<code_context>
}
]
transitions: [
+ Transition {
+ from: "startup-hidden"
+ to: "*"
</code_context>
<issue_to_address>
**issue (bug_risk):** Same transition precedence issue for stashed items as in TrayItemPositioner.
In this case `from: "startup-hidden"; to: "*"` also matches `startup-hidden -> item-invisible`, so the show animation runs even when the item should stay hidden. Restrict the `to` state (e.g., only the visible state) or adjust transition order to prevent animations for items that remain invisible after startup.
</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
+68
to
+71
| // Startup phase timer: end startup phase after 500ms of no new surfaces | ||
| m_startupTimer = new QTimer(this); | ||
| m_startupTimer->setSingleShot(true); | ||
| m_startupTimer->setInterval(500); |
There was a problem hiding this comment.
issue: Consider a fallback to end startupPhase even when no surfaces ever appear.
Since the timer only starts in onAvailableSurfacesChanged, m_startupPhase never ends if no tray surfaces ever appear and QML will keep items hidden indefinitely. Consider starting the timer once in the constructor (or adding an initial one-shot) so startupPhase finishes even when there are no surfaces.
| } | ||
| ] | ||
| transitions: [ | ||
| Transition { |
There was a problem hiding this comment.
issue (bug_risk): Transition from "startup-hidden" to "*" may override the dedicated "item-invisible" transition.
Since from: "startup-hidden"; to: "*" matches all targets, it will also apply to startup-hidden -> item-invisible. Declared before the specific to: "item-invisible" transition, it will likely override it, causing elements that should stay hidden to briefly animate in. Consider narrowing this to a visible state only (e.g. to: "item-visible") or reordering/adjusting conditions so startup-hidden -> item-invisible uses the non-showing transition.
| } | ||
| ] | ||
| transitions: [ | ||
| Transition { |
There was a problem hiding this comment.
issue (bug_risk): Same transition precedence issue for stashed items as in TrayItemPositioner.
In this case from: "startup-hidden"; to: "*" also matches startup-hidden -> item-invisible, so the show animation runs even when the item should stay hidden. Restrict the to state (e.g., only the visible state) or adjust transition order to prevent animations for items that remain invisible after startup.
Member
Author
|
备注：暂时测试效果用，无视commitmsg和许可协议合规检查即可。 |
18202781743
approved these changes
Apr 16, 2026
|
[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 |
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.
Summary by Sourcery
Introduce a startup phase for the dock tray to control initial visibility and animations of tray items.
New Features:
Enhancements: