-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Option to reset settings #9236
Option to reset settings #9236
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 for the PR.
Please remember to make new strings translatable.
There are no debug settings in the release APKs. Therefore, placing this action into the debug settings does not solve the problem or use case mentioned in the issue. We should find a better place for it. Unfortunately, I cannot think of a category that somehow works on this. IMO it's time to create a new settings category (Backup & Restore maybe?). It should contain the backup actions which are currently placed in history & cache.
@ All, what do you think on that suggestion?
app/src/main/java/org/schabi/newpipe/settings/DebugSettingsFragment.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/settings/DebugSettingsFragment.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/settings/DebugSettingsFragment.java
Outdated
Show resolved
Hide resolved
<Preference | ||
android:key="@string/reset_settings" | ||
android:title="@string/settings_category_reset_title" | ||
app:iconSpaceReserved="false" /> |
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.
We need to ensure that the title is fully displayed on small devices:
app:iconSpaceReserved="false" /> | |
app:singleLineTitle="false" | |
app:iconSpaceReserved="false" /> |
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.
I see. Thank you for pointing it out. I have committed the changes.
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.
I see. Thank you for pointing it out. I have committed the changes.
I think you can start working on the solution TobiGr mentioned above. I will help you.
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.
Since the deadline is passed and we have more assignments to do, also 4 final exams and a 2000-word essay are coming. We have to wait until 15 Nov or later to continue to work on this PR
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.
Ok, don't worry, and good luck with exams! ;-)
I will turn this PR into a draft for now, feel free to write a comment once you get back on it, so that we see it
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.
Understood. Thank you.
Kudos, SonarCloud Quality Gate passed! |
You can already reset your settings by clearing your storage for the app. EDIT:I forgot to mention it would also clear out all your history for the app! Sorry for the confusion 😞 |
6bc9581
to
f231a35
Compare
|
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.
LGTM
Please wait for approval by second maintainer regarding new settings structure
I would say let's have an extra precautionary step before the task is executed. The alert dialog should have a check box that needs to be checked before the Yes button can be tapped, until which it will remain greyed out. |
Having one dialog with CANCEL and OK is the same flow used for clearing the watch and search history as well as the playback positions. I think it should be consistent with these similar actions. |
Well, there should be a check box for those too. Any destructive action should have this double check. It's different from PC where you usually have to move the pointer quite a distance to execute the task. On Android, mis-taps are far more likely to occur. (e.g. just today I accidentally deleted an important SMS from its heads-up notification and had to request a re-send.) |
Ensuring title to be fully displayed on small devices. Co-authored-by: Tobi <TobiGr@users.noreply.github.com>
Following settings have been move to the new category: - import database (from ContenttSettings) - export database (from ContenttSettings) - reset settings (from DebugSettings)
8dd28a2
to
df2e0be
Compare
Kudos, SonarCloud Quality Gate passed! |
Quality Gate passedIssues Measures |
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!
What is it?
Description of the changes in your PR
Following settings have been move to the new category:
Before/After Screenshots/Screen Record
Screen.Recording.2022-10-29.at.10.04.33.am.mov
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence