-
Notifications
You must be signed in to change notification settings - Fork 55
feat: enhance trash functionality with file count and drag hints #1290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's GuideThis PR enhances the dock’s trash icon by computing and displaying the number of items in the trash, adding an interactive drag‐and‐drop hint when dragging files over the trash icon, and updating the translation files to include the new UI strings. Sequence diagram for drag-and-drop hint on trash iconsequenceDiagram
actor User
participant "AppItem.qml"
participant "dragToolTip"
User->>"AppItem.qml": Drag file over trash icon
"AppItem.qml"->>"dragToolTip": Show 'Move to Trash' hint
User-->>"AppItem.qml": Drag file away from trash icon
"AppItem.qml"->>"dragToolTip": Hide hint
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Replace manual trash path construction with a standard API (e.g. QStandardPaths::TrashLocation or DTrashManager) to ensure the correct folder on all platforms.
- Switch to Qt pluralization (tr("%n file(s)", "", count)) rather than a fixed "-%1 files" so translations can properly handle singular vs. plural forms.
- Consider caching or watching the trash directory instead of rescanning it on every tooltip display to avoid UI jank when the trash contains many items.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Replace manual trash path construction with a standard API (e.g. QStandardPaths::TrashLocation or DTrashManager) to ensure the correct folder on all platforms.
- Switch to Qt pluralization (tr("%n file(s)", "", count)) rather than a fixed "-%1 files" so translations can properly handle singular vs. plural forms.
- Consider caching or watching the trash directory instead of rescanning it on every tooltip display to avoid UI jank when the trash contains many items.
## Individual Comments
### Comment 1
<location> `panels/dock/taskmanager/package/AppItem.qml:372` </location>
<code_context>
PanelToolTip {
id: toolTip
- text: root.name
+ text: root.itemId === "dde-trash" ? root.name + taskmanager.Applet.getTrashTipText() : root.name
toolTipX: DockPanelPositioner.x
toolTipY: DockPanelPositioner.y
</code_context>
<issue_to_address>
**suggestion:** Consider separating the tooltip text with a space or punctuation for clarity.
The tooltip currently joins root.name and the trash tip text without a separator, which can make the message unclear. Please add a space or punctuation between them for better readability.
```suggestion
text: root.itemId === "dde-trash" ? root.name + " - " + taskmanager.Applet.getTrashTipText() : root.name
```
</issue_to_address>
### Comment 2
<location> `panels/dock/taskmanager/taskmanager.cpp:437` </location>
<code_context>
+ QDir trashDir(trashPath);
+
+ if (trashDir.exists()) {
+ QStringList entries = trashDir.entryList(QDir::Files | QDir::Dirs | QDir::NoDotAndDotDot);
+ fileCount = entries.size();
+ }
</code_context>
<issue_to_address>
**suggestion:** Including directories in the Trash file count may be misleading.
entryList currently counts both files and directories. If you want to display only the number of files, use QDir::Files instead.
</issue_to_address>
### Comment 3
<location> `panels/dock/taskmanager/taskmanager.cpp:441` </location>
<code_context>
+ fileCount = entries.size();
+ }
+
+ return tr("-%1 files").arg(fileCount);
+}
+
</code_context>
<issue_to_address>
**suggestion:** Negative sign in tooltip string may be confusing.
Consider removing the leading dash or clarifying its meaning to avoid confusion about negative file counts.
```suggestion
return tr("%1 files in Trash").arg(fileCount);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Add file count display to trash tooltip showing number of files in trash 2. Implement drag-and-drop hint "Move to Trash" when dragging files over trash icon 3. Add new dragToolTip component that appears during file drag operations 4. Update translation files for multiple languages to support new features feat: 增强回收站功能,添加文件计数和拖拽提示 1. 在回收站工具提示中添加文件数量显示,显示回收站中的文件数量 2. 实现拖拽文件到回收站图标时显示"移动到回收站"提示 3. 添加新的拖拽工具提示组件,在文件拖拽操作期间显示 4. 更新多语言翻译文件以支持新功能 PMS: BUG-336327 BUG-336329
16deb9a to
a5a62ab
Compare
| PanelToolTip { | ||
| id: toolTip | ||
| text: root.name | ||
| text: root.itemId === "dde-trash" ? root.name + "-" + taskmanager.Applet.getTrashTipText() : root.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个是不是加载cpp里比较好,AppItem里直接区分dde-trash后返回需要的name,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
不太好加, 因为 从AM获取的数据,如果在name那边加,意义差不多,性能方面也一样。
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
|
/forcemerge |
|
This pr force merged! (status: behind) |
feat: 增强回收站功能,添加文件计数和拖拽提示
PMS: BUG-336327 BUG-336329
Summary by Sourcery
Enhance trash interactions in the dock by showing the number of items in trash and providing a drag‐and‐drop hint, and update translations to support the new UI strings.
New Features:
Documentation: