Skip to content
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

Add option to delete files after export #247 #259

Conversation

BIBLETUM
Copy link
Contributor

@BIBLETUM BIBLETUM commented Feb 21, 2024

Description:

Add option to delete files after export
#247

Tasks:

  • Add option to delete files after export to settings layout
  • Add option to delete files after export on ImageViewFragment
  • Add option to delete files after export on GalleryFragment
  • Translate all new strings

@BIBLETUM BIBLETUM marked this pull request as draft February 21, 2024 11:30
@BIBLETUM BIBLETUM marked this pull request as ready for review February 21, 2024 11:31
@@ -98,7 +98,10 @@ class ImageViewerViewModel @Inject constructor(
currentPhoto ?: return@launch
currentPhoto!!.id ?: return@launch

val success = photoRepository.exportPhoto(currentPhoto!!)
if (success) onSuccess() else onError()
photoRepository.exportPhoto(currentPhoto!!).let {
Copy link
Owner

Choose a reason for hiding this comment

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

name "it" as "success"

@@ -100,11 +100,13 @@
<!-- EXPORT -->
<string name="export_are_you_sure">Bist du sucher, dass du %1$d Dateien, zurück in deine Galerie, <b>exportieren</b> willst?</string>
<string name="export_are_you_sure_this">Bist du sicher, dass du diese Datei, zurück in deine Galerie, <b>exportieren</b> willst?</string>
<string name="export_and_delete_are_you_sure_this">Sind Sie sicher, dass Sie diese Datei in Ihre Galerie <b>exportieren</b> und löschen möchten?</string>
Copy link
Owner

Choose a reason for hiding this comment

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

In german we use "du" and here are some grammar issues

Copy link
Owner

@leonlatsch leonlatsch Feb 21, 2024

Choose a reason for hiding this comment

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

Also, please don't use google translate for these translations. Add the english text with a TODO in order for the badges in the README to work.

@BIBLETUM BIBLETUM requested a review from leonlatsch February 21, 2024 14:31
Copy link
Owner

@leonlatsch leonlatsch left a comment

Choose a reason for hiding this comment

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

Very nice, BUT the message for exporting multiple images still needs to be adjusted. Its enough to do it for the new gallery and not the old one

Regarding "export & delete"

@leonlatsch
Copy link
Owner

@BIBLETUM and maybe you can find other icons for these options. Hard to imagine but maybe you find something in the assets of android studio. If not its ok to leave ic_delete for both

@BIBLETUM
Copy link
Contributor Author

Unfortunately could not find any other matching icons

@BIBLETUM BIBLETUM requested a review from leonlatsch February 22, 2024 07:47
Copy link
Owner

@leonlatsch leonlatsch left a comment

Choose a reason for hiding this comment

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

👍

@@ -249,5 +254,5 @@
<!-- Recovery Menu -->
<string name="recovery_subtitle">Recovery Menu</string> <!-- TODO -->
<string name="recovery_menu_item_open_photok">Open Photok</string> <!-- TODO -->
<string name="recovery_subtitle_reset_hide_app">Reset Hide App Setting</string> <!-- TODO -->
Copy link
Owner

Choose a reason for hiding this comment

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

this TODO needs to stay

@BIBLETUM BIBLETUM requested a review from leonlatsch February 22, 2024 09:35
@leonlatsch leonlatsch merged commit c4b1e96 into leonlatsch:develop Feb 22, 2024
1 check passed
leonlatsch added a commit that referenced this pull request Feb 22, 2024
**Description:**

- add german translation for feature implemented in #259 
- add missing todo


**Tasks:**
- [x] Build CI Runs
- [x] Code Analysis up to standards
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants