immich-app/immich
1
0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2026-07-15 21:32:32 +03:00
Code Issues 783 Packages Projects Releases 22 Wiki Activity

[PR #25111] fix(mobile): use correct delete action #18074

New Issue
Open
opened 2026-02-05 16:32:14 +03:00 by OVERLORD · 0 comments
OVERLORD commented 2026-02-05 16:32:14 +03:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/immich-app/immich/pull/25111
Author: @ByteSizedMarius
Created: 1/7/2026
Status: 🔄 Open

Base: main ← Head: fix/mobile-use-correct-delete-action

📝 Commits (4)

  • c6ed6c1 fix(mobile): use correct delete action
  • db9387f pass deletedAt instead of computing isTrashed
  • dba886b fix: rm leftover istrashed from remoteasset
  • cfd98df fix: dart analyze

📊 Changes

4 files changed (+25 additions, -7 deletions)

View changed files

📝 mobile/lib/domain/models/asset/remote_asset.model.dart (+8 -2)
📝 mobile/lib/infrastructure/entities/remote_asset.entity.dart (+1 -0)
📝 mobile/lib/presentation/widgets/action_buttons/delete_permanent_action_button.widget.dart (+9 -2)
📝 mobile/lib/presentation/widgets/asset_viewer/bottom_bar.widget.dart (+7 -3)

📄 Description

Description

When viewing a trashed asset in the image viewer (library > trash > image viewer), the delete button incorrectly showed the same delete dialog used for the move to trash action (DeleteActionButton; "...This action will move the asset to the server's trash). This was confusing since the asset is already in the trash. Also, pressing this button and confirming the dialog wouldn't actually delete the image from trash.

Changes:

  • Added isTrashed field to RemoteAsset domain model (mapped from deletedAt in entity) to be able to access this parameter from image viewer
  • Viewer bottom bar checks asset.isTrashed and uses DeletePermanentActionButton for trashed assets
  • Added useShortLabel parameter to DeletePermanentActionButton to display "Delete" text in viewer instead of "Delete permanently", as it would break into two lines, which looked strange

I believe this fixes https://github.com/immich-app/immich/issues/21949, https://github.com/immich-app/immich/issues/20587

Note: DeletePermanentActionButton and DeleteActionButton currently do not currently have confirmation dialogs, which I find to be dangerous. This means, that pressing delete now will immediately delete the image from trash. Maybe this was conscious decision, but I will make a suggestion in a second PR.

Sorry about https://github.com/immich-app/immich/pull/25110 😅

How Has This Been Tested?

  • Open an image from the trash page
  • Verify the delete button does not bring up the old dialog
  • Verify tapping delete permanently removes the asset
  • Verify normal (non-trashed) images still show "Move to bin" behavior

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have written tests for new code (if applicable)
  • I have followed naming conventions/patterns in the surrounding code
  • [N/A] All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
  • [N/A] All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

Claude was used to assist with implementing this pull request (understanding why isTrashed from currentAsset wasn't working and adding isTrashed to domain model).

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/immich-app/immich/pull/25111 **Author:** [@ByteSizedMarius](https://github.com/ByteSizedMarius) **Created:** 1/7/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/mobile-use-correct-delete-action` --- ### 📝 Commits (4) - [`c6ed6c1`](https://github.com/immich-app/immich/commit/c6ed6c1aca029348734a944c89055593b78aea85) fix(mobile): use correct delete action - [`db9387f`](https://github.com/immich-app/immich/commit/db9387f4b17cb0ffde46c568a96965c51c737452) pass deletedAt instead of computing isTrashed - [`dba886b`](https://github.com/immich-app/immich/commit/dba886b16340304e0624466a24963a9d5fa1cfef) fix: rm leftover istrashed from remoteasset - [`cfd98df`](https://github.com/immich-app/immich/commit/cfd98df06dec2f921550e3fafe9d1cfa0078d392) fix: dart analyze ### 📊 Changes **4 files changed** (+25 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `mobile/lib/domain/models/asset/remote_asset.model.dart` (+8 -2) 📝 `mobile/lib/infrastructure/entities/remote_asset.entity.dart` (+1 -0) 📝 `mobile/lib/presentation/widgets/action_buttons/delete_permanent_action_button.widget.dart` (+9 -2) 📝 `mobile/lib/presentation/widgets/asset_viewer/bottom_bar.widget.dart` (+7 -3) </details> ### 📄 Description ## Description When viewing a trashed asset in the image viewer (library > trash > image viewer), the delete button incorrectly showed the same delete dialog used for the move to trash action (`DeleteActionButton`; "...This action will move the asset to the server's trash). This was confusing since the asset is already in the trash. Also, pressing this button and confirming the dialog wouldn't actually delete the image from trash. **Changes:** - Added `isTrashed` field to `RemoteAsset` domain model (mapped from `deletedAt` in entity) to be able to access this parameter from image viewer - Viewer bottom bar checks `asset.isTrashed` and uses `DeletePermanentActionButton` for trashed assets - Added `useShortLabel` parameter to `DeletePermanentActionButton` to display "Delete" text in viewer instead of "Delete permanently", as it would break into two lines, which looked strange I believe this fixes https://github.com/immich-app/immich/issues/21949, https://github.com/immich-app/immich/issues/20587 **Note:** `DeletePermanentActionButton` and `DeleteActionButton` currently do not currently have confirmation dialogs, which I find to be dangerous. This means, that pressing delete now will immediately delete the image from trash. Maybe this was conscious decision, but I will make a suggestion in a second PR. Sorry about https://github.com/immich-app/immich/pull/25110 😅 ## How Has This Been Tested? - [x] Open an image from the trash page - [x] Verify the delete button does not bring up the old dialog - [x] Verify tapping delete permanently removes the asset - [x] Verify normal (non-trashed) images still show "Move to bin" behavior ## Checklist: - [x] I have performed a self-review of my own code - [x] I have made corresponding changes to the documentation if applicable - [x] I have no unrelated changes in the PR. - [x] I have confirmed that any new dependencies are strictly necessary. - [x] I have written tests for new code (if applicable) - [x] I have followed naming conventions/patterns in the surrounding code - [ ] [N/A] All code in `src/services/` uses repositories implementations for database calls, filesystem operations, etc. - [ ] [N/A] All code in `src/repositories/` is pretty basic/simple and does not have any immich specific logic (that belongs in `src/services/`) ## Please describe to which degree, if any, an LLM was used in creating this pull request. Claude was used to assist with implementing this pull request (understanding why `isTrashed` from `currentAsset` wasn't working and adding `isTrashed` to domain model). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2026-02-05 16:32:14 +03:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
accessibility
changelog:enhancement
changelog:security
changelog:skip
changelog:translation
cli
date-time
dependencies
documentation
external-library
format
good first issue
mobile-beta
mobile-beta
mobile-beta
needs-answer
nice to have
pull-request
Mirrored from GitHub Pull Request
 sharing
tech-debt
📱mobile
🖥️web
🗄️server
🧠machine-learning
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
OVERLORD
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: immich-app/immich#18074
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?