Anki-Android
Anki-Android copied to clipboard
CustomStudyDialog uses Alert Dialog
Fixes
- Related #13315
Approach
Replaces usages of Deprecated MaterialDialog and adds the Native AlertDialog For CustomStudyDialog.
Images
Material Dialog | Alert Dialog |
---|---|
Material Dialog | Alert Dialog |
---|---|
Addition Consideration :
.listItems(items = getValuesFromKeys(keyValueMap, listIds).toList().map { it as CharSequence }) { _: MaterialDialog, _: Int, charSequence: CharSequence ->
For this we didn't had extension method I have used manual method for now using
.setItems()
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
tests failing
I am making this PR as draft for now. As I have many PR in queue once its ready for review I will open it again.
Also the code needs improvement for now. It's better first I will create an extension method then use it rather than creating .setItems it will lack DRY approach.
I am making this PR as draft for now. As I have many PR in queue once its ready for review I will open it again.
The PR is now ready for review.
Tests are failing
Hi @david-allison. The test is getting failed because in CustomStudyDialogTest we have used MaterialDialog so when testing it with AlertDialog its getting failed i guess. CustomStudyDialogTest
CustomStudyDialogTest > increaseNewCardLimitRegressionTest FAILED
java.lang.ClassCastException: class androidx.appcompat.app.AlertDialog cannot be cast to class com.afollestad.materialdialogs.MaterialDialog (androidx.appcompat.app.AlertDialog and com.afollestad.materialdialogs.MaterialDialog are in unnamed module of loader org.robolectric.internal.AndroidSandbox$SdkSandboxClassLoader @376498da)
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
Subject: [PATCH] fix: videos with only audio stream
---
Index: AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt
--- a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt (revision a6573e61bbaed9a836f46611564488a27631fc6f)
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt (date 1710368439209)
@@ -15,8 +15,7 @@
*/
package com.ichi2.anki.dialogs
-import android.app.AlertDialog
-import android.content.DialogInterface
+import androidx.appcompat.app.AlertDialog
import androidx.fragment.app.testing.FragmentScenario
import androidx.lifecycle.Lifecycle
import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -24,6 +23,7 @@
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.CustomStudyListener
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialogFactory
+import com.ichi2.anki.dialogs.utils.performPositiveClick
import com.ichi2.libanki.Collection
import com.ichi2.libanki.sched.Scheduler
import com.ichi2.testutils.ParametersUtils
@@ -40,6 +40,8 @@
import org.mockito.kotlin.mock
import org.mockito.kotlin.whenever
import org.robolectric.annotation.Config
+import kotlin.test.assertNotNull
+
@RunWith(AndroidJUnit4::class)
class CustomStudyDialogTest : RobolectricTest() {
@@ -62,12 +64,11 @@
.withArguments(CustomStudyDialog.ContextMenuOption.STUDY_AHEAD, 1)
.arguments
val factory = CustomStudyDialogFactory({ this.col }, mockListener)
- val scenario = FragmentScenario.launch(CustomStudyDialog::class.java, args, factory)
- scenario.moveToState(Lifecycle.State.STARTED)
+ val scenario = FragmentScenario.launch(CustomStudyDialog::class.java, args, androidx.appcompat.R.style.Theme_AppCompat, factory)
+ scenario.moveToState(Lifecycle.State.RESUMED)
scenario.onFragment { f: CustomStudyDialog ->
- val dialog = f.dialog as AlertDialog?
- MatcherAssert.assertThat(dialog, IsNull.notNullValue())
- dialog?.getButton(DialogInterface.BUTTON_POSITIVE)?.callOnClick()
+ val dialog = assertNotNull(f.dialog as AlertDialog?)
+ dialog.performPositiveClick()
}
val customStudy = col.decks.current()
MatcherAssert.assertThat("Custom Study should be dynamic", customStudy.isFiltered)
Index: AnkiDroid/src/test/java/com/ichi2/anki/dialogs/utils/AlertDialogUtils.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/utils/AlertDialogUtils.kt b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/utils/AlertDialogUtils.kt
--- a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/utils/AlertDialogUtils.kt (revision a6573e61bbaed9a836f46611564488a27631fc6f)
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/utils/AlertDialogUtils.kt (date 1710368645170)
@@ -16,9 +16,15 @@
package com.ichi2.anki.dialogs.utils
+import android.content.DialogInterface
import android.widget.TextView
import androidx.appcompat.app.AlertDialog
+import androidx.core.view.isVisible
+import org.hamcrest.MatcherAssert.*
+import androidx.test.platform.app.InstrumentationRegistry
+import com.ichi2.utils.HandlerUtils.executeFunctionUsingHandler
import com.ichi2.utils.getInputField
+import kotlin.test.assertNotNull
var AlertDialog.input
get() = getInputField().text.toString()
@@ -28,3 +34,12 @@
get() = requireNotNull(this.findViewById<TextView>(androidx.appcompat.R.id.alertTitle)) {
"androidx.appcompat.R.id.alertTitle not found"
}.text.toString()
+
+fun AlertDialog.performPositiveClick() {
+ // This exists as callOnClick did not call the listener
+ val positiveButton = assertNotNull(getButton(DialogInterface.BUTTON_POSITIVE), message = "positive button")
+ assertThat("button is visible", positiveButton.isVisible)
+ assertThat("button is enalbed", positiveButton.isEnabled)
+ executeFunctionUsingHandler { positiveButton.callOnClick() }
+ InstrumentationRegistry.getInstrumentation().waitForIdleSync()
+}
\ No newline at end of file
Test case passed!!! Special thanks to David for the patch that fixed the testcase.
Hi, should I squash down to two commits now
- Refactor to Alert Dialog
- Listitem code refactor
or should I squash down to one commit?
Two commits, both of which should compile if you git checkout <hash>
- Each commit in
main
should compile - Each commit should aim to do one thing, as this makes reviewing, diagnosing bugs and reverting easier
Squashed the commits.
Did a git checkout for https://github.com/ankidroid/Anki-Android/pull/15459/commits/b48180b3a861e6e7988654d03d844f0b7ad064a6 and it compiles.
@lukstbit should I remove this redundant package as this is no longer used in our test after CustomStudyDialog
migration?
https://github.com/ankidroid/Anki-Android/blob/6354574aee656b96f732928007618286e9e6c239/AnkiDroid/src/test/java/com/ichi2/testutils/DialogUtils.kt#L17-L37
can do any further refactors as an easy followup PR (e.g., removing a no-longer-needed package if that's the case)
Hi there @neeldoshii! This is the OpenCollective Notice for PRs merged from 2024-04-01 through 2024-04-30
If you are interested in compensation for this work, the process with details is here:
https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid
We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month
Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.
Thanks!