-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
settings: add UI options to import/export subscriptions #12344
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
base: dev
Are you sure you want to change the base?
settings: add UI options to import/export subscriptions #12344
Conversation
|
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.
Thank you, this is a very nice addition to avoid user confusion!
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.
The code you added in this class should be deduplicated with respect to the code in SubscriptionFragment.kt. You could create a few utils functions in a new file SubscriptionsImportExportHelper.kt under app/src/main/java/org/schabi/newpipe/local/subscription/services, and call them both from here and from SubscriptionFragment.kt. Obviously the ActivityResultLauncher cannot be deduplicated, but the rest of the code should be.
| private final ActivityResultLauncher<Intent> requestExportSubsLauncher = | ||
| registerForActivityResult(new ActivityResultContracts.StartActivityForResult(), | ||
| this::requestExportSubsResult); | ||
|
|
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.
| android:title="Export subscriptions" | ||
| android:summary="Export your subscriptions to a .json file" |
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.
Add new strings under strings.xml for these, otherwise they can't be translated
| android:title="Import subscriptions" | ||
| android:summary="Import subscriptions from a previous .json export" |
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.
Same here
| requireContext(), SubscriptionsExportService.class); | ||
| intent.putExtra(SubscriptionsExportService.KEY_FILE_PATH, fileUri); | ||
| requireContext().startService(intent); | ||
| Toast.makeText(requireContext(), R.string.exporting, Toast.LENGTH_SHORT).show(); |
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.
Instead of showing a Toast I would do the same as in SubscriptionFragment and show an ImportConfirmationDialog. This will come naturally after deduplicating the code.
| ); | ||
| intent.putExtra(SubscriptionsImportService.KEY_VALUE, fileUri); | ||
| requireContext().startService(intent); | ||
| Toast.makeText(requireContext(), R.string.importing, Toast.LENGTH_SHORT).show(); |
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.
This Toast is redundant with showToast(R.string.export_ongoing); in SubscriptionsExportService line 112, and should be removed



What is it?
Description of the changes in your PR
Adds two new options to the Backup & Restore settings screen:
Reuses existing import/export logic via SubscriptionsImportService and SubscriptionsExportService. These are 2 .json files created for testing the feature:
Before/After Screenshots/Screen Record
Fixes the following issue(s)
Due diligence