refactor: Automatic Display Answer #9631
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The method is always called from onCollectionLoaded so we can remove a dependency
Extracts "onCollectionLoadError" to a method to remove duplicated code restoreCollectionPreferences: If an exception occurs, this will be because there is a problem with the collection, or a problem with the data Either way, we want to return to the deck picker
We're going to add functionality here, and this is the first step to making this change easier to reason about. Removes two variables from AbstractFlashcardViewer
Removes 4 variables in AbstractFlashcardViewer
on DisplayCardQuestion, we used to check to see which options to use. But these don't change (mOptUseGeneralTimerSettings is loaded in onCollectionLoaded) So we can remove the code, effectively keeping the class as a constant preferences are loaded onCreate collection is loaded onCollectionLoaded
Move loading the concern into its class
Move the generation logic into its own class This moves the initialisation of the class from onCreate to onCollectionLoaded This seems sensible, and removes the need for double initialization
Removes mTimeoutHandler from AbstractFlashcardViewer This is an incremental (bad) refactoring Now we've removed the handler, we can move most of the internal logic into the AutomaticAnswerSettings class This will let us make more methods private
moves more behavior inside the object Still not too well factored, but getting there
moves more behavior inside the object By default, we start the timer after all media has played If we are reading text, we don't calculate the time it takes. Instead, we wait for the text to complete before we display Q/A Allows us to make settings private
moves more behavior inside the object Allows us to make settings private
Split out settings from the answerer
refactor easy methods out of the reviewer getBackgroundColors getTextColors
Even if a media delay is provided, there should be no auto answer if the times are set to 0s
|
This should make #9170 a "good first issue" when it's in. |
|
Already got that ready for when this merges |
|
That's huge! So many people want that |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose / Description
Refactor to remove more code from the CardViewer
Fixes
Pre #2609
How Has This Been Tested?
Unit tested, and it seems to work. I've added follow on commits (not pushed yet) with more functionality, and these work
Checklist
ifstatements)