fix: The right-click menu for the album fails to appear.#379
Open
JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: The right-click menu for the album fails to appear.#379JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
JWWTSL wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JWWTSL 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 |
deepin pr auto review我来对这段代码进行审查和分析：
具体改进建议：
bool AlbumControl::renameAlbum(int UID, const QString &newName)
{
if (newName.isEmpty() || newName.length() > MAX_ALBUM_NAME_LENGTH) {
return false;
}
// 添加对特殊字符的检查
if (newName.contains(QRegExp("[<>:\"/\\|?*]"))) {
return false;
}
// 现有的类型判断逻辑...
}
function deleteSystemAlbum() {
if (sysListModel.count === 0) {
return;
}
var delAlbumName = albumControl.getCustomAlbumByUid(GStatus.currentCustomAlbumUId);
if (!delAlbumName) {
console.error("Failed to get album name");
return;
}
try {
albumControl.removeAlbum(GStatus.currentCustomAlbumUId);
// 其余逻辑...
} catch (e) {
console.error("Failed to delete album:", e);
DTK.sendMessage(thumbnailImage, qsTr("Failed to delete album"), "error");
}
}
bool AlbumControl::isSystemAlbumEditable(int UID) {
// 检查是否有编辑系统相册的权限
return checkSystemAlbumPermission(UID);
}
enum AlbumType {
Custom = 0,
Favourite = 1,
AutoImport = 2,
System = 3
};
AlbumType getAlbumType(int UID) {
if (UID == 0) return AlbumType::Favourite;
if (isAutoImportAlbum(UID)) return AlbumType::AutoImport;
if (isSystemAlbum(UID)) return AlbumType::System;
return AlbumType::Custom;
}这些改进可以提高代码的健壮性、安全性和可维护性。 |
Reviewer's GuideAdjusts system album sidebar behavior and menus so right-click always shows a populated context menu, adds rename/delete actions for system albums, wires up album deletion flow, broadens the empty-view handling to system/auto-import albums, and updates backend album renaming to handle multiple album types. Sequence diagram for system album right-click rename flowsequenceDiagram
actor User
participant SystemSideBar as SystemSideBarView
participant SideBarItem as SideBarItemDelegate
participant SystemMenu
participant AlbumControl
participant DBManager
participant GStatus
User->>SystemSideBar: Click system album item
SystemSideBar->>GStatus: set currentViewType = ViewCustomAlbum
SystemSideBar->>GStatus: set currentCustomAlbumUId = uuid
SystemSideBar->>SystemSideBar: set currentSystemIndex
User->>SystemSideBar: Right click current system album
SystemSideBar->>SystemMenu: popup()
User->>SystemMenu: Choose Rename
SystemMenu->>SideBarItem: invoke rename() on currentItem
SideBarItem->>SideBarItem: show keyLineEdit, user edits name
User->>SideBarItem: Confirm rename (editingFinished)
SideBarItem->>AlbumControl: renameAlbum(model.uuid, newName)
AlbumControl->>AlbumControl: infer AlbumDBType from UID
AlbumControl->>DBManager: renameAlbum(UID, newName, atype)
DBManager-->>AlbumControl: bool ok
AlbumControl-->>SideBarItem: ok
SideBarItem->>SideBarItem: update songName.text
SideBarItem->>GStatus: sigCustomAlbumNameChaged(currentCustomAlbumUId, newName)
Sequence diagram for deleting a system album via context menusequenceDiagram
actor User
participant SystemSideBar as SystemSideBarView
participant SystemMenu
participant RemoveDialog as RemoveAlbumDialog
participant SidebarView as SidebarScrollView
participant AlbumControl
participant SysListModel
participant GStatus
User->>SystemSideBar: Right click system album
SystemSideBar->>SystemMenu: popup()
User->>SystemMenu: Choose Delete
SystemMenu->>RemoveDialog: set deleteType = 2
SystemMenu->>RemoveDialog: show()
User->>RemoveDialog: Confirm delete
RemoveDialog->>SidebarView: deleteAlbum(type = 2)
SidebarView->>SidebarView: branch to deleteSystemAlbum()
SidebarView->>AlbumControl: getCustomAlbumByUid(currentCustomAlbumUId)
SidebarView->>AlbumControl: removeAlbum(currentCustomAlbumUId)
SidebarView->>SysListModel: adjust currentSystemIndex
SidebarView->>SysListModel: remove(currentSystemIndex)
SidebarView->>SidebarView: send DTK.sendMessage
alt sysListModel.count > 0
SidebarView->>SystemSideBar: set view.currentIndex
SidebarView->>SystemSideBar: set currentItem.checked = true
SidebarView->>GStatus: set currentViewType = ViewCustomAlbum
SidebarView->>GStatus: set currentCustomAlbumUId = sysListModel.get(newIndex).uuid
SidebarView->>GStatus: set searchEditText = ""
SidebarView->>SystemSideBar: currentItem.forceActiveFocus()
else no system albums left
SidebarView->>SidebarView: backCollection()
end
Class diagram for updated AlbumControl rename logicclassDiagram
class AlbumControl {
+bool renameAlbum(int UID, QString newName)
+bool isAutoImportAlbum(int UID)
+QString getCustomAlbumByUid(int UID)
+QVariant getAllCustomAlbumId()
+void removeAlbum(int UID)
}
class DBManager {
<<singleton>>
+static DBManager* instance()
+bool renameAlbum(int UID, QString newName, AlbumDBType atype)
}
class AlbumDBType {
<<enum>>
Favourite
Custom
AutoImport
}
AlbumControl --> DBManager : uses
AlbumControl --> AlbumDBType : infers
DBManager --> AlbumDBType : parameter
Flow diagram for NoPictureView visibility and behavior in custom and auto-import albumsflowchart TD
A[Album opened in CustomAlbumView] --> B{numLabelText is empty?}
B -- no --> Z[NoPictureView.visible = false]
B -- yes --> C{filterType == 0?}
C -- no --> Z
C -- yes --> D{isCustom or isSystemAutoImport or isNormalAutoImport?}
D -- yes --> E[NoPictureView.visible = true]
E --> F[bShowImportBtn = true]
E --> G[iconName = nopicture1]
D -- no --> E
E --> H[bShowImportBtn = false]
E --> I{GStatus.currentViewType == ViewCustomAlbum?}
I -- yes --> J[iconName = nopicture2]
I -- no --> K[iconName = nopicture3]
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 there - I've reviewed your changes and found some issues that need to be addressed.
- In
Sidebar.qml,currentSystemIndexis only updated inonItemClicked, so if a user right-clicks a non-selected system album,deleteSystemAlbum()may act on the previously selected album instead of the one under the cursor; consider settingcurrentSystemIndexinonItemRightClicked(e.g. viaindexFromUuid) or passing the index/uuid directly to the delete path. - System albums (
uuid1/2/3) are now markededitable: trueand wired into the genericrenameAlbum/delete flow; double-check whether system albums are intended to be user-renamable/deletable, and if not, gate the new "Rename"/"Delete" menu actions and delegate editing on a dedicated property rather than reusingeditablefor both. - The new
deleteSystemAlbum()largely duplicates selection-management logic from other delete paths; consider extracting a small helper to compute and apply the next selected index to keep the behavior consistent and easier to maintain across custom/import/system albums.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Sidebar.qml`, `currentSystemIndex` is only updated in `onItemClicked`, so if a user right-clicks a non-selected system album, `deleteSystemAlbum()` may act on the previously selected album instead of the one under the cursor; consider setting `currentSystemIndex` in `onItemRightClicked` (e.g. via `indexFromUuid`) or passing the index/uuid directly to the delete path.
- System albums (`uuid` 1/2/3) are now marked `editable: true` and wired into the generic `renameAlbum`/delete flow; double-check whether system albums are intended to be user-renamable/deletable, and if not, gate the new "Rename"/"Delete" menu actions and delegate editing on a dedicated property rather than reusing `editable` for both.
- The new `deleteSystemAlbum()` largely duplicates selection-management logic from other delete paths; consider extracting a small helper to compute and apply the next selected index to keep the behavior consistent and easier to maintain across custom/import/system albums.
## Individual Comments
### Comment 1
<location> `src/qml/SideBar/Sidebar.qml:23-26` </location>
<code_context>
signal sideBarListChanged(string type, string displayName)
property int currentImportCustomIndex: 0 //自动导入相册当前索引值
property int currentCustomIndex: 0 //自定义相册当前索引值
+ property int currentSystemIndex: 0 //系统相册当前索引值
property var devicePaths : albumControl.getDevicePaths()
property var albumPaths : albumControl.getAlbumPaths(GStatus.currentCustomAlbumUId)
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a separate `currentSystemIndex` risks going out of sync with the actual selection and deleting the wrong album.
`currentSystemIndex` is only updated in `onItemClicked`, but the current item in `systemSideBar.view` can also change via keyboard, programmatic selection, or state restore. In `deleteSystemAlbum()` this can delete a different album from `sysListModel` than the one visibly selected. Use `systemSideBar.view.currentIndex` (or `indexFromUuid(GStatus.currentCustomAlbumUId)`) directly in `deleteSystemAlbum()` instead of maintaining a separate index property.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
TAG Bot New tag: 6.0.49 |
Contributor
|
TAG Bot New tag: 6.0.50 |
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
2 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: Design Issues
bug: https://pms.uniontech.com/bug-view-337467.html
Summary by Sourcery
Fix system album sidebar context menu behavior and extend system album management capabilities.
Bug Fixes:
Enhancements: