fix(ui): improve drag-and-drop functionality in SideBarItem #654
+16
−3
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 guide (collapsed on small PRs)Reviewer's GuideSideBarItem drag-and-drop behavior is hardened by making the drop area always cover its parent, safely handling invalid target indices for library imports, and guarding dragFlag updates with bounds checks. Sequence diagram for drag-and-drop import with Library default targetsequenceDiagram
actor User
participant SideBarItemDropArea as SideBarItemDropArea
participant SideModel as SideModel
participant Presenter as Presenter
User->>SideBarItemDropArea: Drop files (drop.urls)
SideBarItemDropArea->>SideBarItemDropArea: Build urlList from drop.urls
SideBarItemDropArea->>SideBarItemDropArea: Determine toIndex
alt Valid toIndex
SideBarItemDropArea->>SideModel: get(toIndex)
SideModel-->>SideBarItemDropArea: item with uuid
SideBarItemDropArea->>Presenter: importMetas(urlList, uuid)
else Invalid toIndex
SideBarItemDropArea->>SideBarItemDropArea: targetUuid = all
SideBarItemDropArea->>Presenter: importMetas(urlList, all)
end
Presenter-->>SideBarItemDropArea: Import completed
Sequence diagram for dragFlag reset with bounds checkingsequenceDiagram
actor User
participant SideBarItemDropArea as SideBarItemDropArea
participant SideModel as SideModel
User->>SideBarItemDropArea: Drag leaves drop area
SideBarItemDropArea->>SideBarItemDropArea: onExited triggered
alt Valid toIndex
SideBarItemDropArea->>SideModel: setProperty(toIndex, dragFlag, false)
SideModel-->>SideBarItemDropArea: dragFlag updated
else Invalid toIndex
SideBarItemDropArea->>SideBarItemDropArea: Skip dragFlag update
end
SideBarItemDropArea->>SideBarItemDropArea: scrollDownTimer.stop()
SideBarItemDropArea->>SideBarItemDropArea: scrollUpTimer.stop()
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:
- The
toIndexbounds check is now duplicated in both the import logic andonExited; consider extracting a small helper or using a shared utility to avoid repeated index validation logic. - The hard-coded "all" UUID used as the default import target could be made a named constant or derived from the model to avoid reliance on a magic string.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `toIndex` bounds check is now duplicated in both the import logic and `onExited`; consider extracting a small helper or using a shared utility to avoid repeated index validation logic.
- The hard-coded "all" UUID used as the default import target could be made a named constant or derived from the model to avoid reliance on a magic string.
## Individual Comments
### Comment 1
<location> `src/music-player/musicbaseandsonglist/SideBarItem.qml:163-168` </location>
<code_context>
urlList.push(drop.urls[i])
- Presenter.importMetas(urlList, sideModel.get(toIndex).uuid)
+ // 对于 Library 类型，如果 toIndex 无效，则默认导入到所有音乐
+ var targetUuid = "all"
+ if (toIndex >= 0 && toIndex < sideModel.count) {
+ targetUuid = sideModel.get(toIndex).uuid
+ }
+ Presenter.importMetas(urlList, targetUuid)
}
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Defaulting to targetUuid = "all" is not constrained to Library type in code
This logic now applies the `"all"` default whenever `toIndex` is invalid, regardless of `control.type` or the model. If this handler is reused by non-Library sidebars, they would also import into the "all" bucket. Please constrain this behavior (e.g., by checking `control.type`/model type) or use a type-specific default so other sections aren’t affected.
Suggested implementation:
```
onEntered: {
console.log("onEntered.............", dragForSort)
for (var i = 0; i < drop.urls.length; i++)
urlList.push(drop.urls[i])
var hasValidIndex = toIndex >= 0 && toIndex < sideModel.count
var targetUuid = ""
if (hasValidIndex) {
targetUuid = sideModel.get(toIndex).uuid
} else if (control.type === "Library") {
// 对于 Library 类型，如果 toIndex 无效，则默认导入到所有音乐
targetUuid = "all"
} else {
console.warn("Drop ignored: invalid toIndex for non-Library sidebar", toIndex)
return
}
Presenter.importMetas(urlList, targetUuid)
}
```
This change assumes that a `control` object with a `type` property is available in this QML context and that `"Library"` is the exact value used for the library sidebar. If the actual identifier/value differs (e.g. `control.sectionType`, `"library"`), update the `control.type === "Library"` comparison accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dengzhongyuan365-dev
force-pushed
the
commit2026204
branch
from
d5d5d6d to
30795ca
Compare
- Adjusted the drop area anchoring to fill the parent regardless of control type. - Enhanced the import logic for Library type to default to importing to all music if the target index is invalid. - Added bounds checking for the drag flag property to prevent errors during drag-and-drop operations. log: improve drag-and-drop functionality in SideBarItem bug: https://pms.uniontech.com/bug-view-309621.html
dengzhongyuan365-dev
force-pushed
the
commit2026204
branch
from
30795ca to
678276e
Compare
deepin pr auto review这段代码是对 QML 中侧边栏项(SideBarItem)的拖放功能进行改进的补丁。主要改进了边界检查和异常处理逻辑。下面我将从多个维度进行详细审查： 1. 语法逻辑审查优点：
改进建议：
2. 代码质量审查优点：
改进建议：
3. 代码性能审查优点：
改进建议：
4. 代码安全审查优点：
改进建议：
5. 改进后的代码示例// 定义常量
readonly property string LIBRARY_UUID: "all"
// ... 其他代码 ...
onDropped: {
if (drop.hasUrls) {
// 使用展开运算符构建 urlList
const urlList = [...drop.urls]
// 简化边界检查逻辑
const isValidIndex = toIndex >= 0 && toIndex < sideModel.count
const targetUuid = isValidIndex ? sideModel.get(toIndex).uuid :
(control.type === "library" ? LIBRARY_UUID : null)
if (targetUuid) {
Presenter.importMetas(urlList, targetUuid)
} else {
console.warn("Drop ignored: invalid toIndex for non-library sidebar", toIndex)
}
}
}
onExited: {
// 简化边界检查
if (toIndex >= 0 && toIndex < sideModel.count) {
sideModel.setProperty(toIndex, "dragFlag", false)
}
scrollDownTimer.stop()
scrollUpTimer.stop()
}总结这段代码的改进主要增强了健壮性和安全性，通过增加边界检查和异常处理，避免了潜在的运行时错误。建议进一步优化代码结构，减少重复逻辑，并使用常量替代魔法字符串。整体来说，这是一次有价值的改进。 |
lzwind
approved these changes
Feb 5, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengzhongyuan365-dev, lzwind 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 |
Contributor
|
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.
log: improve drag-and-drop functionality in SideBarItem
bug: https://pms.uniontech.com/bug-view-309621.html
Summary by Sourcery
Improve sidebar drag-and-drop handling to be more robust and consistent across item types.
Bug Fixes: