fix: Fix the issue of tray area icons moving when released#1488
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 9, 2026
Merged
fix: Fix the issue of tray area icons moving when released#1488deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImplements correct handling of tray icon drops onto the quick-settings action target and adjusts staged-drop commit logic to prevent tray icons from jumping/moving unexpectedly after drag-and-drop operations. Class diagram for updated TraySortOrderModel drop handlingclassDiagram
class TraySortOrderModel {
- QList~QString~ m_pinnedIds
- QString m_stagedSurfaceId
- int m_stagedVisualIndex
- QSet~int~ forbiddenSections
+ bool dropToDockTray(QString draggedSurfaceId, int dropVisualIndex, bool isBefore)
+ void commitStagedDrop()
+ void clearStagedDrop()
+ void updateVisualIndexes()
+ void stagedDropChanged()
}
class SectionContainer {
<<interface>> SectionContainer
+ bool removeOne(QString surfaceId)
}
TraySortOrderModel o--> SectionContainer : sourceSection
class PinnedSection {
+ QList~QString~ ids
+ void move(int fromIndex, int toIndex)
+ int indexOf(QString surfaceId)
+ int count()
+ void append(QString surfaceId)
}
TraySortOrderModel *-- PinnedSection : m_pinnedIds
class QuickSettingsTarget {
+ QString surfaceId
}
TraySortOrderModel .. QuickSettingsTarget : dropOnSurfaceId == internal/action-toggle-quick-settings
class ForbiddenSections {
+ bool contains(int section)
}
TraySortOrderModel *-- ForbiddenSections : forbiddenSections
class SectionsEnum {
<<enumeration>>
SECTION_PINNED
}
ForbiddenSections .. SectionsEnum : uses SECTION_PINNED
note for TraySortOrderModel "dropToDockTray now moves or appends draggedSurfaceId into m_pinnedIds when dropped on QuickSettingsTarget, unless SECTION_PINNED is forbidden; commitStagedDrop now clears staged state before calling updateVisualIndexes and dropToDockTray"
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 commitStagedDrop, you now call updateVisualIndexes() before dropToDockTray(surfaceId, visualIndex, true); please double-check that the visualIndex passed in is still valid after the reindexing and that dropToDockTray’s internal assumptions about indexes remain consistent with this new ordering.
- In the new quick-settings branch of dropToDockTray, consider defensively handling the case where m_pinnedIds.indexOf(draggedSurfaceId) returns -1 before calling move(), to avoid undefined behavior if the internal state ever becomes inconsistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In commitStagedDrop, you now call updateVisualIndexes() before dropToDockTray(surfaceId, visualIndex, true); please double-check that the visualIndex passed in is still valid after the reindexing and that dropToDockTray’s internal assumptions about indexes remain consistent with this new ordering.
- In the new quick-settings branch of dropToDockTray, consider defensively handling the case where m_pinnedIds.indexOf(draggedSurfaceId) returns -1 before calling move(), to avoid undefined behavior if the internal state ever becomes inconsistent.
## Individual Comments
### Comment 1
<location path="panels/dock/tray/traysortordermodel.cpp" line_range="174-175" />
<code_context>
if (dropOnSurfaceId == QLatin1String("internal/action-toggle-quick-settings")) {
- return false;
+ if (forbiddenSections.contains(SECTION_PINNED)) return false;
+ if (sourceSection == &m_pinnedIds) {
+ m_pinnedIds.move(m_pinnedIds.indexOf(draggedSurfaceId), m_pinnedIds.count() - 1);
+ } else {
+ sourceSection->removeOne(draggedSurfaceId);
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against a missing draggedSurfaceId in the pinned list before calling move().
This relies on `draggedSurfaceId` always being in `m_pinnedIds` when `sourceSection == &m_pinnedIds`. If that ever isn’t true, `indexOf` returns `-1` and `move(-1, …)` may assert or behave unpredictably. Please either guard with a `contains`/`indexOf` check before calling `move`, or add an assertion to document and enforce the invariant.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Fix the issue of tray area icons moving when released Log: Fix the issue of tray area icons moving when released pms: BUG-288727
deepin pr auto review这段代码修改涉及到了托盘图标排序逻辑的调整，主要针对拖拽到"快速设置"区域的行为进行了重新定义，并优化了暂存拖拽操作的提交逻辑。以下是对代码的详细审查和改进建议： 1. 语法逻辑审查修改点 1： if (dropOnSurfaceId == QLatin1String("internal/action-toggle-quick-settings")) {
if (forbiddenSections.contains(SECTION_PINNED)) return false;
if (sourceSection == &m_pinnedIds) {
int idx = m_pinnedIds.indexOf(draggedSurfaceId);
if (idx >= 0) {
m_pinnedIds.move(idx, m_pinnedIds.count() - 1);
}
} else {
sourceSection->removeOne(draggedSurfaceId);
m_pinnedIds.append(draggedSurfaceId);
}
return true;
}问题分析：
改进建议： if (dropOnSurfaceId == QLatin1String("internal/action-toggle-quick-settings")) {
if (forbiddenSections.contains(SECTION_PINNED)) return false;
if (!sourceSection) {
qWarning() << "sourceSection is null in dropToDockTray";
return false;
}
if (sourceSection == &m_pinnedIds) {
int idx = m_pinnedIds.indexOf(draggedSurfaceId);
if (idx >= 0) {
// 如果已经在固定区域，只需移动到末尾
beginMoveRows(QModelIndex(), idx, idx, QModelIndex(), m_pinnedIds.count());
m_pinnedIds.move(idx, m_pinnedIds.count() - 1);
endMoveRows();
}
} else {
// 从其他区域移动到固定区域
int sourceIdx = sourceSection->indexOf(draggedSurfaceId);
if (sourceIdx >= 0) {
beginRemoveRows(QModelIndex(), sourceIdx, sourceIdx);
sourceSection->removeOne(draggedSurfaceId);
endRemoveRows();
beginInsertRows(QModelIndex(), m_pinnedIds.count(), m_pinnedIds.count());
m_pinnedIds.append(draggedSurfaceId);
endInsertRows();
}
}
return true;
}修改点 2： void TraySortOrderModel::commitStagedDrop() {
if (m_stagedSurfaceId.isEmpty() || m_stagedVisualIndex < 0) {
return;
}
// Save staged values before clearing
QString surfaceId = m_stagedSurfaceId;
int visualIndex = m_stagedVisualIndex;
m_stagedSurfaceId.clear();
m_stagedVisualIndex = -1;
emit stagedDropChanged();
updateVisualIndexes();
dropToDockTray(surfaceId, visualIndex, true);
}问题分析：
改进建议： void TraySortOrderModel::commitStagedDrop() {
if (m_stagedSurfaceId.isEmpty() || m_stagedVisualIndex < 0) {
return;
}
// Save staged values before clearing
QString surfaceId = m_stagedSurfaceId;
int visualIndex = m_stagedVisualIndex;
// First perform the drop operation
dropToDockTray(surfaceId, visualIndex, true);
// Then clear staged state
m_stagedSurfaceId.clear();
m_stagedVisualIndex = -1;
emit stagedDropChanged();
// Finally update visual indexes after the drop operation
updateVisualIndexes();
}2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
5. 综合改进建议以下是改进后的完整代码： bool TraySortOrderModel::dropToDockTray(const QString &draggedSurfaceId, int dropVisualIndex, bool isBefore) {
if (draggedSurfaceId.isEmpty()) {
return false;
}
// Find source section
QStringList *sourceSection = findSourceSection(draggedSurfaceId);
if (!sourceSection) {
qWarning() << "Source section not found for" << draggedSurfaceId;
return false;
}
// Handle drop to quick settings
if (dropOnSurfaceId == QLatin1String("internal/action-toggle-quick-settings")) {
if (forbiddenSections.contains(SECTION_PINNED)) {
return false;
}
return handleDropToPinnedSection(sourceSection, draggedSurfaceId);
}
// Handle other drop cases...
}
bool TraySortOrderModel::handleDropToPinnedSection(QStringList *sourceSection, const QString &surfaceId) {
if (sourceSection == &m_pinnedIds) {
int idx = m_pinnedIds.indexOf(surfaceId);
if (idx >= 0 && idx < m_pinnedIds.count() - 1) {
beginMoveRows(QModelIndex(), idx, idx, QModelIndex(), m_pinnedIds.count());
m_pinnedIds.move(idx, m_pinnedIds.count() - 1);
endMoveRows();
}
} else {
int sourceIdx = sourceSection->indexOf(surfaceId);
if (sourceIdx >= 0) {
beginRemoveRows(QModelIndex(), sourceIdx, sourceIdx);
sourceSection->removeOne(surfaceId);
endRemoveRows();
beginInsertRows(QModelIndex(), m_pinnedIds.count(), m_pinnedIds.count());
m_pinnedIds.append(surfaceId);
endInsertRows();
}
}
return true;
}
void TraySortOrderModel::commitStagedDrop() {
if (m_stagedSurfaceId.isEmpty() || m_stagedVisualIndex < 0) {
return;
}
QString surfaceId = m_stagedSurfaceId;
int visualIndex = m_stagedVisualIndex;
// Perform the drop operation first
dropToDockTray(surfaceId, visualIndex, true);
// Then clear staged state
m_stagedSurfaceId.clear();
m_stagedVisualIndex = -1;
emit stagedDropChanged();
// Finally update visual indexes
updateVisualIndexes();
}总结
|
BLumia
approved these changes
Mar 9, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, pengfeixx 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 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: unstable) |
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.
Fix the issue of tray area icons moving when released
Log: Fix the issue of tray area icons moving when released
pms: BUG-288727
Summary by Sourcery
Fix tray icon reordering behaviour when dropping on the quick settings toggle and committing staged drops so icons no longer jump or move unexpectedly after release.
Bug Fixes: