Anki-Android
Anki-Android copied to clipboard
ManageSpaceActivity created.
Pull Request template
Purpose / Description
Fix the issue #12366 creating the activity with a small warning stating the only way to manage data is deleting it.
Fixes
Fix the issue #12366.
Approach
Created a Kotlin class to show the activity after declaring it in the Android Manifest.
How Has This Been Tested?
Compiled and ran on my cellphone, using Android Studio's run configuration to directly run the Activity.
Checklist
- [ x] You have a descriptive commit message with a short title (first line, max 50 chars).
- [ x] You have commented your code, particularly in hard-to-understand areas
- [ x] You have performed a self-review of your own code
- [ x] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
EDIT: Now Checked.
- [ x] UI Changes: You have tested your change using the Google Accessibility ##Scanner
First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.
Message to maintainers, this PR contains strings changes.
- Before merging this PR, it is best to run the "Sync Translations" GitHub action, then make and merge a PR from the i18n_sync branch to get translations cleaned out.
- Then merge this PR, and immediately do another translation PR so the huge change made by this PR's key changes are all by themselves.
Read more about updating strings on the wiki,
Hi,
FYI usually here, we amend commit and force push. No big deal since it’s going to squashed I assume. But when we actually use atomic commit, that helps ensure each commit has a single change.
Seems fine to me. Waiting for tests to run. It failed earlier but seems unrelated
I realize this was adding as string. However, we hopefully will never release with only this message, the string will be replaced by buttons. So no need to do all the work to send it to translators.
So I added a commit that hard code the string in English.
This currently impedes someone to clear the app data manually, besides uninstalling. I wouldn't merge this until a provisory button to clear the app data is on the activity
On the one hand, I'm totally fine with not being able to delete without uninstalling. Either the data are on ~/AnkiDroid and the button don't do anything Or the user is on alpha using scoped storage, and there should be virtually nobody that is not a dev' doing it. And I don't know of anyone
On the other hand, I guess that starting to add buttons would be nice, since that's what the UI should looks like and what the the next step should be. However, asking for one or a few entries in the menu may be easy. Ensuring they are linked to correctly implemented actions is going to be really hard for a first PR request.
@Noxor11 Can you please let us know how you fill about:
- adding element(s) in this activity that are clickable
- ensuring that the "Delete collection" ask for confirmation and delete the folder containing the collection
- or otherwise some other action mentioned in #12366 that you believe you can implement
@Noxor11 Can you please let us know how you fill about:
- adding element(s) in this activity that are clickable
- ensuring that the "Delete collection" ask for confirmation and delete the folder containing the collection
- or otherwise some other action mentioned in #12366 that you believe you can implement
Honestly I don't know the appropiate way of doing any of this in the app.
Not even the first step? Creating a list of actions in a menu is standard Android action. Opening a confirmation activity that can be confirmed or cancelled too. It's okay if it do nothing in a first commit, we can always call the function that deletes the collection later. You can also look at our code to see how some actions are done. Or come on discord and discuss with us to ask where to ask questions
This does not seem to be actually pending merge, re-labeling
Not even the first step? Creating a list of actions in a menu is standard Android action. Opening a confirmation activity that can be confirmed or cancelled too. It's okay if it do nothing in a first commit, we can always call the function that deletes the collection later. You can also look at our code to see how some actions are done. Or come on discord and discuss with us to ask where to ask questions
I managed to do it. The activity, along with the implemented "delete collection" button. If anything needs a change please let me know.
So I just pushed a new version. I hope back-end code is better. However, I admit I've been blocked by Front-End. I wished to used snackbar, as @BrayanDSO wanted, but it requires to have "An [Activity] that has a [CoordinatorLayout] with id root_layout
.". And there is no coordinatorLayout here.
Also, even trying to use a toast fail to show anything. (Even when directly called in initSubscreen
, so it should not be a question of scope here). @Noxor11 , when you tried your function, where you able to see the toast?
Same problem with the progress bar, which requires a Fragment activity.
I've just added number of back-up. However, #12562 also applies to it. We don't have something to directly delete extra back-ups. So still need to be done
@Noxor11 , when you tried your function, where you able to see the toast?
I was able to see it after some seconds. The screen was laggy while deleting the files, but the toast did show up.
Sorry for the late response, my laptop's screen broke last monday, and I hadn't been able to check on this.
@oakkitten Thanks a lot. Applied or answered to all. In case it was not clear, we rarely ask specific people for review, unless we believe we actually must uses this very person feedback. Your review is very welcomed, and it's welcome anywhere. As I tried to explain on wiki and sometime on discord, anyone can review. One person with the reviewer status must give a review, but we want two reviews, the second review can be anyone. And that's what we use to decide who to offer the reviewer status to
I've been tinkering with this, and I have a small question about finding unused media. The method that calculates unused media is attempting to normalize fields of cards that refer to media files, and the names of media files themselves. What is the problem that this solves? I am also not entirely sure if this solves the problem in a good way. If both normalized and non-normalized files exist, the non-normalized is silently deleted. Also, do we really at this point expect non-normalized card fields? In order to properly refactor this, or make a more suitable alternative, I want to have a better understanding of this. Any clues? @dae
If the filenames on disk and in fields are not normalized, it can lead to copies being created with a different encoding when syncing across platforms due to the normalization macOS performs, or images failing to appear because the field encoding does not match the disk encoding. Anki ensures the correct encoding is used when writing files into the media folder, so "check media" should only perform changes when users manually write files into the media folder. And no, you can't rely on fields being in the correct encoding, since users can disable automatic normalization.
I would not waste time on refactoring the linked code, since the backend already provides its own implementation.
I see. I made manage space activity start calculating unused media files when it is shown, so I'd prefer that opening it doesn't result in any changes in files or notes. Also, the legacy check()
method seems to be performing normalization unconditionally. So I guess I'll create another method for calculating unused media files only, that returns a marker if media check is required. As I understand, the backend doesn't support this at the time being.
I rebased this to main (Media.kt had a trivial conflict I fixed) and pushed it out (--force-with-lease
) so CI would run and it was up to date - just trying to push it forward a bit
I'll make another PR that will supersede this. (Because I can't add commits here.) Need a little time to polish it
This was the foundation of the PR for manage space activity that just merged, thank you!