Skip to content

Conversation

@ByteSizedMarius
Copy link
Contributor

@ByteSizedMarius ByteSizedMarius commented Jan 7, 2026

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 #21949, #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 #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).

When viewing a trashed asset, the viewer now shows the permanent delete button instead of "Move to bin" which made no sense for already-trashed items.

Changes:
- Added `isTrashed` field to RemoteAsset domain model
- Map `deletedAt` from entity to `isTrashed` in domain model
- Check asset.isTrashed in viewer bottom bar
- Added `useShortLabel` param to DeletePermanentActionButton for compact display
@bo0tzz
Copy link
Member

bo0tzz commented Jan 7, 2026

For future reference, it's basically never necessary to close a PR because the git history is bad; you can just (force) push the history you do want to the same branch.

@ByteSizedMarius
Copy link
Contributor Author

For future reference, it's basically never necessary to close a PR because the git history is bad; you can just (force) push the history you do want to the same branch.

Sorry about that, overeager

@shenlong-tanwen
Copy link
Member

@ByteSizedMarius Static analysis is still failing. Can you look into it?

@ByteSizedMarius
Copy link
Contributor Author

@ByteSizedMarius Static analysis is still failing. Can you look into it?

Also mb, will be more thorough in the future.

@shenlong-tanwen
Copy link
Member

The changes look good, but I'd rather merge this after #25113 is in. I'll add a TextWidget that takes care of rendering the HTML tags inside the translation texts soon so you can rebase that branch on top of it and use the existing key. I'll ping you once that is up. Thanks for working on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Immich trash does not delete assets (on Android app)

3 participants