-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: bring up numeric keyboard if "type answer" is enabled and the expected answer is a number #19247
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
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.
From a quick look, try extracting part of the methods and adding unit tests to the code.
Also, I don't think that the code should be changing the user input. It's part of checking the recall if , or . were necessary. And Anki shows an answer comparison anyway, so I really think that it isn't necessary.
|
Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically |
|
Still working... |
|
@BrayanDSO I changed the code so it doesn’t modify the user’s input anymore and added a unit test. Would be glad if you could take a look in my newest commit. |
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.
All good, a few changes needed in the tests
AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
Show resolved
Hide resolved
AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt
Outdated
Show resolved
Hide resolved
AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt
Outdated
Show resolved
Hide resolved
|
I've made the required changes and hope everything is in order now. It would be great if someone could take another look at the code to make sure everything looks good. |
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'm happy to merge this as-is. Thank you!
Here's a patch containing a few thoughts/nitpicks
Optional: feel free to double tap shift, 'apply patch from clipboard' and select the ones you like
Subject: [PATCH] build(deps): remove mockito-core dependency
Now handled properly in mockito-kotlin
---
Index: AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt (revision 94e5159d0f880f5cbb85a0ba5e364e961a4a86e7)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerFragment.kt (date 1760845230057)
@@ -42,7 +42,6 @@
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.core.content.ContextCompat
import androidx.core.content.getSystemService
-import androidx.core.text.HtmlCompat
import androidx.core.view.ViewCompat
import androidx.core.view.WindowInsetsCompat
import androidx.core.view.WindowInsetsControllerCompat
@@ -101,13 +100,13 @@
import com.ichi2.themes.Themes
import com.ichi2.utils.dp
import com.ichi2.utils.show
+import com.ichi2.utils.stripHtml
import com.squareup.seismic.ShakeDetector
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import org.jetbrains.annotations.VisibleForTesting
import timber.log.Timber
-import java.lang.IllegalArgumentException
import kotlin.math.roundToInt
import kotlin.reflect.jvm.jvmName
@@ -357,9 +356,6 @@
InputType.TYPE_CLASS_TEXT
}
- /** Removes HTML tags from a string */
- fun stripHtml(html: String): String = HtmlCompat.fromHtml(html, HtmlCompat.FROM_HTML_MODE_LEGACY).toString()
-
private fun resetZoom() {
webView.settings.loadWithOverviewMode = false
webView.settings.loadWithOverviewMode = true
Index: AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt
--- a/AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt (revision 94e5159d0f880f5cbb85a0ba5e364e961a4a86e7)
+++ b/AnkiDroid/src/androidTest/java/com/ichi2/anki/ReviewerFragmentTest.kt (date 1760844768988)
@@ -127,7 +127,8 @@
closeGetStartedScreenIfExists()
closeBackupCollectionDialogIfExists()
- val inputTypeNumber = InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_FLAG_DECIMAL or InputType.TYPE_NUMBER_FLAG_SIGNED
+ val inputTypeNumber =
+ InputType.TYPE_CLASS_NUMBER or InputType.TYPE_NUMBER_FLAG_DECIMAL or InputType.TYPE_NUMBER_FLAG_SIGNED
val inputTypeText = InputType.TYPE_CLASS_TEXT
val testValues: List<Pair<String, Int>> =
@@ -142,14 +143,16 @@
"" to inputTypeText,
)
- testValues.forEachIndexed { index, (value, _) ->
- addNewBasicWithTypingCardAndDeck(value, index)
+ testValues.forEachIndexed { index, (typedAnswer, _) ->
+ addTypedAnswerNote(answer = typedAnswer).firstCard(col).update {
+ did = col.decks.id("Default$index")
+ }
}
// Check decks after adding all notes to ensure that the deck list is updated with the new cards
testValues.forEachIndexed { index, (_, expectedInputType) ->
// Ensures that we are in the deckpicker screen to make reviewDeckWithName work
- if (index > 0)onView(withId(R.id.back_button)).perform(click())
+ if (index > 0) onView(withId(R.id.back_button)).perform(click())
checkInputType(expectedInputType, index)
}
}
@@ -167,16 +170,6 @@
}
}
- private fun addNewBasicWithTypingCardAndDeck(
- value: String,
- index: Int,
- ) {
- val note = addBasicWithTypingNote(value, value)
- val card = note.firstCard(col)
- card.did = col.decks.id("Default$index")
- col.updateCards(listOf(card))
- }
-
private fun clickShowAnswerAndAnswerGood() {
clickShowAnswer()
ensureAnswerButtonsAreDisplayed()
Index: AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt
--- a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt (revision 94e5159d0f880f5cbb85a0ba5e364e961a4a86e7)
+++ b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/InstrumentedTest.kt (date 1760844707523)
@@ -190,10 +190,10 @@
return n
}
- internal fun addBasicWithTypingNote(
+ internal fun addTypedAnswerNote(
front: String = "Front",
- back: String = "Back",
- ): Note = addNoteUsingNoteTypeName("Basic (type in the answer)", front, back)
+ answer: String = "Answer",
+ ): Note = addNoteUsingNoteTypeName("Basic (type in the answer)", front, answer)
@DuplicatedCode("This is copied from RobolectricTest. This will be refactored into a shared library later")
fun addClozeNote(
Index: AnkiDroid/src/main/java/com/ichi2/utils/HtmlUtils.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/utils/HtmlUtils.kt b/AnkiDroid/src/main/java/com/ichi2/utils/HtmlUtils.kt
--- a/AnkiDroid/src/main/java/com/ichi2/utils/HtmlUtils.kt (revision 94e5159d0f880f5cbb85a0ba5e364e961a4a86e7)
+++ b/AnkiDroid/src/main/java/com/ichi2/utils/HtmlUtils.kt (date 1760697000276)
@@ -15,6 +15,7 @@
*/
package com.ichi2.utils
+import androidx.core.text.HtmlCompat
import com.ichi2.anki.common.utils.htmlEncode
object HtmlUtils {
@@ -30,3 +31,6 @@
fun escape(html: String): String = html.htmlEncode()
}
+
+/** Removes HTML tags from a string */
+fun stripHtml(html: String): String = HtmlCompat.fromHtml(html, HtmlCompat.FROM_HTML_MODE_LEGACY).toString()Optional: After this, please squash and force push your changes.
| } | ||
|
|
||
| /** Removes HTML tags from a string */ | ||
| fun stripHtml(html: String): String = HtmlCompat.fromHtml(html, HtmlCompat.FROM_HTML_MODE_LEGACY).toString() |
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.
nit: Either:
- move this inside
chooseInputType- doesn't need to be public - Move this to a reasnable utils class, it doesn't belong in the reviewer
bf45691 to
f80f982
Compare
Purpose / Description
bring up numeric keyboard if "type answer" is enabled and the expected answer is a number
Fixes
Approach
Check if the expected answer is a number; if it is, show the numeric keyboard; otherwise, show the regular text keyboard.
How Has This Been Tested?
Tested on physical devices:
Samsung S21
Samsung A54
Checklist
Please, go through these checks before submitting the PR.
Before:

After:
