Entry points for review reminders
Purpose / Description
- Created the
ScheduleRemindersfragment in a newreviewreminderspackage - Added a new string resource for
ScheduleRemindersto12-dont-translate.xml - Added buttons to launch the
ScheduleRemindersactivity fromDeckPicker,StudyOptionsFragment, andHeaderFragment - Created ScheduleRemindersDestination to comply with new
DeckPickerViewModelcode - Made
HeaderFragmentindex the notifications and review reminders options conditionally - Updated a pre-existing
DeckPickerTesttest to also test theScheduleRemindersbutton - Made toggling the review reminders developer option reload the settings fragment
Fixes
- Part of GSoC 2025: Review Reminders
Approach
- Currently starting by creating the space within which the new system will function.
- Created a new
ScheduleRemindersactivity and defined the deck-specific and global editing scopes. This deviates slightly from my original GSoC proposal as per discussions on Discord: [1] [2] - This is updated from the originally planned name,
EditNotifications. - The next steps will be to fill in the RecyclerView within
ScheduleRemindersand to begin trying to create/read/update/delete notification data.
How Has This Been Tested?
- DeckPickerTest executes successfully.
- Opening
ScheduleRemindersthrough all created routes (long pressing a deck in theDeckPickerscreen, clicking the toolbar bell icon inStudyOptionsFragment, and clicking the option in Settings) works and displays expected information. - When the developer option is disabled, no new changes are accessible.
- Tested on a physical Samsung S23, API 34.
Learning
- This took me a while longer than I expected because I suddenly ran into some problems first with building and then with executing tests within
DeckPickerTest. I think Android Studio didn't like running the parameterized tests for some reason and I had to manually edit the build configuration of each individual test before hitting run. - I initially was trying to figure out how to import a new bell icon and even asked around the Discord server for the process for importing svgs before realizing there was already a bell icon in the resources folder. Whoops.
- Decided to remove the cumbersome three editing scopes and just focus on two to improve the UX experience.
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)
- [x] UI Changes: You have tested your change using the Google Accessibility Scanner
Apologies for the repeated force pushes, my code is failing a few tests. Running the tests on my own machine is really annoying because a lot of them are flaky (in particular I'm running up against this known bug here: #14796). Does anyone know if there's a fix for that issue? Discussion in that thread on fixing it seems to have stalled out.
For the future, in your PR and commit message, please uses backticks instead of square brackets. This way, github shows this as code.
Square bracket is great in kotlin documentation, because that's the way javadoc works and javadoc links to other part of the code. But it's not something that github or most git tools know how to interprets. So instead we should use markdown and not javadoc syntax
I'm not asking you to change the current commits or PR. I don't care enough about this issue to ask you to do extra work here
Apologies for the square brackets. I've corrected them to backticks where possible and will continue to do so in the future.
As per discussions on Discord, I've removed basically all edits to 12-dont-translate.xml and am now hardcoding all strings as literals within the code itself. I'll pull them out into a string resource file in August once the feature is stable. The exception is "Edit notifications", which I need to keep as a string resource so that DeckPickerContextMenu can accept it as an argument in an enum declaration (it doesn't work if I just pass in the string literal), and so that it can be used as a title in study_options_fragment.xml (the linter gets mad if I don't). For now, I've kept this single resource in 12-dont-translate.xml. Let me know if I should move it somewhere else.
Ah, something I've just thought of. Should I rename EditNotifications to EditReviewReminders? I've noticed that I sometimes substitute the word "notifications" for "review reminders" in my code which is probably not good. It might be best to standardize the name of this feature. Is "Review Reminders" the final official English name for this feature? It was the original name of this idea on the GSoC ideas list.
As per @lukstbit's recommendation, I've rebased and squashed all my commits together. I've done a major refactoring to switch EditNotifications into ScheduleReminders and am now using a fragment within SingleFragmentActivity rather than creating a whole new activity. The UI has changed marginally (the top bar is now white instead of blue) in keeping with how most other SingleFragmentActivity fragments are styled within AnkiDroid. Updated images are in the PR description.
Please do not review this PR. I'm actively refactoring it as I've realized I can streamline the UX. Sorry for asking for reviews and then going back and changing things again, I really do appreciate the constructive criticism of my code and am sorry for pulling it out of the merge pipeline when it's so close to the end.
Refactored. UI images in the PR description have been updated. Looking forward to reviews!
Thanks Ashish and Sanjay! Updated.
- Replaced
launchCatchingIOwithviewModelScope.launch
Cool! Thanks for the review. Since there are two approvals, I can mark this as Pending Merge, right?
I've incorporated the above suggestions into my code and will open up a PR for the updates once https://github.com/ankidroid/Anki-Android/pull/18364 is merged.