Skip to content

Commit 3155af4

Browse files
david-allisonBrayanDSO
david-allison
authored and
BrayanDSO
committed
fix(custom-study): defer loading of 'customStudyDefaults'
Speeds up dialog appearance This takes > 100ms on my phone, and was reported to take >1s on larger collections * load 'defaults' async * if it loads before we build the initial dialog, no change * if it loads after the initial dialog, if appropriate, update the UI to disable 'increase limit' options * if a 'increase limit' option is clicked * wait for defaults to load, and inform the user if unavailable Tested with delay(5_000) inside loadCustomStudyDefaults Issue introduced in caadde7 Fixes 18416
1 parent c620082 commit 3155af4

File tree

2 files changed

+140
-39
lines changed

2 files changed

+140
-39
lines changed

‎AnkiDroid/src/main/java/com/ichi2/anki/dialogs/customstudy/CustomStudyDialog.kt

Lines changed: 126 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import androidx.core.os.bundleOf
4040
import androidx.core.view.isVisible
4141
import androidx.core.widget.doAfterTextChanged
4242
import androidx.fragment.app.setFragmentResult
43+
import androidx.lifecycle.lifecycleScope
4344
import anki.scheduler.CustomStudyDefaultsResponse
4445
import anki.scheduler.CustomStudyRequest.Cram.CramKind
4546
import anki.scheduler.copy
@@ -48,8 +49,8 @@ import com.ichi2.anki.CollectionManager.TR
4849
import com.ichi2.anki.CollectionManager.withCol
4950
import com.ichi2.anki.R
5051
import com.ichi2.anki.analytics.AnalyticsDialogFragment
52+
import com.ichi2.anki.asyncIO
5153
import com.ichi2.anki.common.utils.annotation.KotlinCleanup
52-
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption
5354
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.EXTEND_NEW
5455
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.EXTEND_REV
5556
import com.ichi2.anki.dialogs.customstudy.CustomStudyDialog.ContextMenuOption.STUDY_AHEAD
@@ -62,6 +63,7 @@ import com.ichi2.anki.dialogs.customstudy.TagLimitFragment.Companion.KEY_INCLUDE
6263
import com.ichi2.anki.dialogs.customstudy.TagLimitFragment.Companion.REQUEST_CUSTOM_STUDY_TAGS
6364
import com.ichi2.anki.launchCatchingTask
6465
import com.ichi2.anki.preferences.sharedPrefs
66+
import com.ichi2.anki.snackbar.showSnackbar
6567
import com.ichi2.anki.ui.internationalization.toSentenceCase
6668
import com.ichi2.anki.utils.ext.dismissAllDialogFragments
6769
import com.ichi2.anki.utils.ext.sharedPrefs
@@ -74,13 +76,15 @@ import com.ichi2.libanki.undoableOp
7476
import com.ichi2.utils.BundleUtils.getNullableInt
7577
import com.ichi2.utils.bundleOfNotNull
7678
import com.ichi2.utils.cancelable
79+
import com.ichi2.utils.coMeasureTime
7780
import com.ichi2.utils.customView
7881
import com.ichi2.utils.dp
7982
import com.ichi2.utils.negativeButton
8083
import com.ichi2.utils.positiveButton
8184
import com.ichi2.utils.setPaddingRelative
8285
import com.ichi2.utils.textAsIntOrNull
8386
import com.ichi2.utils.title
87+
import kotlinx.coroutines.Deferred
8488
import kotlinx.coroutines.runBlocking
8589
import kotlinx.parcelize.Parcelize
8690
import timber.log.Timber
@@ -115,7 +119,8 @@ import timber.log.Timber
115119
*
116120
* @see TagLimitFragment
117121
*/
118-
@KotlinCleanup("remove 'runBlocking' calls'")
122+
@KotlinCleanup("remove 'runBlocking' call'")
123+
@NeedsTest("deferredDefaults")
119124
class CustomStudyDialog : AnalyticsDialogFragment() {
120125
/** ID of the [Deck] which this dialog was created for */
121126
private val dialogDeckId: DeckId
@@ -139,9 +144,6 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
139144
?.findViewById<EditText>(R.id.custom_study_details_edittext2)
140145
?.textAsIntOrNull()
141146

142-
/** @see CustomStudyDefaults */
143-
private lateinit var defaults: CustomStudyDefaults
144-
145147
override fun onCreate(savedInstanceState: Bundle?) {
146148
super.onCreate(savedInstanceState)
147149
parentFragmentManager.setFragmentResultListener(REQUEST_CUSTOM_STUDY_TAGS, this) { _, bundle ->
@@ -166,9 +168,9 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
166168
override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
167169
super.onCreate(savedInstanceState)
168170
val option = selectedSubDialog
169-
this.defaults = runBlocking { withCol { sched.customStudyDefaults(dialogDeckId).toDomainModel() } }
170171
return if (option == null) {
171172
Timber.i("Showing Custom Study main menu")
173+
deferredDefaults = loadCustomStudyDefaults()
172174
// Select the specified deck
173175
runBlocking { withCol { decks.select(dialogDeckId) } }
174176
buildContextMenu()
@@ -182,8 +184,18 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
182184
* Continues the custom study process by showing an input dialog where the user can enter an
183185
* amount specific to that type of custom study(eg. cards, days etc).
184186
*/
185-
private fun onMenuItemSelected(item: ContextMenuOption) {
186-
val dialog: CustomStudyDialog = createInstance(dialogDeckId, item)
187+
private suspend fun onMenuItemSelected(item: ContextMenuOption) {
188+
// on a slow phone, 'extend limits' may be clicked before we know there's no new/review cards
189+
// show 'no cards due' if this occurs
190+
if (item.checkAvailability != null) {
191+
val defaults = withProgress { deferredDefaults.await() }
192+
if (!item.checkAvailability(defaults)) {
193+
showSnackbar(getString((R.string.studyoptions_no_cards_due)))
194+
return
195+
}
196+
}
197+
198+
val dialog: CustomStudyDialog = createSubDialog(dialogDeckId, item)
187199
requireActivity().showDialogFragment(dialog)
188200
}
189201

@@ -204,23 +216,44 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
204216
)
205217
val ta = TypedValue()
206218
requireContext().theme.resolveAttribute(android.R.attr.selectableItemBackground, ta, true)
207-
ContextMenuOption.entries
208-
.map {
209-
when (it) {
210-
EXTEND_NEW -> Pair(it, defaults.extendNew.isUsable)
211-
EXTEND_REV -> Pair(it, defaults.extendReview.isUsable)
212-
else -> Pair(it, true)
219+
220+
fun buildMenuItems() {
221+
ContextMenuOption.entries
222+
.map { option ->
223+
Pair(
224+
option,
225+
// if there's no availability check, it's enabled
226+
option.checkAvailability == null ||
227+
// if data hasn't loaded, defer the check and assume it's enabled
228+
!deferredDefaults.isCompleted ||
229+
// if unavailable, disable the item
230+
option.checkAvailability(deferredDefaults.getCompleted()),
231+
)
232+
}.forEach { (menuItem, isItemEnabled) ->
233+
(layoutInflater.inflate(android.R.layout.simple_list_item_1, container, false) as TextView)
234+
.apply {
235+
text = menuItem.getTitle(requireContext().resources)
236+
isEnabled = isItemEnabled
237+
setBackgroundResource(ta.resourceId)
238+
setTextAppearance(android.R.style.TextAppearance_Material_Body1)
239+
setOnClickListener {
240+
launchCatchingTask { onMenuItemSelected(menuItem) }
241+
}
242+
}.also { container.addView(it) }
213243
}
214-
}.forEach { (menuItem, isItemEnabled) ->
215-
(layoutInflater.inflate(android.R.layout.simple_list_item_1, container, false) as TextView)
216-
.apply {
217-
text = menuItem.getTitle(requireContext().resources)
218-
isEnabled = isItemEnabled
219-
setBackgroundResource(ta.resourceId)
220-
setTextAppearance(android.R.style.TextAppearance_Material_Body1)
221-
setOnClickListener { onMenuItemSelected(menuItem) }
222-
}.also { container.addView(it) }
244+
}
245+
246+
buildMenuItems()
247+
248+
// add a continuation if 'defaults' was not loaded
249+
if (!deferredDefaults.isCompleted) {
250+
launchCatchingTask {
251+
Timber.d("awaiting 'defaults' continuation")
252+
deferredDefaults.await()
253+
container.removeAllViews()
254+
buildMenuItems()
223255
}
256+
}
224257

225258
return AlertDialog
226259
.Builder(requireActivity())
@@ -236,6 +269,7 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
236269
*/
237270
@NeedsTest("17757: fragment not dismissed before result is output")
238271
private fun buildInputDialog(contextMenuOption: ContextMenuOption): AlertDialog {
272+
require(deferredDefaults.isCompleted || selectedSubDialog!!.checkAvailability == null)
239273
/*
240274
TODO: Try to change to a standard input dialog (currently the thing holding us back is having the extra
241275
TODO: hint line for the number of cards available, and having the pre-filled text selected by default)
@@ -398,12 +432,30 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
398432
}
399433
}
400434

401-
/** Line 1 of the number entry dialog */
435+
/**
436+
* Loads [CustomStudyDefaults] from the backend
437+
*
438+
* This method may be slow (> 1s)
439+
*/
440+
private fun loadCustomStudyDefaults() =
441+
lifecycleScope.asyncIO {
442+
coMeasureTime("loadCustomStudyDefaults") {
443+
withCol { sched.customStudyDefaults(dialogDeckId).toDomainModel() }
444+
}
445+
}
446+
447+
/**
448+
* Line 1 of the number entry dialog
449+
*
450+
* e.g. "Review forgotten cards"
451+
*
452+
* Requires [ContextMenuOption.checkAvailability] to be null/return true
453+
*/
402454
private val text1: String
403455
get() =
404456
when (selectedSubDialog) {
405-
EXTEND_NEW -> defaults.labelForNewQueueAvailable()
406-
EXTEND_REV -> defaults.labelForReviewQueueAvailable()
457+
EXTEND_NEW -> deferredDefaults.getCompleted().labelForNewQueueAvailable()
458+
EXTEND_REV -> deferredDefaults.getCompleted().labelForReviewQueueAvailable()
407459
STUDY_FORGOT,
408460
STUDY_AHEAD,
409461
STUDY_PREVIEW,
@@ -427,13 +479,25 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
427479
}
428480
}
429481

430-
/** Initial value of the number entry dialog */
482+
/**
483+
* Initial value of the number entry dialog
484+
*
485+
* Requires [ContextMenuOption.checkAvailability] to be null/return true
486+
*/
431487
private val defaultValue: String
432488
get() {
433489
val prefs = requireActivity().sharedPrefs()
434490
return when (selectedSubDialog) {
435-
EXTEND_NEW -> defaults.extendNew.initialValue.toString()
436-
EXTEND_REV -> defaults.extendReview.initialValue.toString()
491+
EXTEND_NEW ->
492+
deferredDefaults
493+
.getCompleted()
494+
.extendNew.initialValue
495+
.toString()
496+
EXTEND_REV ->
497+
deferredDefaults
498+
.getCompleted()
499+
.extendReview.initialValue
500+
.toString()
437501
STUDY_FORGOT -> prefs.getInt("forgottenDays", 1).toString()
438502
STUDY_AHEAD -> prefs.getInt("aheadDays", 1).toString()
439503
STUDY_PREVIEW -> prefs.getInt("previewDays", 1).toString()
@@ -467,16 +531,19 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
467531

468532
/**
469533
* Context menu options shown in the custom study dialog.
534+
*
535+
* @param checkAvailability Whether the menu option is available
470536
*/
471537
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
472538
enum class ContextMenuOption(
473539
val getTitle: Resources.() -> String,
540+
val checkAvailability: ((CustomStudyDefaults) -> Boolean)? = null,
474541
) {
475542
/** Increase today's new card limit */
476-
EXTEND_NEW({ TR.customStudyIncreaseTodaysNewCardLimit() }),
543+
EXTEND_NEW({ TR.customStudyIncreaseTodaysNewCardLimit() }, checkAvailability = { it.extendNew.isUsable }),
477544

478545
/** Increase today's review card limit */
479-
EXTEND_REV({ TR.customStudyIncreaseTodaysReviewCardLimit() }),
546+
EXTEND_REV({ TR.customStudyIncreaseTodaysReviewCardLimit() }, checkAvailability = { it.extendReview.isUsable }),
480547

481548
/** Review forgotten cards */
482549
STUDY_FORGOT({ TR.customStudyReviewForgottenCards() }),
@@ -608,15 +675,40 @@ class CustomStudyDialog : AnalyticsDialogFragment() {
608675
}
609676

610677
companion object {
611-
fun createInstance(
678+
/**
679+
* @see CustomStudyDefaults
680+
*
681+
* Singleton; initialized when the main screen is loaded
682+
* This exists so we don't need to pass an unbounded object between fragments
683+
*/
684+
private lateinit var deferredDefaults: Deferred<CustomStudyDefaults>
685+
686+
/**
687+
* Creates an instance of the Custom Study Dialog: a user can select a custom study type
688+
*/
689+
fun createInstance(deckId: DeckId): CustomStudyDialog =
690+
CustomStudyDialog().apply {
691+
arguments =
692+
bundleOfNotNull(
693+
ARG_DID to deckId,
694+
)
695+
}
696+
697+
/**
698+
* Creates an instance of the Custom Study sub-dialog for a user to configure
699+
* a selected custom study type
700+
*
701+
* e.g. After selecting "Study Ahead", entering the number of days to study ahead by
702+
*/
703+
fun createSubDialog(
612704
deckId: DeckId,
613-
contextMenuAttribute: ContextMenuOption? = null,
705+
contextMenuAttribute: ContextMenuOption,
614706
): CustomStudyDialog =
615707
CustomStudyDialog().apply {
616708
arguments =
617709
bundleOfNotNull(
618710
ARG_DID to deckId,
619-
contextMenuAttribute?.let { ARG_SUB_DIALOG_ID to it.ordinal },
711+
ARG_SUB_DIALOG_ID to contextMenuAttribute.ordinal,
620712
)
621713
}
622714

‎AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomStudyDialogTest.kt

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import com.ichi2.anki.dialogs.utils.performPositiveClick
3838
import com.ichi2.annotations.NeedsTest
3939
import com.ichi2.libanki.Collection
4040
import com.ichi2.libanki.Consts
41+
import com.ichi2.libanki.DeckId
4142
import com.ichi2.libanki.sched.Scheduler
4243
import com.ichi2.testutils.AnkiFragmentScenario
4344
import com.ichi2.testutils.isJsonEqual
@@ -217,20 +218,28 @@ class CustomStudyDialogTest : RobolectricTest() {
217218
}
218219
}
219220

220-
private fun argumentsDisplayingSubscreen(subscreen: ContextMenuOption) =
221-
requireNotNull(
221+
private fun argumentsDisplayingSubscreen(subscreen: ContextMenuOption): Bundle {
222+
@Suppress("RedundantValueArgument")
223+
fun setupDefaultValuesSingleton() {
224+
withCustomStudyFragment(argumentsDisplayingMainScreen(deckId = Consts.DEFAULT_DECK_ID)) { }
225+
}
226+
227+
setupDefaultValuesSingleton()
228+
229+
return requireNotNull(
222230
CustomStudyDialog
223-
.createInstance(
231+
.createSubDialog(
224232
deckId = Consts.DEFAULT_DECK_ID,
225233
contextMenuAttribute = subscreen,
226234
).arguments,
227235
)
236+
}
228237

229-
private fun argumentsDisplayingMainScreen() =
238+
private fun argumentsDisplayingMainScreen(deckId: DeckId = Consts.DEFAULT_DECK_ID) =
230239
requireNotNull(
231240
CustomStudyDialog
232241
.createInstance(
233-
deckId = Consts.DEFAULT_DECK_ID,
242+
deckId = deckId,
234243
).arguments,
235244
)
236245

0 commit comments

Comments
 (0)