Skip to content

Conversation

@ByteSizedMarius
Copy link
Contributor

@ByteSizedMarius ByteSizedMarius commented Jan 7, 2026

Description

Adds confirmation dialogs to DeleteActionButton and DeletePermanentActionButton to prevent accidental deletion. Addresses #19775.

When deleting photos/videos from grid view (multiselect), there's no confirmation dialog. This is particularly dangerous for locked folder content where deletion is permanent and irreversible. This is an opinionated change and open to discussion, but in my opinion, deletions (especially permanent, but also when sent to trash) should always have a confirmation.

I also changed the confirmation text in english, as I found it confusing/difficult to understand.

Ref #25111

Changes

  • DeleteActionButton: Change showConfirmation default from false to true (note: maybe these params could be removed, but I kept/added them for consistency)
  • DeletePermanentActionButton: Add showConfirmation parameter (default true) with confirmation dialog
  • Rm redundant showConfirmation: true in bottom_bar.widget.dart
  • Change confirmation message text (previously said "will prompt if you want to delete locally" but there was no such prompt)
  • Add i18n key delete_permanently_action_confirmation_message

How Has This Been Tested?

  • Manual testing: Grid view multiselect → Delete → Confirmation dialog appears
  • Manual testing: Grid view multiselect → Delete Permanently → Confirmation dialog appears
  • Manual testing: Single-item viewer → Delete → Confirmation dialog still works

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 Code was used to assist with implementation.

Adds confirmation dialogs to DeleteActionButton and DeletePermanentActionButton
in grid view to prevent accidental deletion. Addresses immich-app#19775.

- Change DeleteActionButton default showConfirmation to true
- Add showConfirmation parameter to DeletePermanentActionButton
- Add i18n key for permanent delete confirmation message
@immich-push-o-matic
Copy link

immich-push-o-matic bot commented Jan 7, 2026

Label error. Requires exactly 1 of: changelog:.*. Found: 📱mobile. A maintainer will add the required label.

"delete_local_dialog_ok_force": "Delete Anyway",
"delete_others": "Delete others",
"delete_permanently": "Delete permanently",
"delete_permanently_action_confirmation_message": "Are you sure you want to permanently delete this asset? This action cannot be undone.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not permanently_delete_assets_prompt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad.

Copy link
Contributor Author

@ByteSizedMarius ByteSizedMarius Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

permanently_delete_assets_prompt contains <b> for multiple assets, which is not rendered in flutter dialogs. How should this be handled? Has this usecase happened before?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass the total number of assets as the count and the translation will be resolved accordingly to the singular or multiple variant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didnt put <b> in inline code. What I was trying to communicate is, that the strings contain html that I do not see the mobile app currently being able to resolve or render. I understood how the singular/plural string resolving works.

@ByteSizedMarius
Copy link
Contributor Author

this PR depends on #25111 for delete actions to be correct. Without 25111, trash > imageviewer > delete still doesn't show a confirmation

@shenlong-tanwen
Copy link
Member

@ByteSizedMarius You can rebase this branch on top of #25739 which adds the ImmichHtmlText widget

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.

2 participants